Re: Supporting a perl without . in @INC

2017-01-27 Thread Leon Timmermans
On Thu, Jan 19, 2017 at 12:23 AM, Olivier Mengué 
wrote:

> When I started to seriously code in Perl 10 years ago (going besides
> scripts to modules and CPAN distributions) I immediately thought: "Having
> '.' in @INC doesn't seem a good idea. It could be exploited in a similar
> way as having '.' in $ENV{PATH}". But Perl was already 20 years old, CPAN
> 10 years old and I thought it was already too late for a fix. So I have
> started to write test suite code assuming the tests were run from the root
> of the distribution and that the root of the distribution was in @INC. So I
> wrote many tests modules in a directory t/lib and teir package names start
> with "t::lib::" so I can just "use t::lib::TestModule". And I'm not alone
> using that style: http://grep.cpan.me/?q=t%3A%3Alib%3A%3A
>

Tests are the easiest part to fix. If all test runners set it, you're done.
Remaining question is how exactly to do that. For example, what should
prove default to? (it should also allow the other one IMO).

I would propose that we fix this first, and then see what isn't working.
Module::install is obvious, but there's value in knowing what more will
break.

- If the CPAN clients and the modules installer will get
> PERL_USE_UNSAFE_INC=1 built-in, I don't see how I will be able build an
> environment to test my CPAN distributions that doesn't have . in @INC at
> all. So it seems that in the name of preserving backwards compatibility you
> are stopping the move towards your long term goal. Or did I miss something?
>

I think the solution there is to never override it if it is already set, so
that an explicit PERL_USE_UNSAFE_INC=0 will DWIM.


> - If the testsuite runs with PERL_USE_UNSAFE_INC=1, but once installed the
> runtime environment is not the same, issues with @INC will just be hidden
> in the test run to be discovered at runtime. Adding "use lib '.';" at the
> top of each test script will hide issues in the same way. So what's the
> point of running the testsuite if it can't show issues?
>

I don't see how that would happen in practice.


> But I have another proposal. Instead of modifying CPAN clients, builders
> and App::Prove to inject '.' into @INC, what about instead injecting in
> @INC the *absolute path of the root of the distribution* being
> configured/built/tested/installed? I think that this would considerably
> reduce the number of side effects and it would help to really isolate
> runtime code that relies on '.' being in @INC (assuming that code is
> covered by a testsuite).
>

Currently there simply isn't any way of injecting in the right place, short
of terrible PERL5OPT hackery that I don't want to think about too much.

Leon


Re: Supporting a perl without . in @INC

2017-01-23 Thread Graham Knop
On 1/16/17 12:06 PM, Todd Rinaldo wrote:
> * Inject PERL_USE_UNSAFE_INC=1 into the environment early in the
> following clients. This assures that everything spawned by these clients
> gets . in @INC during test/install.
> CPAN
> CPANPLUS
> App::cpanminus

App::MechaCPAN
App::cpm


Re: Supporting a perl without . in @INC

2017-01-18 Thread Todd Rinaldo

> On Jan 18, 2017, at 5:23 PM, Olivier Mengué  wrote:
> 
> Hi Todd, 
> 
> When I started to seriously code in Perl 10 years ago (going besides scripts 
> to modules and CPAN distributions) I immediately thought: "Having '.' in @INC 
> doesn't seem a good idea. It could be exploited in a similar way as having 
> '.' in $ENV{PATH}". But Perl was already 20 years old, CPAN 10 years old and 
> I thought it was already too late for a fix. So I have started to write test 
> suite code assuming the tests were run from the root of the distribution and 
> that the root of the distribution was in @INC. So I wrote many tests modules 
> in a directory t/lib and teir package names start with "t::lib::" so I can 
> just "use t::lib::TestModule". And I'm not alone using that style: 
> http://grep.cpan.me/?q=t%3A%3Alib%3A%3A 

Sure. Even more of a problem is "use inc::Module::Install;"

> So I don't think your major break can avoid a major effort to patch CPAN and 
> DarkPAN code. Either with new fixed CPAN releases or with distroprefs.

Others have claimed this. I don't think it's as hopeless as you say. Much of 
what I'm suggesting has already been implemented via patches in the perl that 
cPanel ships with little or no fallout. 

> > What at this point I feel is lacking is agreement and/or discussion that 
> > the above is the correct approach to solving this problem.
> 
> A few questions:
> - If the CPAN clients and the modules installer will get 
> PERL_USE_UNSAFE_INC=1 built-in, I don't see how I will be able build an 
> environment to test my CPAN distributions that doesn't have . in @INC at all. 
> So it seems that in the name of preserving backwards compatibility you are 
> stopping the move towards your long term goal. Or did I miss something?

My initial goal is to get . out of @INC in production environments. The build 
option -Ddefault_inc_excludes_dot provides this functionality. 

The one place that seems to be incurably broken with its dependance on . in 
@INC is the build/test/install system. I am not trying to fix that system at 
this time though I would encourage best practices going forward to discourage 
dependance on . in @INC.

> - If the testsuite runs with PERL_USE_UNSAFE_INC=1, but once installed the 
> runtime environment is not the same, issues with @INC will just be hidden in 
> the test run to be discovered at runtime. Adding "use lib '.';" at the top of 
> each test script will hide issues in the same way. So what's the point of 
> running the testsuite if it can't show issues?

For the most part that has not shown to be true. In the distro of RPMs that I 
maintain with about 1000 RPMs and the distro Debian maintains with many more, 
we have not seen any valid dependance on . in @INC once the modules are 
installed.

> But I have another proposal. Instead of modifying CPAN clients, builders and 
> App::Prove to inject '.' into @INC, what about instead injecting in @INC the 
> *absolute path of the root of the distribution* being 
> configured/built/tested/installed? I think that this would considerably 
> reduce the number of side effects and it would help to really isolate runtime 
> code that relies on '.' being in @INC (assuming that code is covered by a 
> testsuite).


Technically perl -Mblib does something very similar to this already. How would 
/home/cpan/Moo instead of . be a safer or saner path? Keep in mind nothing that 
has been done so far has discouraged require/use from supporting relative 
paths. We've only taken it out of the default.

Thanks for your thoughts!

Todd



Re: Supporting a perl without . in @INC

2017-01-18 Thread Todd Rinaldo

> On Jan 18, 2017, at 6:49 AM, James E Keenan  wrote:
> 
> On 01/16/2017 12:06 PM, Todd Rinaldo wrote:
>> All,
>> 
>> What at this point I feel is lacking is agreement and/or discussion that the 
>> above is the correct approach to solving this problem.
>> 
> 
> Well, since no one else has responded in this location, I will.
> 
> I suppose the first step would be to open bug tickets for each of the 
> distributions mentioned (github issue in the case of cpanminus).  So far, 
> this issue is referred to in only one ticket for CPAN.

I was asked to open a discussion rather than opening with the solution. 
However, 2 months of effort has yielded almost no response. I'll be opening the 
relevant tickets this weekend. 

>> If you are not for this plan and/or you are a maintainer of one of the above 
>> mentioned packages, your response would be appreciated. We're running out of 
>> time to complete this in time for perl 5.26.
>> 
> 
> Have you written, or do you need to have written, any program identifying 
> affected distributions on CPAN?
> 

Almost all Module::Install based modules will need to be fixed for sure with a 
minor change to Makefile.PL. Something along the lines of: 
perl -pi -e 's/(use inc::Module::Install;)/BEGIN{push \@INC, "."}\n$1/' 
Makefile.PL

Once the mentioned modules in my email are fixed, it was my hope we could 
simply flip the default bit on this feature in perl and let smokers reveal the 
scope of the remaining problem. I expect it to be 100-200 modules which will 
need a more hand crafted fix, though it may be more like 10-20.

Todd


Re: Supporting a perl without . in @INC

2017-01-18 Thread Olivier Mengué
Hi Todd,

When I started to seriously code in Perl 10 years ago (going besides
scripts to modules and CPAN distributions) I immediately thought: "Having
'.' in @INC doesn't seem a good idea. It could be exploited in a similar
way as having '.' in $ENV{PATH}". But Perl was already 20 years old, CPAN
10 years old and I thought it was already too late for a fix. So I have
started to write test suite code assuming the tests were run from the root
of the distribution and that the root of the distribution was in @INC. So I
wrote many tests modules in a directory t/lib and teir package names start
with "t::lib::" so I can just "use t::lib::TestModule". And I'm not alone
using that style: http://grep.cpan.me/?q=t%3A%3Alib%3A%3A

So I don't think your major break can avoid a major effort to patch CPAN
and DarkPAN code. Either with new fixed CPAN releases or with distroprefs.


> What at this point I feel is lacking is agreement and/or discussion that
the above is the correct approach to solving this problem.

A few questions:
- If the CPAN clients and the modules installer will get
PERL_USE_UNSAFE_INC=1 built-in, I don't see how I will be able build an
environment to test my CPAN distributions that doesn't have . in @INC at
all. So it seems that in the name of preserving backwards compatibility you
are stopping the move towards your long term goal. Or did I miss something?
- If the testsuite runs with PERL_USE_UNSAFE_INC=1, but once installed the
runtime environment is not the same, issues with @INC will just be hidden
in the test run to be discovered at runtime. Adding "use lib '.';" at the
top of each test script will hide issues in the same way. So what's the
point of running the testsuite if it can't show issues?

But I have another proposal. Instead of modifying CPAN clients, builders
and App::Prove to inject '.' into @INC, what about instead injecting in
@INC the *absolute path of the root of the distribution* being
configured/built/tested/installed? I think that this would considerably
reduce the number of side effects and it would help to really isolate
runtime code that relies on '.' being in @INC (assuming that code is
covered by a testsuite).

Olivier Mengué.

2017-01-16 18:06 GMT+01:00 Todd Rinaldo :

> All,
>
> Currently in blead is a change that will begin breaking many CPAN
> installs. This is a result of a non-default change to perl builds which
> removes . from @INC. There is currently a separate proposal (
> https://rt.perl.org/Public/Bug/Display.html?id=130467 )being discussed to
> remove . from @INC by default in 5.26.
>
> More information on the impact of this can also be found here.
> http://blogs.perl.org/users/todd_rinaldo/2016/11/how-removing-from-inc-is-
> about-to-break-cpan.html
>
> As I understand things, this is the closest thing to a mailing list for
> the toolchain group, so I'm trying this list first.
>
> In order to action RT 130467 without completely breaking CPAN, I propose
> the following patches to CPAN install related modules to fix the problem:
>
> * Inject PERL_USE_UNSAFE_INC=1 into the environment early in the following
> clients. This assures that everything spawned by these clients gets . in
> @INC during test/install.
> CPAN
> CPANPLUS
> App::cpanminus
>
> * Inject PERL_USE_UNSAFE_INC=1 into TAP::Harness to support ad-hoc use of
> prove. (Leon is already working on this)
>
> * Inject PERL_USE_UNSAFE_INC=1 into install modules to try to address as
> many Makefile.PL missing . in @INC issues as possible:
> ExtUtils::MakeMaker
> Module::Build
> Module::Build::Tiny
>
> What at this point I feel is lacking is agreement and/or discussion that
> the above is the correct approach to solving this problem.
>
> If you are not for this plan and/or you are a maintainer of one of the
> above mentioned packages, your response would be appreciated. We're running
> out of time to complete this in time for perl 5.26.
>
> Thanks,
> Todd Rinaldo
>
>


Re: Supporting a perl without . in @INC

2017-01-18 Thread James E Keenan

On 01/16/2017 12:06 PM, Todd Rinaldo wrote:

All,

Currently in blead is a change that will begin breaking many CPAN installs. This is a 
result of a non-default change to perl builds which removes . from @INC. There is 
currently a separate proposal ( https://rt.perl.org/Public/Bug/Display.html?id=130467 
 )being discussed to 
remove . from @INC by default in 5.26.

More information on the impact of this can also be found here. 
http://blogs.perl.org/users/todd_rinaldo/2016/11/how-removing-from-inc-is-about-to-break-cpan.html
 


As I understand things, this is the closest thing to a mailing list for the 
toolchain group, so I'm trying this list first.

In order to action RT 130467 without completely breaking CPAN, I propose the 
following patches to CPAN install related modules to fix the problem:

* Inject PERL_USE_UNSAFE_INC=1 into the environment early in the following 
clients. This assures that everything spawned by these clients gets . in @INC 
during test/install.
CPAN
CPANPLUS
App::cpanminus

* Inject PERL_USE_UNSAFE_INC=1 into TAP::Harness to support ad-hoc use of 
prove. (Leon is already working on this)

* Inject PERL_USE_UNSAFE_INC=1 into install modules to try to address as many 
Makefile.PL missing . in @INC issues as possible:
ExtUtils::MakeMaker
Module::Build
Module::Build::Tiny

What at this point I feel is lacking is agreement and/or discussion that the 
above is the correct approach to solving this problem.



Well, since no one else has responded in this location, I will.

I suppose the first step would be to open bug tickets for each of the 
distributions mentioned (github issue in the case of cpanminus).  So 
far, this issue is referred to in only one ticket for CPAN.



If you are not for this plan and/or you are a maintainer of one of the above 
mentioned packages, your response would be appreciated. We're running out of 
time to complete this in time for perl 5.26.



Have you written, or do you need to have written, any program 
identifying affected distributions on CPAN?


Thank you very much.
Jim Keenan


Re: Supporting a perl without . in @INC

2017-01-18 Thread James E Keenan

On 01/16/2017 12:06 PM, Todd Rinaldo wrote:

All,

Currently in blead is a change that will begin breaking many CPAN
installs. This is a result of a non-default change to perl builds which
removes . from @INC. There is currently a separate
proposal ( https://rt.perl.org/Public/Bug/Display.html?id=130467 )being
discussed to remove . from @INC by default in 5.26.

More information on the impact of this can also be found
here. 
http://blogs.perl.org/users/todd_rinaldo/2016/11/how-removing-from-inc-is-about-to-break-cpan.html

As I understand things, this is the closest thing to a mailing list for
the toolchain group, so I'm trying this list first.

In order to action RT 130467 without completely breaking CPAN, I propose
the following patches to CPAN install related modules to fix the problem:

* Inject PERL_USE_UNSAFE_INC=1 into the environment early in the
following clients. This assures that everything spawned by these clients
gets . in @INC during test/install.
CPAN
CPANPLUS
App::cpanminus

* Inject PERL_USE_UNSAFE_INC=1 into TAP::Harness to support ad-hoc use
of prove. (Leon is already working on this)

* Inject PERL_USE_UNSAFE_INC=1 into install modules to try to address as
many Makefile.PL missing . in @INC issues as possible:
ExtUtils::MakeMaker
Module::Build
Module::Build::Tiny

What at this point I feel is lacking is agreement and/or discussion that
the above is the correct approach to solving this problem.

If you are not for this plan and/or you are a maintainer of one of the
above mentioned packages, your response would be appreciated. We're
running out of time to complete this in time for perl 5.26.

Thanks,
Todd Rinaldo



Supporting a perl without . in @INC

2017-01-16 Thread Todd Rinaldo
All,

Currently in blead is a change that will begin breaking many CPAN installs. 
This is a result of a non-default change to perl builds which removes . from 
@INC. There is currently a separate proposal ( 
https://rt.perl.org/Public/Bug/Display.html?id=130467 
 )being discussed to 
remove . from @INC by default in 5.26. 

More information on the impact of this can also be found here. 
http://blogs.perl.org/users/todd_rinaldo/2016/11/how-removing-from-inc-is-about-to-break-cpan.html
 


As I understand things, this is the closest thing to a mailing list for the 
toolchain group, so I'm trying this list first.

In order to action RT 130467 without completely breaking CPAN, I propose the 
following patches to CPAN install related modules to fix the problem:

* Inject PERL_USE_UNSAFE_INC=1 into the environment early in the following 
clients. This assures that everything spawned by these clients gets . in @INC 
during test/install.
CPAN
CPANPLUS
App::cpanminus

* Inject PERL_USE_UNSAFE_INC=1 into TAP::Harness to support ad-hoc use of 
prove. (Leon is already working on this)

* Inject PERL_USE_UNSAFE_INC=1 into install modules to try to address as many 
Makefile.PL missing . in @INC issues as possible:
ExtUtils::MakeMaker
Module::Build
Module::Build::Tiny

What at this point I feel is lacking is agreement and/or discussion that the 
above is the correct approach to solving this problem. 

If you are not for this plan and/or you are a maintainer of one of the above 
mentioned packages, your response would be appreciated. We're running out of 
time to complete this in time for perl 5.26.

Thanks,
Todd Rinaldo