Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Perl 5.38.2.2 for 32bit? #177

Open
gregoa opened this issue Apr 20, 2024 · 23 comments
Open

Perl 5.38.2.2 for 32bit? #177

gregoa opened this issue Apr 20, 2024 · 23 comments

Comments

@gregoa
Copy link

gregoa commented Apr 20, 2024

I notice that there is a 64bit version for 5.38.2.2 but the 32bit version is still at 5.38.2.1. Would it be possible to update the latter as well?

@shawnlaffan
Copy link
Contributor

The fourth digit in the version number is used for Strawberry Perl versioning, i.e. where changes are made to the SP build options, packaged modules and similar. The changes for 64 bit are listed with the release: https://github.com/StrawberryPerl/Perl-Dist-Strawberry/releases/tag/SP_53822_64bit

There have been no such changes to the 32 bit build for 5.38.2 so there is no need for a new release at this time. If there is a 5.38.3 Perl release then we will likely also build a Strawberry Perl version.

@sisyphus
Copy link

A couple of observations about that 32-bit build (5.38.2.1):

  1. I can't find any link to it at https://strawberryperl.com/releases.html;
  2. It has optimize level -Os.

IME it's only the 64-bit build that can't handle optimize level -O2, and -O2 is fine with the 32-bit builds.

@shawnlaffan, I hadn't realized that 5.38.2.2 had all of those additional optimization flags. (I've just been using plain ol' -Os for my 64-bit builds.)
Are those extra flags all included in level -O2 ?
(I think I should start including them in my own 64-bit builds.)

@mohawk2
Copy link

mohawk2 commented Apr 22, 2024

What causes the problem with 64-bit and -O2? Is it reported to Perl5?

@sisyphus
Copy link

Is it reported to Perl5?

The original report is at Perl/perl5#20024 .
The PR that was merged to work around this issue is at Perl/perl5#20136

@shawnlaffan
Copy link
Contributor

1. I can't find any link to it at https://strawberryperl.com/releases.html;

It does need to be added to the list. cc @genio

2. It has optimize level `-Os`.

The GNUMakefile does not differentiate between 64 and 32 bit, and neither does the SP build recipe. -Os has nearly all of the -O2 optimisations so hopefully the speed difference is small in practice.

@shawnlaffan, I hadn't realized that 5.38.2.2 had all of those additional optimization flags. (I've just been using plain ol' -Os for my 64-bit builds.) Are those extra flags all included in level -O2 ? (I think I should start including them in my own 64-bit builds.)

The extra flags should all be from -O2. They are the set identified by @kenneth-olwing in Perl/perl5#20081 (comment)

@gregoa
Copy link
Author

gregoa commented Apr 22, 2024

The fourth digit in the version number is used for Strawberry Perl versioning, i.e. where changes are made to the SP build options, packaged modules and similar. The changes for 64 bit are listed with the release: https://github.com/StrawberryPerl/Perl-Dist-Strawberry/releases/tag/SP_53822_64bit

There have been no such changes to the 32 bit build for 5.38.2 so there is no need for a new release at this time. If there is a 5.38.3 Perl release then we will likely also build a Strawberry Perl version.

Right; I just happened to notice that there's 1 commit which adds a patch and is so far only applied to the 64bit recipe:
09bf10c#diff-a1fb92f3b097292ccdbfc7cb64931b1bb2ebca12e73ea469d2cd8df62b9b2203

@shawnlaffan
Copy link
Contributor

That patch is needed to build perl 5.38 with gcc 13, which is used for the 64 bit releases (I cannot find the relevant issue right now).

The 32 bit release is built using gcc 8 which does not suffer the underlying issue. It is also built using the same stack as SP 5.32 so is getting a bit long in the tooth. Rebuilding all the external libraries at 32 bits for a newer compiler will likely be a fair amount of work so we are focusing on the 64 bit builds given they are much more commonly used.

@gregoa
Copy link
Author

gregoa commented Apr 23, 2024

That patch is needed to build perl 5.38 with gcc 13, which is used for the 64 bit releases (I cannot find the relevant issue right now).

Oh, I see, thanks.

@genio
Copy link
Member

genio commented Apr 23, 2024

I'll take a look at it when I land in Portugal for the Toolchain Summit

@shawnlaffan
Copy link
Contributor

@genio - are you able to update the releases list on the website to add the 32-bit 5.38?

@genio
Copy link
Member

genio commented Jun 14, 2024

I'm most of the way through a rebuild of the releases page to have all of that content auto-generated from the releases.json file. It's way too giant of a table mess at the moment and I'd rather make it somewhat more manageable. The front page already does that on the browser side.

Is there any reason we can't parse that releases.json in the browser and populate the page? Or, do we think that people are parsing that page as static content and would be upset at that large of a change? I changed the main page during the Perl ToolChain Summit and half expected to get yelled at by people that were parsing that page, but nobody has complained yet.

@shawnlaffan
Copy link
Contributor

I see no reason not to use this approach.

I suspect most people would use the releases.json file directly rather than scrape the web site.

@genio
Copy link
Member

genio commented Jul 6, 2024

Can I get a 👍 or 👎 on a re-design at https://strawberryperl.com/foo.html ?

@shawnlaffan
Copy link
Contributor

shawnlaffan commented Jul 7, 2024

Looks good overall.

A couple of minor comments:

  • Can we add links to the respective release notes? (Should the URLs be in releases.json? Edit - they are already there)
  • It might be useful to pad out where we have no recent releases. For example the 32-bit page has 5.16 as the latest MSI release. Padding can help make it clear there is no release for a specific version.
  • Although it would seem there is an issue with the generation process since we have 32-bit MSI releases up to 5.32 on the current page and that version is in the releases.json file (I did not check others but assume it is consistent).
  • There is the same basic issue for the 32-bit PDL releases.

@genio
Copy link
Member

genio commented Jul 7, 2024

  1. Can we add links to the release notes? Yes. That's now done.
  2. Padding out releases might be a bit more difficult. I started with that in mind and changed tack when seeing how inconsistent things were.
  3. I don't think there's an issue. The ones listed on the old page don't distinguish between plain 32-bit vs 32-bit with 64-bit ints.
  4. Same issue as 3. They're all there, just in the more specific tab.

Should we combine the two 32-bit release types into one category (preference to one vs the other?)?
Same question for the 64-bit release types.

@mohawk2
Copy link

mohawk2 commented Jul 7, 2024

Can the release notes for each version be expanded to include which version of PDL is included?

@shawnlaffan
Copy link
Contributor

  1. Can we add links to the release notes? Yes. That's now done.

Thanks.

2. Padding out releases might be a bit more difficult. I started with that in mind and changed tack when seeing how inconsistent things were.

This might not be necessary given the full set is now listed, although a marker that there is (currently) no 5.34 (32- or 64-bit) and no 32-bit 5.36 would be useful.

3. I don't think there's an issue. The ones listed on the old page don't distinguish between plain 32-bit vs 32-bit with 64-bit ints.

4. Same issue as `3`. They're all there, just in the more specific tab.

Should we combine the two 32-bit release types into one category (preference to one vs the other?)? Same question for the 64-bit release types.

I see now. Combining the 32-bit releases makes sense. Same for the 64-bit-ld releases given there are none after 5.26.

@shawnlaffan
Copy link
Contributor

Can the release notes for each version be expanded to include which version of PDL is included?

Not easily. The site is built from the releases.json file, and this does not include such information.

The release notes html files list the installed modules, but these seem only to be done for the main installation.
https://github.com/StrawberryPerl/Perl-Dist-Strawberry/blob/39de5b0bb7362887239b4376b2773d17e57fa5c3/share/64bit-5.38.2.2.pp#L523C49-L523C67

I'll see if I can add an extra step after PDL is built in future releases

@shawnlaffan
Copy link
Contributor

I think this issue is closable and will do so in a few days, or when next I do a sweep of the issues. If there are reasons to keep it open then please add a comment.

@sisyphus
Copy link

I still don't see any mention of a 32-bit 5.38.2 release at https://strawberryperl.com/releases.html .
(Admittedly, I don't know whether that's a reason to keep this issue open.)

@shawnlaffan
Copy link
Contributor

It does seem to be missing from the releases.json file. In that case we'll keep this open.

@jsoref
Copy link

jsoref commented Aug 16, 2024

@genio asked for 👍 / 👎 about #177 (comment)

  1. The lack of a loading progress indicator made the page seem broken -- I was staring for a while and it didn't occur to me that I should wait longer -- the fix is to add a spinner (in each column) until the table is ready.
    image
  2. The page doesn't work for people who don't use JavaScript (that's actually me in my default browser configuration) -- we're a tiny minority, so don't feel obligated to cater to us -- a simple improvement would be to have a that shows Sorry, these columns require JavaScript. Note that the index page has a block like this:
image
  1. You might consider changing the formatting of the line below the gray header rows and the "No Releases" cells. It isn't obvious to me what they are -- thinking about it for a while, I'm guessing that that first row is a description of the column category -- it clearly isn't a thing I can download, but since it's left aligned and kinda in the same font as the things below it... -- I'd be inclined to horizontally center No Releases to make it even more distinguished from the rows that would otherwise appear in its place.
    image
  2. There should be some text above the four tabs that explains what this page is -- I understand the url won't be /foo.html eventually, but you can't rely on people seeing page titles as they're probably in tiny tabs. I'd suggest both a header and a paragraph...
    image

@shawnlaffan
Copy link
Contributor

It does seem to be missing from the releases.json file. In that case we'll keep this open.

PR at StrawberryPerl/strawberryperl.com#50

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants