Skip Menu |

This queue is for tickets about the ExtUtils-MakeMaker CPAN distribution.

Report information
The Basics
Id: 121258
Status: rejected
Priority: 0/
Queue: ExtUtils-MakeMaker

People
Owner: Nobody in particular
Requestors: djerius [...] cpan.org
Cc: DBOOK [...] cpan.org
KENTNL [...] cpan.org
AdminCc:

Bug Information
Severity: (no value)
Broken in: 7.24
Fixed in: (no value)



Subject: '.' is injected into @INC uring EU:MM test phase
I'm using the ExtUtils::MakeMaker bundled with 5.25.11. I've compiled 5.25.11 with the -Ddefault_inc_excludes_dot flag to ensure that it excludes '.' from inc. % perl -V:config_args config_args='-Dprefix=/home/dj/.plenv/versions/5.25.11-no-dot -de -Dusedevel -Ddefault_inc_excludes_dot -A'eval:scriptdir=/home/dj/.plenv/versions/5.25.11-no-dot/bin''; Here's the default @INC from perl -V: @INC: /home/dj/.plenv/versions/5.25.11-no-dot/lib/perl5/site_perl/5.25.11/x86_64-linux /home/dj/.plenv/versions/5.25.11-no-dot/lib/perl5/site_perl/5.25.11 /home/dj/.plenv/versions/5.25.11-no-dot/lib/perl5/5.25.11/x86_64-linux /home/dj/.plenv/versions/5.25.11-no-dot/lib/perl5/5.25.11 I expected the lack of '.' to persist throughout the Perl toolchain, but it seems that '.' is added to @INC during the ExtUtils::MakeMaker test phase, which allowed tests to pass which should have failed. I've attached a very simply distribution whose test exhibits this behavior. The test simply dumps @INC, and results in: % make test PERL_DL_NONLAZY=1 "/data/pelf1/dj/hd0/root/dot/plenv/versions/5.25.11-no-dot/bin/perl5.25.11" "-MExtUtils::Command::MM" "-MTest::Harness" "-e" "undef *Test::Harness::Switches; test_harness(0, 'blib/lib', 'blib/arch')" t/*.t t/test.t .. # [ # '/data/macabretmp/dj/test-inc/blib/lib', # '/data/macabretmp/dj/test-inc/blib/arch', # '/home/dj/.plenv/versions/5.25.11-no-dot/lib/perl5/site_perl/5.25.11/x86_64-linux', # '/home/dj/.plenv/versions/5.25.11-no-dot/lib/perl5/site_perl/5.25.11', # '/home/dj/.plenv/versions/5.25.11-no-dot/lib/perl5/5.25.11/x86_64-linux', # '/home/dj/.plenv/versions/5.25.11-no-dot/lib/perl5/5.25.11', # '.' # ] Notice the trailing '.'. If I set PERL_USE_UNSAFE_INC=0, I get the expected results: % PERL_USE_UNSAFE_INC=0 make test PERL_DL_NONLAZY=1 "/data/pelf1/dj/hd0/root/dot/plenv/versions/5.25.11-no-dot/bin/perl5.25.11" "-MExtUtils::Command::MM" "-MTest::Harness" "-e" "undef *Test::Harness::Switches; test_harness(0, 'blib/lib', 'blib/arch')" t/*.t t/test.t .. # [ # '/data/macabretmp/dj/test-inc/blib/lib', # '/data/macabretmp/dj/test-inc/blib/arch', # '/home/dj/.plenv/versions/5.25.11-no-dot/lib/perl5/site_perl/5.25.11/x86_64-linux', # '/home/dj/.plenv/versions/5.25.11-no-dot/lib/perl5/site_perl/5.25.11', # '/home/dj/.plenv/versions/5.25.11-no-dot/lib/perl5/5.25.11/x86_64-linux', # '/home/dj/.plenv/versions/5.25.11-no-dot/lib/perl5/5.25.11' # ] I find this behavior confusing. If a '.' in @INC is evil, shouldn't the entire toolchain shun it? It may be the case that this is fixed in version not bundled with Perl 5.25.11, but perlbug directed me here. Thanks! Diab
Subject: Foo-Bar-0.01.tar.gz
Download Foo-Bar-0.01.tar.gz
application/gzip 1k

Message body not shown because it is not plain text.

On Thu Apr 20 15:47:05 2017, DJERIUS wrote: Show quoted text
> I find this behavior confusing. If a '.' in @INC is evil, shouldn't > the entire toolchain shun it?
A '.' in @INC is a security risk *if* you don't control which directory your code is being executed from. For tests, where the code is being executed from the root of an unpacked distro tarball, and executing it from any other path is unsupported and unlikely to work, there is no risk at all. As such, your assertion that such tests "should" fail is inaccurate - there's absolutely nothing to be gained by forcing them to fail, and lots of extra pain for downstream packagers and users if we did that, since there's not actually anything wrong or risky involved. As such, this is 100% expected, was done intentionally to avoid taking a steaming dump on our userbase, and is not a bug. Sorry for any confusion.
Subject: Re: [rt.cpan.org #121258] '.' is injected into @INC uring EU:MM test phase
Date: Thu, 20 Apr 2017 17:38:01 -0400
To: bug-ExtUtils-MakeMaker [...] rt.cpan.org
From: Diab Jerius <djerius [...] cpan.org>
On Thu, Apr 20, 2017 at 4:45 PM, Matt S Trout via RT < bug-ExtUtils-MakeMaker@rt.cpan.org> wrote: Show quoted text
> <URL: https://rt.cpan.org/Ticket/Display.html?id=121258 > > > On Thu Apr 20 15:47:05 2017, DJERIUS wrote:
> > I find this behavior confusing. If a '.' in @INC is evil, shouldn't > > the entire toolchain shun it?
> > A '.' in @INC is a security risk *if* you don't control which directory > your code is being executed from. > > For tests, where the code is being executed from the root of an unpacked > distro tarball, and executing it from any other path is unsupported and > unlikely to work, there is no risk at all. > > As such, your assertion that such tests "should" fail is inaccurate - > there's absolutely nothing to be gained by forcing them to fail, and lots > of extra pain for downstream packagers and users if we did that, since > there's not actually anything wrong or risky involved. > > As such, this is 100% expected, was done intentionally to avoid taking a > steaming dump on our userbase, and is not a bug. > > Sorry for any confusion. >
​I'm getting reports that one of my modules is failing tests because the tester sets​ PERL_USE_UNSAFE_INC=0 ​, and thus the the '.' added by ExtUtils::MakeMaker is being removed. That flag breaks ExtUtils::MakeMaker behavior. What's the proper response to that tester? Stop doing that and it won't hurt any more? Ignore them? PERL_USE_UNSAFE_INC=0​ seems to be the go to method for testing things out, yet it changes the behavior of a core module. I'm a part of that userbase you're worried about, and I feel some of that steaming dump is being slung my way.
On 2017-04-20 14:39:08, DJERIUS wrote: Show quoted text
> ​I'm getting reports that one of my modules is failing tests because the > tester sets​ > PERL_USE_UNSAFE_INC=0 > ​, and thus the the '.' added by ExtUtils::MakeMaker is being removed. > > That flag breaks ExtUtils::MakeMaker behavior. What's the proper response > to that tester? Stop doing that and it won't hurt any more? Ignore them? > PERL_USE_UNSAFE_INC=0​ seems to be the go to method for testing things out, > yet it changes the behavior of a core module. I'm a part of that userbase > you're worried about, and I feel some of that steaming dump is being slung > my way.
It's fair for a tester to be setting that variable -- it's checking what *would* happen if we removed the workaround in EUMM and Test::Harness to add . to @INC (which we might do in a few years -- I think the current plan was to do this for the 5.30 release?) For an author, the proper response is to fix the code -- either add '.' to @INC, or to restructure the code so it doesn't need . in @INC anymore. e.g. something like: use lib '.'; or (for example): mkdir t/lib mv t/Foo.pm t/lib/Foo.pm perl -p -i -e"s/package t::Foo/package Foo/" t/lib/Foo.pm perl -p -i -e"s/use t::Foo/use lib 't';\nuse Foo/' t/*
Subject: Re: [rt.cpan.org #121258] '.' is injected into @INC uring EU:MM test phase
Date: Fri, 21 Apr 2017 10:15:55 -0400
To: bug-ExtUtils-MakeMaker [...] rt.cpan.org
From: Diab Jerius <djerius [...] cpan.org>
On Thu, Apr 20, 2017 at 8:36 PM, Karen Etheridge via RT < bug-ExtUtils-MakeMaker@rt.cpan.org> wrote: Show quoted text
> <URL: https://rt.cpan.org/Ticket/Display.html?id=121258 > > > On 2017-04-20 14:39:08, DJERIUS wrote: >
> > ​I'm getting reports that one of my modules is failing tests because the > > tester sets​ > > PERL_USE_UNSAFE_INC=0 > > ​, and thus the the '.' added by ExtUtils::MakeMaker is being removed. > > > > That flag breaks ExtUtils::MakeMaker behavior. What's the proper
> response
> > to that tester? Stop doing that and it won't hurt any more? Ignore
> them?
> > PERL_USE_UNSAFE_INC=0​ seems to be the go to method for testing things
> out,
> > yet it changes the behavior of a core module. I'm a part of that userbase > > you're worried about, and I feel some of that steaming dump is being
> slung
> > my way.
> > It's fair for a tester to be setting that variable -- it's checking what > *would* happen if we removed the workaround in EUMM and Test::Harness to > add . to @INC (which we might do in a few years -- I think the current plan > was to do this for the 5.30 release?) > > For an author, the proper response is to fix the code -- either add '.' to > @INC, or to restructure the code so it doesn't need . in @INC anymore. > > e.g. something like: > > use lib '.'; > > or (for example): > > mkdir t/lib > mv t/Foo.pm t/lib/Foo.pm > perl -p -i -e"s/package t::Foo/package Foo/" t/lib/Foo.pm > perl -p -i -e"s/use t::Foo/use lib 't';\nuse Foo/' t/* >
​ I rarely rant in public, but I'm going to make an exception here. I'm rather frustrated with the situation and the tone of the response that I'm getting. Let me lay this out in another fashion. I'm a CPAN author. I download the latest development version of Perl, make sure I'm in a clean environment, fix the code so that it works. Now I'm informed, not by p5p, nor anyone working on the toolchain, but via a bug report from a tester, that there's a special environment variable that now has to be set to pass CPAN tests. A non-standard, poorly documented variable at that. There's nothing in the release notes which indicates that PERL_USE_UNSAFE_INC=0 is special, only that setting it to 1 has a particular effect. Any reasonable person would assume that setting it to 0 would be the same as not setting it, but that's obviously not the case. It may be fair for the testers to set that variable, but it's blatantly *unfair* to CPAN authors not to inform them of this new requirement. There has been no communications from p5p or the toolchain maintainers to CPAN authors. You do have our email addresses! A simple email would suffice to clear everything up. We aren't privy to what's going on in the innards. Give developers clear guidance on what to do. Make it public! Don't make us read through the code or pore through release notes for umpteen things to figure out why special environment variables work or don't work. And please, don't tell me what the "proper" response as a developer is. That's patronizing and offensive. I wouldn't have started this conversation if I didn't already take my ​responsibility seriously. Diab

On 2017-04-22 02:16:56, DJERIUS wrote:
>
> I rarely rant in public, but I'm going to make an exception
> here. I'm rather frustrated with the situation and the tone of
> the response that I'm getting.
>
> Let me lay this out in another fashion.
>
> I'm a CPAN author.
>
> I download the latest development version of Perl, make sure I'm
> in a clean environment, fix the code so that it works.
>
> Now I'm informed, not by p5p, nor anyone working on the
> toolchain, but via a bug report from a tester, that there's a
> special environment variable that now has to be set to pass CPAN
> tests.


Hi. It may have been me, or one of the other 4 people I saw filing bugs, that reported this, so if it was me, I apologize for lack of clarity.

But it should stand that the reason for filing that bug was that I was taking this problem seriously, and taking it upon myself to at least inform authors in some manner that a future Perl release will, in some future, break your code in some way.

Thus, maybe it wasn't clear, but I am intending to work in some capacity as a toolchain worker :)

Sadly, there is just too many of these problems, >20% of CPAN is broken under this condition.

I've practically spent every free minute of my life for the last 2 months ( and that is far from an exaggeration ) simply hunting down and reporting distributions that are affected, in the hope that there will be at  least *some* indicator of this problem before it starts affecting people.

So believe me when I say "This is an unprecedented situation"

This is incredibly problematic and difficult, and the change, though long planned, was activated and deployed very last minute, and there's subsequently a mad rush to mitigate the fallout.

> A non-standard, poorly documented variable at that. There's
> nothing in the release notes which indicates that
> PERL_USE_UNSAFE_INC=0 is special, only that setting it to 1 has a
> particular effect. Any reasonable person would assume that
> setting it to 0 would be the same as not setting it, but that's
> obviously not the case.

Indeed, and agreed. And such documentation is planned ... but I still can't find it anywhere. Rumour has it it might land in the 5.26 release notes.

But its only existed since 5.25.10!

But the point is *not* that end users should be setting it. 

The point is a significant and long relied upon mechanic in Perl has become broken By Design, and P5P have provided a *temporary hack* to counter-act the reality that lots of this code ( See aforementioned lack of precedent ) is broken by this change.

P5P could have opted never to produce said variable, and we'd just be in the same sticky mess we're in now, but it would be more dire,  because affected code would not only be broken for people using non-CPAN tooling like me, not just for maintainers and hand-installations, but for _everyone_ on CPAN.

And there are so many ADAMK dists  that would be broken, its just not realistic to have no-workaround :/

> It may be fair for the testers to set that variable, but it's
> blatantly *unfair* to CPAN authors not to inform them of this new
> requirement. There has been no communications from p5p or the
> toolchain maintainers to CPAN authors. You do have our email
> addresses!
>
> A simple email would suffice to clear everything up. We aren't
> privy to what's going on in the innards.
>
> Give developers clear guidance on what to do. Make it public!
>
> Don't make us read through the code or pore through release notes
> for umpteen things to figure out why special environment
> variables work or don't work.
>
> And please, don't tell me what the "proper" response as a
> developer is. That's patronizing and offensive. I wouldn't have
> started this conversation if I didn't already take my
> ​responsibility seriously.

In reality, there's so much confusion here, and I don't blame you really. I've seen ex-pumpkings and many other people who could be imagined they *should* know better mess this up.

In reality, even though this variable is created ( and instituted in tooling ) as a stop-gap to protect end users from catastrophe, its very existence and the mechanisms we use to protect users act as a direct disservice to CPAN authors, because its nature and prevalence is simply that *having it set* hides *real bugs*

Consequently, there's been a raft of "I reported this dist failed" and people applying fixes and releasing them ... only to find they didn't fix the issue, because their tools hid the failures from them. That's great for end users, but for maintainers, that's just giving them a false sense of security which will collapse in one way or another ( either immediately after installation, or 2 years from now )

I really wish we'd found an approach that made sense where only users were protected, but authors still got the horror shows of breakage ... but . Well, nobody managed to figure out how yet.

Yes, this is all horrible, and the nightmare is long from over for me :/

What you should do?

You want your code to work with that flag in either condition, you should assume it won't be there one day, and forcibly turn it off to simulate how it will eventually be [ or in some cases, will be outside `cpanm` and friends ]

Think of the flag as being called PERL_BE_524_COMPATIBLE=1

With the flag on in 5.25.11, you're seeing Perl how it behaves on perls before 5.25

With the flag off in 5.25.11, you're seeing how Perl is designed to work "now"

End users shouldn't care about the flag, because its not really an end users job to fix CPAN modules that no longer work on newer perls, but end users have that flag as a defence because CPAN is a huge unmaintained wasteland sometimes.

Again, sorry for the lack of clarity, everything here is awful

Additionally, hopefully the draft documentation here is useful, and if not, please ask for clarification/corrections, because I'd hate for our final documentation in 5.26 to also be useless.

http://www.nntp.perl.org/group/perl.perl5.porters/2017/04/msg243878.html

I do think this situation is significant enough that every CPAN author needs to know about it, but how do we achieve that?

We may have *your* email on file, I assume your @CPAN address works, but how do we deal with the large numbers of @CPAN addresses that now just end up bouncing?

There were numerous blogs about this on blogs.perl.org , they got referenced on reddit, its been a daily conversation on IRC. ( http://blogs.perl.org/users/ryan_voots/2017/04/trials-and-troubles-with-changing-inc.html )

What other channels are there to explore to ensure people who need to know know?

Would social networks help?

-- 
- CPAN kentnl@cpan.org
- Gentoo Perl Maintainer kentnl@gentoo.org ( perl@gentoo.org )
 

On 2017-04-21 07:16:56, DJERIUS wrote: Show quoted text
> there's a special environment variable that now has to be set to pass CPAN tests. > It may be fair for the testers to set that variable, but it's > blatantly *unfair* to CPAN authors not to inform them of this new > requirement...
I think you misunderstood. This variable is not one that is intended to be widely used. It is a back door to (temporarily!) circumvent stricter requirements that are coming in 5.26.0. This is the security issue that is being closed that is discussed at http://blogs.perl.org/users/ryan_voots/2017/04/trials-and-troubles-with-changing-inc.html (I see you saw that thread, as you replied to it). Neither authors are being expected to set this variable to install their modules. It is being set temporarily in the toolchain so that not all of CPAN breaks at once when the no-dot-in-INC change rolls out. The communication failure here is from testers (automated systems, so we must forgive them for being terse) to authors that there is a new failure mode that needs to be closed "soon", and that there is a temporary workaround in place which results in some PASS results still coming to you. We're sorry about that; we (developers and computers alike) are terrible at communicating.
Hi Diab, I can certainly understand being frustrated about changes. It seems like the explanations given here describe what is going on and the reasoning behind it. What appears to be the problem here is communication, so I'll take a quick stab at hopefully clearing that up. If your dist currently relies on '.' being in @INC, you'll need to ensure your dist has it there by altering your Makefile.PL like ether suggested. http://blogs.perl.org/users/ryan_voots/2017/04/trials-and-troubles-with-changing-inc.html That blog article explains a bit about the need to remove '.' from @INC in general. Having the '.' in @INC during the build/test phase isn't a security problem, as was described in a reply above. So, EUMM tries to be helpful and give you that so that your dist won't break if you only rely on that feature for installation. Testers can test with EUMM's helpful addition and without. Since the testers may test both ways (with EUMM injecting the '.' for you and without) they may find issues in distributions that don't yet handle it themselves. If you inject it yourself, they won't find an issue. That being said, the real fix for the change has to be done within each individual dist that's affected. Ether's reply was a suggestion on how many dists can be fixed. Apologies if the replies here seemed to be patronizing, but I really don't think that was anyone's intention. I think everyone here would be more than happy to help you find a way to update your dists to get around the changes. Maybe a few minutes with us in IRC would help as the conversation could go a bit more rapidly? https://chat.mibbit.com/?channel=%23perl&server=irc.perl.org Again, apologies for the miscommunication as it certainly wasn't anyone's intent to be patronizing. Thanks, Chase