perl5 icon indicating copy to clipboard operation
perl5 copied to clipboard

BBC: Blead Breaks Sub::Exporter

Open cjg-cguevara opened this issue 1 year ago • 99 comments

This is a bug report for perl from "Carlos Guevara" [email protected], generated with the help of perlbug 1.43 running under perl 5.39.1.


BBC: Blead Breaks Sub::Exporter

Please see http://fast-matrix.cpantesters.org/?dist=Sub::Exporter


Flags

  • category=core
  • severity=low

Perl configuration

Site configuration information for perl 5.39.1:

Configured by cpan at Wed Jul 19 23:35:05 EDT 2023.

Summary of my perl5 (revision 5 version 39 subversion 1) configuration:
  Commit id: 923dde9b8ebe55ba73323ed675156712bc0062c9
  Platform:
    osname=linux
    osvers=5.15.120-0-lts
    archname=x86_64-linux-thread-multi
    uname='linux cjg-alpine3 5.15.120-0-lts #1-alpine smp wed, 05 jul 2023 18:59:57 +0000 x86_64 linux '
    config_args='-des -Dprefix=/home/cpan/bin/perl -Dscriptdir=/home/cpan/bin/perl/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.39.1:
    /home/cpan/bin/perl/lib/site_perl/5.39.1/x86_64-linux-thread-multi
    /home/cpan/bin/perl/lib/site_perl/5.39.1
    /home/cpan/bin/perl/lib/5.39.1/x86_64-linux-thread-multi
    /home/cpan/bin/perl/lib/5.39.1

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

cjg-cguevara avatar Jul 20 '23 04:07 cjg-cguevara

This is a bug in Sub::Exporter's test suite, and is fixed by https://github.com/rjbs/Sub-Exporter/pull/17

demerphq avatar Jul 20 '23 11:07 demerphq

Happy Birthday @rjbs.

demerphq avatar Jul 20 '23 11:07 demerphq

This is a bug in Sub::Exporter's test suite, and is fixed by rjbs/Sub-Exporter#17

Bisecting shows that this failure shows up in blead at this commit:

commit 2dcf3cf50d7cb67422ffe2c2a3e2d3dc404ea6c4 (HEAD, refs/bisect/bad)
Author:     Yves Orton <[email protected]>
AuthorDate: Mon Feb 14 12:24:21 2022 +0100
Commit:     Yves Orton <[email protected]>
CommitDate: Tue Jul 18 20:26:14 2023 +0200

    Fix assorted bugs related to not having a UNIVERSAL::import

... which is consistent with @demerphq's explanation. I suspect that since this commit went into blead just two days ago, we'll see other CPAN breakage connected to that commit and have to file tickets for other distros upstream.

jkeenan avatar Jul 20 '23 13:07 jkeenan

I suspect that since this commit went into blead just two days ago, we'll see other CPAN breakage connected to that commit and have to file tickets for other distros upstream.

Yep. Very very likely. This patch is specifically intended to detect people doing stuff that wouldn't be detected prior to it.

demerphq avatar Jul 20 '23 14:07 demerphq

Hey, thanks! I'm out of town on business, but this all looks like Yves is probably right, and I'll dig into it pretty soon.

rjbs avatar Jul 20 '23 23:07 rjbs

Also affected: LEONT/CPAN-Meta-Check-0.017.tar.gz @Leont please take note

andk avatar Jul 21 '23 08:07 andk

Re CPAN::Meta::Check:

------------------------------
PROGRAM OUTPUT
------------------------------

Output from '/usr/bin/make test':

PERL_DL_NONLAZY=1 "/tmp/basesmoker-reloperl-uCLf/bin/perl" "-MExtUtils::Command::MM" "-MTest::Harness" "-e" "undef *Test::Harness::Switches; test_harness(0, 'blib/lib', 'blib/arch')" t/*.t
Attempt to call undefined import method with arguments via package "CPAN::Meta::Prereqs" (Perhaps you forgot to load the package?) at /tmp/loop_over_bdir-101935-CrkcWo/CPAN-Meta-Check-0.017-0/blib/lib/CPAN/Meta/Check.pm line 12.
BEGIN failed--compilation aborted at /tmp/loop_over_bdir-101935-CrkcWo/CPAN-Meta-Check-0.017-0/blib/lib/CPAN/Meta/Check.pm line 12.
Compilation failed in require at t/10-basics.t line 8.
BEGIN failed--compilation aborted at t/10-basics.t line 8.
t/10-basics.t ......... 
Dubious, test returned 255 (wstat 65280, 0xff00)
No subtests run 
Attempt to call undefined import method with arguments via package "CPAN::Meta::Prereqs" (Perhaps you forgot to load the package?) at /tmp/loop_over_bdir-101935-CrkcWo/CPAN-Meta-Check-0.017-0/blib/lib/CPAN/Meta/Check.pm line 12.
BEGIN failed--compilation aborted at /tmp/loop_over_bdir-101935-CrkcWo/CPAN-Meta-Check-0.017-0/blib/lib/CPAN/Meta/Check.pm line 12.
Compilation failed in require at t/20-undef-version.t line 6.
BEGIN failed--compilation aborted at t/20-undef-version.t line 6.
t/20-undef-version.t .. 
Dubious, test returned 255 (wstat 65280, 0xff00)
No subtests run 

Test Summary Report
-------------------
t/10-basics.t       (Wstat: 65280 (exited 255) Tests: 0 Failed: 0)
  Non-zero exit status: 255
  Parse errors: No plan found in TAP output
t/20-undef-version.t (Wstat: 65280 (exited 255) Tests: 0 Failed: 0)
  Non-zero exit status: 255
  Parse errors: No plan found in TAP output
Files=2, Tests=0,  0 wallclock secs ( 0.01 usr  0.00 sys +  0.14 cusr  0.02 csys =  0.17 CPU)
Result: FAIL
Failed 2/2 test programs. 0/0 subtests failed.
make: *** [Makefile:873: test_dynamic] Error 255

demerphq avatar Jul 21 '23 09:07 demerphq

Re: CPAN::Meta::Check, the offending line of code is

use CPAN::Meta::Prereqs '2.132830';

I guess this is intended to be a version Check, but since it's not a number it gets treated as an argument to import. We could probably handle this case more gracefully in UNIVERSAL::import(), but I'm not sure if we should, it seems preferable to keep it very simple. But perhaps we could add a bespoke error for this case to make it easier to understand what is going on.

Again, given the highly experienced nature of the author of this code it seems likely we will discover quite a bit more code where people thought they were doing something that were actually no-ops in disguise.

demerphq avatar Jul 21 '23 09:07 demerphq

The impact of this breakage is massive. On both FreeBSD and Linux, I built perls from the v5.39.1 tag, attempted to install Task::CPAN::Reporter against them and generate CPANtesters reports from a list of 500 modules. Because Sub::Exporter is a prereq to Test::Reporter::Transport::Metabase, even though the modules were actually tested, no reports were sent and all were marked DISCARD.

We should consider reverting the breaking commit until we can better assess the impact of the change.

jkeenan avatar Jul 21 '23 13:07 jkeenan

We should consider reverting the breaking commit until we can better assess the impact of the change.

That sounds like an over-reaction to me personally. When @rjbs can roll a new release of Sub-Exporter things will go back to normal. Presumably if he wasn't on holiday this would be fixed already. We can wait a few days IMO.

Just force install Sub::Exporter for now. The only thing broken there is a test that doesn't quite do what he thought it did.

demerphq avatar Jul 21 '23 13:07 demerphq

Looks like Moose is also hit by this

#   Failed test '... got the right list of applicable methods for Foo'
#   at t/cmop/methods.t line 211.
#     Structures begin differing at:
#          $got->[28] = 'import'
#     $expected->[28] = 'isa'

#   Failed test '... got the right list of applicable methods for Bar'
#   at t/cmop/methods.t line 302.
#     Structures begin differing at:
#          $got->[28] = 'import'
#     $expected->[28] = 'isa'
# Looks like you failed 2 tests of 81.

Out of my to-install list, I see ~80 failures, but didn't dive into them thoroughly.

dur-randir avatar Jul 21 '23 14:07 dur-randir

@dur-randir it would be helpful if you could look into the different cases you are seeing and give us some kind of summary of the error modes you see.

The Moose one is interesting, it is enumerating the list of methods in a given class, and is now seeing an import method that didn't used to exist. This is interesting because it is really a very different type of error compared to the errors that are expected from this and possibly we can fix it without breaking the intent of this change.

Cases where people really are calling non-existent imports with arguments thinking they are doing something useful, like a version check or import, really should be errors, even if it is irritating that so many cases are to be found in key parts of the CPAN toolchain. Those can be managed for the time being in the internals where they are important.

So have a breakdown of the different causes of this would be very useful if possible.

demerphq avatar Jul 21 '23 14:07 demerphq

Also affected: LEONT/CPAN-Meta-Check-0.017.tar.gz @Leont please take note

Released a fixed version

Leont avatar Jul 21 '23 14:07 Leont

  • JSON::Any, Template::Toolkit, Perl::Critic, Catalyst::Stuff: Attempt to call undefined import method with arguments via package "..." (Perhaps you forgot to load the package?)

  • Data::Printer

#   at t/011-class.t line 275.
#          got: 'Foo  {
#     parents: Bar
#     public methods (11): bar (Bar), baz, borg, can (UNIVERSAL), DOES (UNIVERSAL), foo, import (UNIVERSAL), isa (UNIVERSAL), new, unimport (UNIVERSAL), VERSION (UNIVERSAL)
#     private methods (2): _moo (Bar), _other
#     internals: {
#         test   42
#     }
# }'
#     expected: 'Foo  {
#     parents: Bar
#     public methods (9): bar (Bar), baz, borg, can (UNIVERSAL), DOES (UNIVERSAL), foo, isa (UNIVERSAL), new, VERSION (UNIVERSAL)
#     private methods (2): _moo (Bar), _other
#     internals: {
#         test   42
#     }
# }'
  • TAP::Formatter::TeamCity - no idea what's going on, but it's bug queue is empty

dur-randir avatar Jul 21 '23 15:07 dur-randir

Also affected: XAVIER/Graphics-ColorNames-Mozilla-0.11.tar.gz Sample report: http://www.cpantesters.org/cpan/report/3f34e38e-27b2-11ee-888a-1af50765aee6

andk avatar Jul 21 '23 15:07 andk

Also affected: PMAKHOLM/Encode-IMAPUTF7-1.05.tar.gz Sample fail report: http://www.cpantesters.org/cpan/report/cc70611e-27b3-11ee-8776-32f80765aee6

andk avatar Jul 21 '23 15:07 andk

Graphics-ColorNames-Mozilla

This has broken code in its test logic. It is attempting to import a sub from a class.

Issue reported in: https://rt.cpan.org/Ticket/Display.html?id=149090

Encode-IMAPUTF7

This also appears to be broken test logic. It attempts to import subs, but it does not use Exporter or define its own import method. It inherits from Encode::Encoding which also appears to not use Exporter or provide an import method.

Issue report in: https://rt.cpan.org/Ticket/Display.html?id=149091

Author is asking for a new owner and has issues that are over a decade old.

demerphq avatar Jul 21 '23 16:07 demerphq

I will push a patch shortly which will make the new UNIVERSAL::import() method handle version numbers the same as Exporter would, and silently convert

Thing->import("1.234");

into

Thing->VERSION("1.234");

Which should cover some of the issues reported here.

Basically there are three classes of issues that the UNIVERSAL::import() patch has surfaced:

  1. places where people are doing Thing->import("1.234") and expecting Exporter style behavior. Or writing use Thing "1.234"; and expecting it to be the same as use Thing 1.234; (this case may be combined with case 2)
  2. places where people are doing use Thing qw(blah); when there is not import method defined, and the blah isnt being used in any way.
  3. Places where people are testing the methods that are expected to be populated in a package, and seeing the new import/unimport methods and getting confused.

Case 1 should be fixed by my latest patch in https://github.com/Perl/perl5/pull/21279. Case 2 is a legit problem with the code that we report is broken, and that code should simply be fixed. Case 3 is a bit of a grey zone. I see broadly two positions one could take: A) You could argue that we should be able to add special methods to UNIVERSAL so people should just ignore any methods which are defined in it. B) You could also argue that since we have not defined a UNIVERSAL::import/UNIVERSAL::unimport for a long time that we should continue to not define it and do some other trick to achieve the goals we are trying to achieve here. I lean towards the A) interpretation.

demerphq avatar Jul 21 '23 16:07 demerphq

TAP::Formatter::TeamCity also fails on 5.36, so it's a red herring.

dur-randir avatar Jul 21 '23 17:07 dur-randir

Fix for JSON-Any at karenetheridge/JSON-Any#3

haarg avatar Jul 21 '23 18:07 haarg

shlomif/perl-file-find-object#3

haarg avatar Jul 21 '23 18:07 haarg

Also affected: TEVERETT/Class-Prototyped-1.13.tar.gz Sample "unknown" report (fails during call of Build.PL): http://www.cpantesters.org/cpan/report/3fa170f8-2785-11ee-87ea-7996116755ad

andk avatar Jul 21 '23 18:07 andk

moose/Moose#183

haarg avatar Jul 21 '23 18:07 haarg

Patched Perl::Critic: https://github.com/Perl-Critic/Perl-Critic/pull/1037

demerphq avatar Jul 21 '23 18:07 demerphq

Also affected: IBB/Acme-Damn-0.08.tar.gz Sample fail report: http://www.cpantesters.org/cpan/report/b90db89a-27ec-11ee-bc75-9caf0865aee6

andk avatar Jul 21 '23 18:07 andk

TEVERETT/Class-Prototyped-1.13.tar.gz has

use Module::Build '0.24';

and Module::Build doesnt define an import(), and especially does not use Exporter. It also has a bug report from 8 years ago, so I assume it is unmaintained.

demerphq avatar Jul 21 '23 18:07 demerphq

Also affected: IBB/Acme-Damn-0.08.tar.gz

fixed with https://github.com/denormal/perl-Acme-Damn/pull/1

demerphq avatar Jul 21 '23 19:07 demerphq

garu/Data-Printer#180

haarg avatar Jul 21 '23 19:07 haarg

abw/Template2#309

haarg avatar Jul 21 '23 19:07 haarg

Also affected: STRANGE/Locale-CLDR-Locales-Gd-v0.34.1.tar.gz Sample fail report: http://www.cpantesters.org/cpan/report/7c58c174-27fb-11ee-b708-a3f90865aee6

andk avatar Jul 22 '23 01:07 andk