Default perl builds to not include . in @INC (default_inc_excludes_dot) #15786
Comments
From @toddrCreated by @toddrIn light of recent events, including CVE-2016-1238, I propose changing There are multiple approaches to making this happen. The approach we This solved 95% of our issues. In discussions with #toolchain, they As I understand things, we must make a decision on this ticket, Perl Info |
From @jkeenanOn Sat, 31 Dec 2016 17:36:49 GMT, TODDR wrote:
Can you explain why you chose to do this in these locations? (I realize you probably have already done so in other threads or on IRC, but since we're consolidating the discussion here, I'd like your reasoning to be clear to people just coming upon this discussion.)
Can you describe what was in the 95% and what was in the other 5%?
Is that already being tracked in a BBC ticket?
Thanks for taking the initiative on this and for moving the discussion forward. -- |
|
The RT System itself - Status changed from 'new' to 'open' |
From @toddrOn Sat, 31 Dec 2016 15:36:53 -0800, jkeenan wrote:
So ultimately, the goal is to get your build environments to have . in 1. Makefile.PL requires . in @INC because it expects to be able to 2. While Test::Harness uses blib, it does not include . and among 3. There are modules that have multiple Makefile.PL files so simply 4. There are unit tests that call perl scripts that expect . There may be others I'm not thinking of right now but those will do. In essence you need a tool that can inject . into @INC for all of its child While all of the above problems could in theory be fixed on CPAN,
I did a quick scan of our CPAN RPM repo and I could actually only
No this is the only recorded location I am aware of. Other than that,
And thank you for all your work! Todd |
From @toddrCrypt-Twofish.patchFrom 9e864236afe097437e74ca9bf47384d362d93ade Mon Sep 17 00:00:00 2001
From: Todd Rinaldo <toddr@cpanel.net>
Date: Fri, 8 Jan 2016 01:24:39 -0600
Subject: [PATCH] Add PERL_USE_UNSAFE_INC support
---
modules/Crypt-Twofish/Crypt-Twofish/Makefile.PL | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/modules/Crypt-Twofish/Crypt-Twofish/Makefile.PL b/modules/Crypt-Twofish/Crypt-Twofish/Makefile.PL
index 0fc3b99..f6b954c 100644
--- a/modules/Crypt-Twofish/Crypt-Twofish/Makefile.PL
+++ b/modules/Crypt-Twofish/Crypt-Twofish/Makefile.PL
@@ -49,7 +49,7 @@ open(F, ">platform.h") || die "platform.h: $!\n";
print F $text;
close F;
-sub MY::postamble { "tables.h: tab/tables.pl\n\t\$(PERL) tab/tables.pl\n" }
+sub MY::postamble { "tables.h: tab/tables.pl\n\tPERL_USE_UNSAFE_INC=1 \$(PERL) tab/tables.pl\n" }
WriteMakefile(
NAME => 'Crypt::Twofish',
--
2.7.0
|
From @toddrFile-NFSLock.patchFrom dddeafacc38952f5b565152f4ffe30747fbeb106 Mon Sep 17 00:00:00 2001
From: Nicolas Rochelemagne <rochelemagne@cpanel.net>
Date: Fri, 8 Jan 2016 12:07:32 -0600
Subject: [PATCH] Add PERL_USE_UNSAFE_INC support
---
modules/File-NFSLock/File-NFSLock/Makefile.PL | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/modules/File-NFSLock/File-NFSLock/Makefile.PL b/modules/File-NFSLock/File-NFSLock/Makefile.PL
index abc3381..5d68926 100644
--- a/modules/File-NFSLock/File-NFSLock/Makefile.PL
+++ b/modules/File-NFSLock/File-NFSLock/Makefile.PL
@@ -28,9 +28,10 @@ package MY;
sub processPL {
my $self = shift;
my $block = $self->SUPER::processPL(@_);
- # "Version:" in spec needs to match
+ # "Version:" in spec needs to match
# "$VERSION" from VERSION_FROM
- $block =~ s%(spec.PL\s*)$%$1 \$\(VERSION_FROM\)%m;
+ $block =~ s[(\$\(PERLRUNINST\)\s+File-NFSLock.spec.PL\b)][PERL_USE_UNSAFE_INC=1 $1]m;
+ $block =~ s%(spec.PL\s*)$%1 \$\(VERSION_FROM\)%m;
$block;
}
--
2.7.0
|
From @toddrMail-SpamAssassin.patchFrom 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Todd Rinaldo <toddr@cpanel.net>
Date: Fri, 8 Jan 2016 01:46:15 -0600
Subject: [PATCH 08/13] Add PERL_USE_UNSAFE_INC support
---
modules/Mail-SpamAssassin/Mail-SpamAssassin/Makefile.PL | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/modules/Mail-SpamAssassin/Mail-SpamAssassin/Makefile.PL b/modules/Mail-SpamAssassin/Mail-SpamAssassin/Makefile.PL
index 33ca2af01..2142dfff0 100644
--- a/modules/Mail-SpamAssassin/Mail-SpamAssassin/Makefile.PL
+++ b/modules/Mail-SpamAssassin/Mail-SpamAssassin/Makefile.PL
@@ -1050,7 +1050,7 @@ QSPAMC_SRC = spamc/qmail-spamc.c spamc/utils.c
LIBSPAMC_SRC = spamc/libspamc.c spamc/utils.c
$(SPAMC_MAKEFILE): $(SPAMC_MAKEFILE).in $(SPAMC_MAKEFILE).win spamc/spamc.h.in
- $(CONFIGURE) --prefix="$(I_PREFIX)" --sysconfdir="$(I_CONFDIR)" --datadir="$(I_DATADIR)" --enable-ssl="$(ENABLE_SSL)"
+ PERL_USE_UNSAFE_INC=1 $(CONFIGURE) --prefix="$(I_PREFIX)" --sysconfdir="$(I_CONFDIR)" --datadir="$(I_DATADIR)" --enable-ssl="$(ENABLE_SSL)"
spamc_has_moved:
$(NOECHO) echo "***"
|
From @LeontOn Sun, Jan 1, 2017 at 12:36 AM, James E Keenan via RT <
There are essentially 2 kinds of breakages in the toolchain: Injecting the environmental variable in the CPAN clients fixes both issues, Injecting in the install tool can only fix testing issues, but can actually To me the first approach is more valuable than the second one, but there's Manually installing non-updated Module::Install dists will be the painful Leon |
From @toddrOn Sun, 01 Jan 2017 11:34:36 -0800, LeonT wrote:
Sure in addition to this, we might be provide advice to distro maintainers It probably wouldn't be hard to steal from Debian to get a skeleton
If you do the install tool + Test::Harness, you get most of with the
Yeah, I propose a holy crusade to eradicate M:I similar to what they're Todd |
From @jkeenanOn 01/01/2017 03:07 PM, Todd Rinaldo via RT wrote:
Can we get funding from the Bill and Melinda Gates Foundation? |
From @xsawyerxOn 01/01/2017 08:33 PM, Leon Timmermans wrote:
I'd like to ping this again. :) |
From @LeontOn Sun, Jan 15, 2017 at 11:52 AM, Sawyer X <xsawyerx@gmail.com> wrote:
I'm working on a patch to Test::Harness that should solve the test-time Leon |
From @LeontOn Sun, Jan 1, 2017 at 9:07 PM, Todd Rinaldo via RT <
Yeah, though having a clear description of for to fix it without switching Leon |
From @toddrOn Sun, 15 Jan 2017 02:52:51 -0800, xsawyerx@gmail.com wrote:
I have pinged the CPAN-workers mailing list about this. If I get no reply, I'm just going to go ahead with the environment variable fix and submit the fixes to the relevant modules. http://www.nntp.perl.org/group/perl.cpan.workers/2017/01/msg1523.html Todd |
From @toddrModule::Build patch submitted here: Perl-Toolchain-Gang/Module-Build#69 |
From @toddrExtUtils::MakeMaker patch submitted here: Perl-Toolchain-Gang/ExtUtils-MakeMaker#283 |
From @jkeenanOn Mon, 16 Jan 2017 15:34:24 GMT, LeonT wrote:
Should we need to do bulk creation of rt.cpan.org bug tickets for this purpose, the following program will get us part of the way there: https://github.com/jkeenan/cpan-mini-visit-simple/blob/master/examples/module-install-130467.pl Thank you very much. -- |
From @toddrOne of the common feedback on patches is that PERL_USE_UNSAFE_INC should not be injected if the environment variable exists in any form when the injection point is hit. This gives people control of disabling the work around and seeing the fail in its full glory. I could envision updating dzil to set PERL_USE_UNSAFE_INC=0 so that development fails and is quickly fixed as a result. |
From @toddrModule::Build::Tiny patch - Perl-Toolchain-Gang/module-build-tiny#22 |
From @toddrI declared myself dumb for scripts/cpan and reported to the issues queue to see if I could get some advice. |
From @toddrcpanm patch was submitted here: miyagawa/cpanminus#521 |
From @toddrSo I have cancelled the pull requests for Module::Build, Module::Build::Tiny, and ExtUtils::MakeMaker. It appears that the majority of issues can be addressed by: 1. Merge this release into core: https://metacpan.org/release/LEONT/Test-Harness-3.37_01 If we can solve number 2, then I can't think of anything blocking the suggested action in this ticket of changing the default build option to not include . in @INC. One concern raised so far is that while shipping M::I with perl would solve the problem, it might indicate that p5p endorses the use of Module::Install. Karen has pointed out to me that we already explicitly discourage new use of this module so that should not be a significant issue? |
From @xsawyerxOn 02/06/2017 08:40 PM, Todd Rinaldo via RT wrote:
Including Module::Install *does* implicitly encourage it. The only way This raises another question on my part: Is the intention for this to be |
From @toddrOn Wed, 08 Feb 2017 03:02:39 -0800, xsawyerx@gmail.com wrote:
So the very ROUGH numbers I have are that ~3900 out of 35,000 modules on CPAN use Module::Install. That's ~11% of CPAN. I'm not optimistic even Karen could get patched or adopt that many modules. This would probably be with us for some time. I got this by putting all of minicpan into a git repo and I know there are a few dupes. https://github.com/toddr/minicpan_grep After talking with toolchain, the below idea has been floated. It would allow us to ship ONLY this one file and not M::I with Perl. We could add docs, warnings, etc. to make it clear this is a shim ONLY for working around Module::Install's incompatibility with . not being in @INC. https://gist.github.com/toddr/a0d877645575df5615cff30fcaf379ad File: lib/inc/Module/Install.pm BEGIN { 1; |
From @jkeenanOn Wed, 08 Feb 2017 18:28:33 GMT, TODDR wrote:
Where would this file reside in the Perl 5 core distribution? -- |
From @ikegamiOn Sat, Dec 31, 2016 at 12:36 PM, Todd Rinaldo <perlbug-followup@perl.org>
It's my understanding the plan is to remove "." from @INC (and that work |
From @toddrOn Wed, 08 Feb 2017 10:40:34 -0800, jkeenan wrote:
At the moment, I'm actually thinking lib/inc/Module/Install.pm. The assumption is that this would install to PRIVLIB and be overriden by site_lib. The other item is we may only want to install this if . is missing from @INC but that starts to get complicated. |
From @toddrMechaCPAN reported atrodo/App-MechaCPAN#3 |
From @xsawyerxOn 02/08/2017 08:28 PM, Todd Rinaldo via RT wrote:
Considering this has been discussed exhaustively with toolchain, despite I would like to know if anyone objects to this. If not, we should [1] Shared by all included in the debate around it, as far as I've seen. :) |
From @kentfredricOn 13 February 2017 at 00:34, Sawyer X <xsawyerx@gmail.com> wrote:
It really hasn't by any stretch of imagination been discussed This seems more apropos of "Hastily", not "Exhaustively". And I'd imagine "Haste" is in application here because you want to "Heatedly" discussed may also be correct. But suggesting that this subject has been given a thorough dissection Say what you want, do what you want, just don't try deceiving yourself -- KENTNL - https://metacpan.org/author/KENTNL |
From @xsawyerxOn 02/13/2017 12:12 AM, Kent Fredric wrote:
Kent, you are rash in your analysis. You do not know about every conversation that takes place outside your |
From @kentfredricOn 13 February 2017 at 10:21, Sawyer X <xsawyerx@gmail.com> wrote:
Then don't blame toolchain for making this decision dude. If it was Maybe you did discuss it extensively with some members who might be But your assertion is it "has been discussed extensively with toolchain". And that, based on the above arguments, suggests that you're making a -- KENTNL - https://metacpan.org/author/KENTNL |
From @haargI have to second Kent here. "Toolchain" as an entity is #toolchain, There has been an increasing number of things that were discussed in As for the direct issue of an inc::Module::Install shim, for CPAN the On Sun, Feb 12, 2017 at 5:30 PM, Kent Fredric <kentfredric@gmail.com> wrote:
|
From @xsawyerxOn 02/13/2017 12:57 PM, Graham Knop wrote:
"Extensive" is admittedly far from the best description here, but the Importantly enough, Kent's position is not "We need a bit more time on My point remains that I am willing to accept toolchain's position, I *do not* want to muddle in the exact words used. Please accept my That is all.
p5p has made it clear that it defers to toolchain for making a decision |
From @kentfredricOn 14 February 2017 at 00:33, Sawyer X <xsawyerx@gmail.com> wrote:
To clarify: I don't have a fixed position, and that suggestion was Its simply an example of the sort of thing that *could* be done if we It was in my mind, an ideal, a feasible strategy for the least amount The inc:: approach here stands on a line between "lots of breakage" There may be strategies somewhere on that line between the inc:: Just the perception given is that the inc:: approach is "settled" now, And the pressure to create such a settlement is placed upon us by the I'd we'd tackled this problem earlier in the development cycle, we'd We can't change that now, no time machine. But using the "we have to because time" is something that is almost I know you see me as "negative", and read me like I'm just nay-saying My point is usually to establish that there *are* other points on the But that you see me myself as trying to establish a "this is how it For the record, I have no direct opposition to inc::Module::Install Those problem cases *could* in theory be fixed by submitting patches to CPAN. The problem is there's no way to smoke and fix code outside CPAN, and Hence, it is important to me that we at least properly explore options But for an example of something that I *might* reluctantly not oppose Some sort of mechanic that simply gives a better error message when Or perhaps we can ascertain there are classes of "Less volatile" entry -- KENTNL - https://metacpan.org/author/KENTNL |
From @LeontOn Mon, Feb 13, 2017 at 12:33 PM, Sawyer X <xsawyerx@gmail.com> wrote:
Toolchain is an even looser collection of people than p5p; unlike p5p it's I may have missed some of the discussion on IRC, but I haven't seen a Some of these questions are toolchain, some p5p, some both. All of them Leon |
From @toddrOn Tue, 14 Feb 2017 11:37:47 -0800, LeonT wrote:
Yes I'm learning this the hard way. Is there any interest in changing
Given we're talking about ~3,700 dists on CPAN that would need to
That should be easy to do. See line 6 in the updated gist
This module is more a shim with re-produces what the installed
I think the breakage is low enough that we just submit patches I am happy to get a solid number if that is the only outstanding hold
Given it's a shim, I don't think that's a major issue. We boot it out
p5p is easy. It can be discussed here. I don't know how to get any sort of consensus from toolchain. However, /me ducks. Todd |
From @kentfredricOn 15 February 2017 at 09:10, Todd Rinaldo via RT
We could consider the idea of a fortnightly, monthly, or bimonthly As long as sufficient advance warning of said meeting is advertised to As I imagine that as long as we had them regularly scheduled and at a This would also double as a kind of transparency provider for people I think such a strategy (or similar) would be a good adjunct to the -- KENTNL - https://metacpan.org/author/KENTNL |
From @haargOn Tue, Feb 14, 2017 at 4:06 PM, Kent Fredric <kentfredric@gmail.com> wrote:
I'm not sure if frequent scheduled meetings are really appropriate for We definitely should do better in terms of visibility and I think the scheduling of the Toolchain Summit is working against us |
From @xsawyerxOn 02/15/2017 11:00 AM, Graham Knop wrote:
I think Kent and Graham are raising important issues here. Moving the I would like to raise one more issue that Todd and myself have observed I would be happy if this is one thing toolchain changes as well, to make |
From @kentfredricOn 15 February 2017 at 22:32, Sawyer X <xsawyerx@gmail.com> wrote:
My reasons for this idea stem more or less from your next comment and Basically, /because/ there is no central authority, some sort of Instead of relying on them being omnipresent and detecting it by But I won't force the idea, it just seems like something I'd expect to
I guess it depends on the nature of the issue, if the issue only The broader the impact, the more people who have to be involved Given the nature of toolchain, I don't think a "Central Authority" Historically, I think toolchain has mostly been a "person with commit It seems disorganized, but I prefer to see it as "Self-Organised" :) Sort of like when a small group of friends decide they want to go to a Ultimately in order for collective success, all the individuals have We should probably re-hash this stuff as a new P5P thread though. -- KENTNL - https://metacpan.org/author/KENTNL |
From @LeontOn Tue, Feb 14, 2017 at 9:10 PM, Todd Rinaldo via RT
I'm sure that process has been exhausting to you :-/.
I can't think of any case where the stars aligned quite like they did
It affects a lot of distributions, but fairly few users (installing
What if this shim was a CPAN distribution? Ironically but conveniently An important advantage is that this would actually allow for the shim The main practical complication in this is that inc::Module::Install And even if we would put it into core after releasing it to CPAN, this Leon |
From @xsawyerxOn 02/15/2017 01:19 PM, Kent Fredric wrote:
My perspective on this is Whatever Works. You're in a better position
I agree. |
From @toddrOn Wed, 15 Feb 2017 16:09:31 -0800, LeonT wrote:
If I can get consensus, I'm totally cool with actioning this case (flipping default installs to not include . in @INC by default) on top of a merge of the CPAN.pm and Test::Harness changes already released to CPAN. Leon, it sounds like this is what you're suggesting? We could then submit patches to the ~3700 modules just as a courtesy. As for the shim on CPAN, I don't see that it would make sense, since anyone who needs this simply needs to bootstrap Module::Install to work around it. Todd |
From @jkeenanOn Fri, 17 Feb 2017 01:24:17 GMT, TODDR wrote:
We're at the point where, to go forward with this for perl-5.26.0, we need to merge what we've got into blead so that we can do a monthly dev release that can serve as the basis for CPANtesters to do a complete traversal of CPAN. Who is going to do that? When? Thank you very much. |
From @jkeenanOn Mon, 06 Mar 2017 14:22:16 GMT, jkeenan wrote:
I'm attaching a file derived from reviewing 'git log' in blead since Feb 20, i.e., since the release of perl-5.25.10. I examined the commit messages to see if they pertained to the subject of this ticket. Are there any other steps we need to take for this ticket before marking it 'Resolved' or, perhaps 'Pending Release'? Thank you very much. -- |
From @jkeenancommit a03e9f8 Documentation fixes for '.' possibly no longer being in @INC commit 3137772 Add "default_inc_excludes_dot" to "perl -V" output commit 458ea8f Make DEFAULT_INC_EXCLUDES_DOT the default on Windows commit 4634f48 Turn on removal of dot in @INC by default: commit 12e8377 initialize default_inc_excludes_dot to '' like every variable ommit 2a0461a warn if do "somefile" fails when . not default in @INC and somefile exists commit c615971 configure.com: default_inc_excludes_dot catch-up commit 025582b Fix copyright test: commit b27c755 Porting/sync-with-cpan: handle absence of "." from @INC commit ffb91b6 Update Test::Harness 3.36 -> 3.38 |
From @iabynOn Sun, Mar 19, 2017 at 05:19:52PM -0700, James E Keenan via RT wrote:
As far as I can tell, there's nothing further needing doing to blead to So I'll close this ticket tomorrow unless anyone objects. This is the last open 5.26.0 blocker ticket. Whoo hoo! -- |
From @xsawyerxOn 04/18/2017 06:24 PM, Dave Mitchell wrote:
\o/ |
|
@iabyn - Status changed from 'open' to 'resolved' |
From qd1qupwe.hs2@20minutemail.comOn Tue, 18 Apr 2017 12:56:28 -0700, xsawyerx@gmail.com wrote:
Hello. |
From [Unknown Contact. See original ticket]On Tue, 18 Apr 2017 12:56:28 -0700, xsawyerx@gmail.com wrote:
Hello. |
From @jkeenanOn 06/01/2017 12:07 PM, John Doe via RT wrote:
Just add '.' to @INC. Contrast the output of: $ perl -E 'say $_ for @INC;' ... with that of: $ perl -I. -E 'say $_ for @INC;'
I know of no such plans and believe that such plans would be Setting 'default_inc_excludes_dot' => 0 in Config.pm don't switch perl Config.pm is to a large extent just a record of what was decided during Thank you very much. |
From @iabynOn Thu, Jun 01, 2017 at 09:07:10AM -0700, John Doe via RT wrote:
There are no such plans. If a particular script needs dot in in @INC, then that script should be -- |
Migrated from rt.perl.org#130467 (status was 'resolved')
Searchable as RT130467$
The text was updated successfully, but these errors were encountered: