-
Notifications
You must be signed in to change notification settings - Fork 561
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
Drop the 5 (implement PPC0025) #22857
base: blead
Are you sure you want to change the base?
Conversation
Debugger needs an update too:
which looks like a simple fix. (trying to track down the rogue |
The
|
I definitely need to fix all those comparisons. |
I wonder how many more are on CPAN. |
Over 300 distributions contain files that match Once we get the branch to pass the test suite, it would be great to test a bunch of CPAN modules against it. |
@@ -1645,7 +1645,7 @@ CORE::evalbytes ''; | |||
#### | |||
# feature features when feature has been disabled by use VERSION | |||
# CONTEXT no warnings 'deprecated'; | |||
use feature (sprintf(":%vd", $^V)); | |||
use feature (sprintf(":%s", $^V =~ /^v(5\.\d+|\d+)/g)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This shouldn't be needed. feature.pm is meant to accept the full version as a bundle, and will shorten it as needed. If it isn't doing that correctly, it should be fixed.
@@ -86,7 +86,7 @@ print "# $@\nnot " if $@; | |||
print "ok ",$i++," - require v5 ignores sub named v5\n"; | |||
|
|||
eval { require 10.0.2; }; | |||
print "# $@\nnot " unless $@ =~ /^\QPerl v10.0.2 required\E/; | |||
print "# $@\nnot " if $@; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change doesn't seem correct. This is meant to check the error from requiring a version number that is too high.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You meant this changed the intended meaning of the test? I agree.
I'm not sure if the best solution is to simply delete those tests (there are other tests that do that), or come up with bigger numbers.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The easier answer is to massively increase the number to one we'll never hit. Otherwise, calculate a future version and use a string eval to run the require statement.
This commit drop PERL_REVISION from $] and $^V. Since $^V expects 3 components, we add a .0 at the end.
The FEATURE_BUNDLE_xxx constants in feature.h must be ordered by version. The 'as_bundles' sort function ensures that all bundles will be sorted correctly (except for 5.9.5, which ends up between 5.41 and 41).
The fix follows Zefram's suggestion from https://www.nntp.perl.org/group/perl.perl5.porters/2012/05/msg186846.html > On older perls, however, $] had a numeric value that was built up using > floating-point arithmetic, such as 5+0.006+0.000002. This would not > necessarily match the conversion of the complete value from string form > [perl #72210]. You can work around that by explicitly stringifying > $] (which produces a correct string) and having *that* numify (to a > correctly-converted floating point value) for comparison. I cultivate > the habit of always stringifying $] to work around this, regardless of > the threshold where the bug was fixed. So I'd write > > use if "$]" >= 5.014, warnings => "non_unicode";
4849b36
to
6061f4f
Compare
I built the psc/ppc0025 branch, unthreaded, on linux at 6061f4f and observed the following build-time warnings.
The
|
Thanks! I'll rework the patch to remove those variables. |
use-ing these versions will load the :5.41 bundle, since that's the highest one before them. This also means that we don't need to give hints about why 'use 5.6' or 'use 5.10' fail, because they won't anymore.
I've built the psc/ppc0025 branch on Windows 11, using a mingw-w64 port of gcc-14.2.0:
My main issue is that
For context, here's the relevant portion of win32/GNUmakefile, beginning at line 1778:
I don't really know what's going on here, but there's a lot of hard-coded configuration specs in win32/config.gc, win32/config_H.gc and win32/config_sh.PL - and I wonder whether there's some stuff in one (or more) of those files that requires editing.
but I don't know about the correctness of this (which is hard coded into win32/config_sh.PL) : UPDATE: I've just done a build on Ubuntu-24.04 for reference, and I can see that UPDATE 2: Amending config_sh.PL corrects the output of
Here is full
|
Thank you!
I'm currently working on getting the last failing tests to pass for me. |
Same thing happens on Ubuntu (with the same error as on Windows). On Windows, I've hacked win32/config.gc such that I'm wondering whether patchlevel.h (which is referenced by config.sh) requires some amendment ?? UPDATE:
I think the assignment to |
A couple of observations/questions:
The patches that I used in order to be able to install perl:
|
It's annoying, but I think it's coming from
I suppose we would drop the 5 there too, and have |
Yes, presumably that third bit is something like
It would be a whole lot easier to leave the "5" in there. Another concern is the number of cpan modules (especially the unmaintained ones) that rely on I'm not trying to be negative, and I'll deal with whatever gets thrown at me, but I really don't want to witness another |
It's only less than the version in string comparisons. 41.7 is definitely larger than
As documented in PPC0025, we have basically three options:
If we can't "drop the 5", for the reasons you've stated above, we're effectively stuck with 5 forever.
This is maintained software, so we can hopefully expect that when they build Strawberry Perl 42 for Windows, they will make the necessary adaptations.
This is the reason for this branch. Have some actual code we can experiment with, hopefully finding out if the plan is workable or if there is such a roadblock that force us to remain Perl 5 forever.
Surely we can come up with a naming scheme for those. Don't forget that the perldeltas for dev releases are eventually deleted and merged in the perldelta for the stable release. These would be
This is indeed the goal of this branch: assess the size of the problem, and decide if this is going to block the proposal.
"Fiasco" might be a strong word for "we changed our mind in the middle of the development track". |
What happens to path #4 of a version object, the |
|
This proposal does not change how version objects work - and underscores do not create an additional segment (in recent versions of version), but may have an arbitrary number of segments, which must each be under 999 to convert properly to a decimal form. |
I mean that, as I keep building this branch of blead over the coming months, the version will progress from 41.7 to 41.8 to 41.9 and then to 41.10.
I agree that this is the way to proceed. |
As a number, the version is 41.010, which is numerically greater than 41.009. As a dotted decimal, 41.10.0 is greater than 41.7.0. It is a risk that people will specify the version incorrectly in code if we suggest using only two segments, but I'm not sure if that's what you're referring to. |
I'm talking about Under the current system it's "5.41.7", which is clearly not a valid number. This new approach alters $Config{version} to "41.7" which looks like a number, but if one expects "41.10" to be greater than than "41.7", one will probably be disappointed. (And perl won't warn about the folly.) There's a trap there that's easily avoided by having $Config{version} instead set to (eg) "41.007". |
Important fixes include: * sorting numerical versions using <=> * properly computing the family for versions larger than 5
@@ -3255,7 +3255,10 @@ PPD_PERLVERS | |||
# archname did not change from 5.6 to 5.8, but those versions may | |||
# not be not binary compatible so now we append the part of the | |||
# version that changes when binary compatibility may change | |||
if ("$]" >= 5.008) { | |||
if ("$]" >= 41) { | |||
$archname .= "-$Config{api_version}"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
According to @haarg, this should be $Config{api_versionstring}
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And the test should be:
if ("$]" >= 5.041) {
This is a draft MR, to test how things would actually work.
Many of the remaining failing tests are dual-life modules (which perform a string comparison againsg
$]
), or misplaced expectations (use v6
should now work, and enable the:5.41
feature and builtin bundles).