BBC: Blead Breaks Number::Format #20571
Comments
Bisecting with this invocation ... ... points to commit 04de022 @khwilliamson, can you take a look? |
The blamed commit, 04de022, exposed a bug in the module itself. I will submit a PR to fix it. But this ticket did tell me that there was a problem with that commit. It returned a C language value, CHAR_MAX, which doesn't really have a corresponding concept in Perl. Instead we use -1 to indicate that a positive-valued variable is in some abnormal state. This commit changes to do that, and documents the changes, which should have been done in 04de022.
The blamed commit, 04de022, exposed a bug in the module itself. I will submit a PR to fix it. But this ticket did tell me that there was a problem with that commit. It returned a C language value, CHAR_MAX, which doesn't really have a corresponding concept in Perl. Instead we use -1 to indicate that a positive-valued variable is in some abnormal state. This commit changes to do that, and documents the changes, which should have been done in 04de022.
The blamed commit, 04de022, exposed a bug in the module itself. I will submit a PR to fix it. But this ticket did tell me that there was a problem with that commit. It returned a C language value, CHAR_MAX, which doesn't really have a corresponding concept in Perl. Instead we use -1 to indicate that a positive-valued variable is in some abnormal state. This commit changes to do that, and documents the changes, which should have been done in 04de022.
The blamed commit, 04de022, exposed a bug in the module itself. I will submit a PR to fix it. But this ticket did tell me that there was a problem with that commit. It returned a C language value, CHAR_MAX, which doesn't really have a corresponding concept in Perl. Instead we use -1 to indicate that a positive-valued variable is in some abnormal state. This commit changes to do that, and documents the changes, which should have been done in 04de022.
This fixes Perl/perl5#20571 The commit blamed in that tickect caused POSIX::localeconv() to return all the defined fields in the lconv structure. Number::Format had been relying on it to suppress empty fields. The solution is simply for Number::Format to instead ignore empty fields itself. This is entirely backwards compatible, as previously those fields were never delivered to Number::Format. There is a slight complication in that the POSIX standard allows for the monetary decimal point and the monetary thousands separator to both be empty. There is a check in Number::Format against both being empty that must be relaxed to allow this legal combination. The check previously was effectively useless as Number::Format would never see those fields if they were empty, as POSIX::localeconv() would suppress them.
|
I have submitted billward/number-format-perl#13 to get Number::Format to work with perls that have this commit, as well as earlier ones |
I can confirm that that pull request will, when applied, address the breakage caused by 04de022 reported in this Issue. I'm going to label this ticket both "sendToCPAN" and "Stalled". Number-Format has not had a new CPAN release since 2015. In the period 2015-17 there was some work committed to its GH repository, but that work has never been released to CPAN. Even that unreleased work has "BBC" errors, but those are simply due to upgrades in CPAN modules that ship with core. It will be the responsibility of the Number-Format maintainers to address those problems if and when they prepare a new CPAN release. In the meantime, Number-Format will unfortunately continue to generate CPANtesters FAILures, but we've done as much as we can do about this. |
|
Andreas is there any procedure we can use to resolve matters like this? If
perl core devs prep BBC patches to cpan modules is there a way we can do
"bbc" fixes without having to take over formal ownership? I realize it is
sensitive ground, but I fear this is going to become more and more common.
It seems like it would be better to release a fix than leave the module
broken for ever...
Yves
…On Fri, 3 Feb 2023, 05:47 James E Keenan, ***@***.***> wrote:
I have submitted billward/number-format-perl#13
<billward/number-format-perl#13> to get
Number::Format to work with perls that have this commit, as well as earlier
ones
I can confirm that that pull request will, when applied, address the
breakage caused by 04de022
<04de022>
reported in this Issue.
I'm going to label this ticket both "sendToCPAN" and "Stalled".
Number-Format has not had a new CPAN release since 2015. In the period
2015-17 there was some work committed to its GH repository, but that work
has never been released to CPAN. Even that unreleased work has "BBC"
errors, but those are simply due to upgrades in CPAN modules that ship with
core. It will be the responsibility of the Number-Format maintainers to
address those problems if and when they prepare a new CPAN release. In the
meantime, Number-Format will unfortunately continue to generate CPANtesters
FAILures, but we've done as much as we can do about this.
—
Reply to this email directly, view it on GitHub
<#20571 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAAZ5R7FOMO42KYVHRYQJ4DWVQTN7ANCNFSM6AAAAAASRQD3JE>
.
You are receiving this because you are subscribed to this thread.Message
ID: ***@***.***>
|
|
No, that's why the process to take ownership exists. |
|
That is a circular argument. "You can't fix this without taking ownership because we have an ownership process". If nobody is willing to take ownership then it doesn't get fixed? Isn't that a recipe for bitrot and doesn't it just undermine the perception and reputation of Perl and CPAN in longer term? Is that the best thing for the community in general? So again, I ask @andk, (not you @Grinnz ) is there anything we can do about this? I wouldn't mind putting in the work to fix some of these things on CPAN if it didn't mean an extended responsibility for maintenance than ownership implies. |
|
That is not what I suggested; the process to take ownership exists because we cannot do anything else about it. The namespace belongs to those with permissions to the namespace, intentionally. |
|
Relevant reading: https://github.com/andk/pause/blob/master/doc/operating-model.md |
|
The best thing we can do is to create a distropref file that applies the patch(es), and convince people to use cpan clients that support distroprefs (cpanm does not, to my knowledge, which I feel is a shame). |
|
@Grinnz said: Relevant reading: https://github.com/andk/pause/blob/master/doc/operating-model.md I don't know if I agree with the certainty you guys are stating. To me this is pretty similar to the following case in 4.2. (Bold added by me):
My point is that a BBC patch that is peer reviewed on p5p (or via github in some way), which only attempts to "resurrect" a module to continue working on a new perl, does not change any interfaces, and attempts to minimize the changes to the least possible change seems is pretty close to the description above. If there were such a process I would say that the new release must include clear data on the specific changes made, the reason why, and the list of peer reviewers who validated that the the change was as conservative as possible and strictly limited to ensuring interoperability and compatible with modern perl releases. I realize that "making it not die on a new perl" is not /quite/ the same thing as "follow modern CPAN/packaging conventions", nor is it /quite/ the same thing as "address a security issue", but it is not far off them either, especially the "follow modern ... conventions" part. Also note that I specifically asked if there was a way we could set up a process where this would occur without changing indexing permissions, eg as a one-off to get the show on the road again. As a BBC responder I would not want to have to take co-maint to get the show on the road, but I would be willing to write the minimal patch that would get released to keep things working. There is a big difference between "while you were away we made minor changes to your module so it did not fail on the latest perl" and "while you were away we removed your ownership so we could make minor changes to your module so it did not fail on the latest perl". One I would see as lending assistance, the other I would see as personal invasion. It seems to me that @andk made the rules, and he can change them, and I don't think anything I have proposed here violates the spirit or intent of the principals behind CPAN. I think it represents a natural extension of rules already stated. You guys might disagree (although that is not entirely clear, you seem to be interpreting the existing rules more than stating a personal opinion about what you think the rules should be), and @andk certainly should take whatever your opinion is under advisement, but ultimately it seems to be that it his call to make (while taking interested parties viewpoints into account), and we should let him do his job as he sees appropriate. IMO rules can and should change as circumstances need, and indeed the doc at https://github.com/andk/pause/blob/master/doc/operating-model.md is on version 2 so it has changed at least once in the past. Also id like to point out this line from the document:
I dont think anything I am saying in terms of this proposal violates that spirit. If I was long absent and when I returned I found out that p5p devs had patched my code to work on Perl 5.54 I would be happy that my code was still used and running after so much time away and I would be grateful that it was done. I think most developers (not every developer obviously) would similarly prefer that someone fixed their code while they were absent rather than have their module replaced because because nobody could fix it. I think most developers are like parents who would rather their code has a long life of happy usefulness instead of just being dead and broken, provided that their fundamental artistic perspectives and coding style etc, were respected in the process. Repeating myself again I think there is a big difference between a collective like p5p making an ultra-minimal peer reviewed fix which results in a one-time upload to PAUSE/CPAN, and having the ownership /transferred/ to someone else who then gets to apply their own artistic license to the resulting product. For instance if I took ownership of a module one of the first things I would likely do to it would be to make sure it runs under strict and warnings, and I would pertidy it. However I would do none of those things if I was performing a "BBC continuity fix", in such a case I would be ultra-careful to follow to the best my ability and discernment the intent, style and preferences of the true owner, with the hope that when they return they would say "I can't even tell what changes you made without checking the doc/patch/changelog to see what they were". |
This is a policy discussion which is way beyond the scope of a single BBC issue. Please move this discussion to the perl5-porters mailing list. |
|
I have contacted @billward and the PAUSE admins. |
The blamed commit, 04de022, exposed a bug in the module itself. I will submit a PR to fix it. But this ticket did tell me that there was a problem with that commit. It returned a C language value, CHAR_MAX, which doesn't really have a corresponding concept in Perl. Instead we use -1 to indicate that a positive-valued variable is in some abnormal state. This commit changes to do that, and documents the changes, which should have been done in 04de022.
The blamed commit, 04de022, exposed a bug in the module itself. I will submit a PR to fix it. But this ticket did tell me that there was a problem with that commit. It returned a C language value, CHAR_MAX, which doesn't really have a corresponding concept in Perl. Instead we use -1 to indicate that a positive-valued variable is in some abnormal state. This commit changes to do that, and documents the changes, which should have been done in 04de022.
|
I have made a new release, v1.76, with a fix that should work on both newer and older perls. |
The blamed commit, 04de022, exposed a bug in the module itself. I will submit a PR to fix it. But this ticket did tell me that there was a problem with that commit. It returned a C language value, CHAR_MAX, which doesn't really have a corresponding concept in Perl. Instead we use -1 to indicate that a positive-valued variable is in some abnormal state. This commit changes to do that, and documents the changes, which should have been done in 04de022.
This is a bug report for perl from "Carlos Guevara" [email protected],
generated with the help of perlbug 1.43 running under perl 5.37.7.
BBC: Blead Breaks Number::Format
Please see http://fast-matrix.cpantesters.org/?dist=Number::Format
Flags
Perl configuration
The text was updated successfully, but these errors were encountered: