Perl / perl5

🐪 The Perl programming language
https://dev.perl.org/perl5/
Other
1.98k stars 559 forks source link

BBC: Blead Breaks Data::Clone and others #20782

Open cjg-cguevara opened 1 year ago

cjg-cguevara commented 1 year ago

This is a bug report for perl from "Carlos Guevara" carlos@carlosguevara.com, generated with the help of perlbug 1.43 running under perl 5.37.6.


BBC: Blead Breaks Data::Clone

Please see http://fast-matrix.cpantesters.org/?dist=Data::Clone


Flags

Configured by cpan at Wed Feb 8 22:16:32 EST 2023.

Summary of my perl5 (revision 5 version 37 subversion 9) configuration: Commit id: fe7859256d3a8e646ec90c7313a105c8d273db32 Platform: osname=linux osvers=5.15.90-0-lts archname=x86_64-linux-thread-multi uname='linux cjg-alpine3 5.15.90-0-lts #1-alpine smp wed, 25 jan 2023 08:18:30 +0000 x86_64 linux ' config_args='-des -Dprefix=~/bin/perl-blead -Dscriptdir=~/bin/perl-blead/bin -Dusedevel -Duse64bitall -Duseithreads' hint=recommended useposix=true d_sigaction=define useithreads=define usemultiplicity=define use64bitint=define use64bitall=define uselongdouble=undef usemymalloc=n default_inc_excludes_dot=define Compiler: cc='cc' ccflags ='-D_REENTRANT -D_GNU_SOURCE -D_GNU_SOURCE -fwrapv -fno-strict-aliasing -pipe -fstack-protector-strong -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64' optimize='-O2' cppflags='-D_REENTRANT -D_GNU_SOURCE -D_GNU_SOURCE -fwrapv -fno-strict-aliasing -pipe -fstack-protector-strong' ccversion='' gccversion='12.2.1 20220924' gccosandvers='' intsize=4 longsize=8 ptrsize=8 doublesize=8 byteorder=12345678 doublekind=3 d_longlong=define longlongsize=8 d_longdbl=define longdblsize=16 longdblkind=3 ivtype='long' ivsize=8 nvtype='double' nvsize=8 Off_t='off_t' lseeksize=8 alignbytes=8 prototype=define Linker and Libraries: ld='cc' ldflags =' -fstack-protector-strong -L/usr/local/lib' libpth=/usr/lib /usr/local/lib /lib libs=-lpthread -ldb -ldl -lm -lcrypt -lutil -lc perllibs=-lpthread -ldl -lm -lcrypt -lutil -lc libc=/usr/lib/libc.a so=so useshrplib=false libperl=libperl.a gnulibc_version='' Dynamic Linking: dlsrc=dl_dlopen.xs dlext=so d_dlsymun=undef ccdlflags='-Wl,-E' cccdlflags='-fPIC' lddlflags='-shared -O2 -L/usr/local/lib -fstack-protector-strong'


@INC for perl 5.37.9: /home/cpan/bin/perl-blead/lib/site_perl/5.37.9/x86_64-linux-thread-multi /home/cpan/bin/perl-blead/lib/site_perl/5.37.9 /home/cpan/bin/perl-blead/lib/5.37.9/x86_64-linux-thread-multi /home/cpan/bin/perl-blead/lib/5.37.9


Environment for perl 5.37.9: HOME=/home/cpan LANG=C.UTF-8 LANGUAGE (unset) LC_COLLATE=C LD_LIBRARY_PATH (unset) LOGDIR (unset) PATH=/home/cpan/bin/perl-blead/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin PERL_BADLANG (unset) SHELL=/bin/bash

andk commented 1 year ago

Bisect leads me to v5.37.8-34-g7e2d91e6d3:

commit 7e2d91e6d3a09e2ebb61242bb18ff95d30d9560d
Author: Tony Cook <tony@develop-help.com>
Date:   Wed Dec 7 16:17:45 2022 +1100

    toke.c: deprecation warning for ' as a package separator
andk commented 1 year ago

Also affected: SPROUT/DBM-Deep-2.0016.tar.gz

demerphq commented 1 year ago

Wouldn't surprise if everything from sprout uses ' as a package separator. He is/was fond of it and said as much a few times.

jkeenan commented 1 year ago

@tonycoz, the breaking commit was said to be first in a series supporting implementation of an RFC. Are we sufficiently committed to this series such that we can say, "CPAN authors you must adapt now"? (If I were one of the author/maintainers affected by this problem, I would only want to adapt my code once.)

demerphq commented 1 year ago

@jkeenan the next step is to remove support entirely.

jkeenan commented 1 year ago

@jkeenan the next step is to remove support entirely.

This module and DBM::Deep are both using use warnings FATAL => 'all'; in their test suites, e.g., https://metacpan.org/release/SPROUT/DBM-Deep-2.0016/source/t/01_basic.t#L2. How should we advise their maintainers to proceed?

demerphq commented 1 year ago

As far as I know sprout is Father Chrysotomos. Really he should know better than to use fatal warnings. Regardless we should push BBC fixes to remove use of the single apostrophe package separators. He has said in the past he likes them however.

jkeenan commented 1 year ago

Data::Clone uses inc::Module::Install in its Makefile.PL. That rules me out as someone to submit a pull request for that module :-(

demerphq commented 1 year ago

On Thu, 9 Feb 2023 at 14:20, James E Keenan @.***> wrote:

Data::Clone uses inc::Module::Install in its Makefile.PL. That rules me out as someone to submit a pull request for that module :-(

Patch pushed as https://github.com/gfx/p5-Data-Clone/pull/3

demerphq commented 1 year ago

Also affected: SPROUT/DBM-Deep-2.0016.tar.gz

patch pushed as https://github.com/robkinyon/dbm-deep/pull/22

tonycoz commented 1 year ago

@tonycoz, the breaking commit was said to be first in a series supporting implementation of an RFC. Are we sufficiently committed to this series such that we can say, "CPAN authors you must adapt now"? (If I were one of the author/maintainers affected by this problem, I would only want to adapt my code once.)

As @demerphq pointed out, only one adaption is needed: removing use of ' as a package name separator.

In this case I've simply implemented the RFC, unless the PSC decides otherwise the next (and last) in the series will be in 5.40, where this will likely produce an error rather than a warning (should it throw on cases where ' was accepted as part of a symbol, or just not allow the ' resulting in syntax error in most cases).

andk commented 1 year ago

Also affected: SPROUT/CSS-DOM-0.17.tar.gz

Report: http://www.cpantesters.org/cpan/report/4e7393c8-b240-11ed-8a6a-e4de36e23651

demerphq commented 1 year ago

There is a very good chance that ALL of the modules listed at https://metacpan.org/author/SPROUT are affected.

demerphq commented 1 year ago

I have created a patch for SPROUT/CSS-DOM-0.17 at https://rt.cpan.org/Ticket/Display.html?id=146661

andk commented 1 year ago

Also affected: JSWARTZ/Poet-0.16.tar.gz

Report: http://www.cpantesters.org/cpan/report/61f14778-b30e-11ed-ab62-20de1ff81187

(and a big thanks to @demerphq for dealing with the fallout)

demerphq commented 1 year ago

(and a big thanks to @demerphq for dealing with the fallout)

Thank you @andk, you are very welcome. I am still not sure what we should do about all these BBC tickets where we have fixes, but folks dont apply them (i fear this will be the case for SPROUT's modules). I have been thinking about whether I want to take ownership of some of them, and frankly I really don't. I don't mind fixing them to keep pace with perl core changes, but I don't want get into a "last person who touched the file owns it". I have worked like that before and it is very taxing on contributors; that i clean something up once shouldn't require a commitment to keep cleaning it up for ever more.

I would really like if you and the other PAUSE admins would consider my proposal from before, to allow a peer reviewed point release for continuity purposes for BBC related breakage. I am aware of multiple BBC related patches that I have made that are just languishing, and frankly it is a bit demoralizing to do work and then have it just sit there.

Fwiw, the section I think that kind of covers what i would like to see happen is this:

If you only want to update the distribution to follow modern CPAN / packaging conventions (e.g. to ensure it includes metadata files), or to address a security issue, then the PAUSE admins may consider a permissions request. Such modules will be considered on a case-by-case basis.

I realize "silence new warnings" is neither a security issue or packaging issue, but it feels like "build cleanly on a modern perl" is pretty close.

andk commented 1 year ago

may consider a permissions request

To consider a permissions request requires somebody to issue a permissions request.

demerphq commented 1 year ago

@andk are you saying the type of scenario I described is acceptable grounds to make a request for a one time release without taking ownership? I just want clarity on the rules here. Is "build cleanly on modern perl" reasonable grounds to make such a request? So far I have not done so because the rules seem to suggest i need to volunteer to take ownership, but I don't want to do that. If you are saying that it is reasonable grounds then I will start building up a list of formal requests.

demerphq commented 1 year ago

Fwiw, I am working on getting Poet sorted out right now.

demerphq commented 1 year ago

Patch for Carp::Assert pushed to https://github.com/schwern/Carp-Assert/pull/4

demerphq commented 1 year ago

BTW, Poet is transitive breakage due to Carp::Assert being broken, it itself is fine, it is just that its tests are sensitive and pick up the breakage from Carp::Assert. I suspect Carp::Assert is somewhat widely used, so lets hope we can get a fix released ASAP so we dont have more such transitive false-positives.

demerphq commented 1 year ago

I am going through SPROUT's (Father Chrysotomos / @cpansprout ) modules in river order.

As expected Sub-Delete is also broken by the apostrophe deprecation.

https://rt.cpan.org/Ticket/Display.html?id=146682

is the patch to fix it.

demerphq commented 1 year ago

SPROUT/HTML-DOM is also affected. Patch posted as https://rt.cpan.org/Ticket/Display.html?id=146683

Note there are bug reports that have not been responded to in that queue for two years.

demerphq commented 1 year ago

SPROUT/JE is also affected. Patch posted as https://rt.cpan.org/Ticket/Display.html?id=146685

andk commented 1 year ago

@andk are you saying the type of scenario I described is acceptable grounds to make a request for a one time release without taking ownership? I just want clarity on the rules here. Is "build cleanly on modern perl" reasonable grounds to make such a request? So far I have not done so because the rules seem to suggest i need to volunteer to take ownership, but I don't want to do that. If you are saying that it is reasonable grounds then I will start building up a list of formal requests.

@demerphq: I have not changed how CPAN works in my sentence above.

demerphq commented 1 year ago

@andk all i am asking you for is a clarification of what the rules are. If I take the time to build a release, say for example some of SPROUT's modules, but I do not want to take ownership to get the module updated, will I be turned down because I do not want to take ownership? The rules are not clear, and seem to suggest that I would be turned down because I do not wish to take ownership. So please can you clarify the rules?

FWIW, File::HomeDir::TIny is affected by this ticket. Patch at https://rt.cpan.org/Ticket/Display.html?id=146686

demerphq commented 1 year ago

SPROUT/constant-lexical is also affected, patch at https://rt.cpan.org/Ticket/Display.html?id=146692

andk commented 1 year ago

Also affected: NHORNE/Class-Simple-Cached-0.04.tar.gz NHORNE/Class-Simple-Readonly-Cached-0.08.tar.gz

Reports have been sent to cpantesters but are not yet available from there