Skip Menu |
 

This queue is for tickets about the Module-Build CPAN distribution.

Report information
The Basics
Id: 49350
Status: resolved
Priority: 0/
Queue: Module-Build

People
Owner: Nobody in particular
Requestors: CRAKRJACK [...] cpan.org
Cc:
AdminCc:

Bug Information
Severity: Normal
Broken in: 0.35
Fixed in: 0.36



Subject: new_from_context inlines 'Build.PL'
Download (untitled) / with headers
text/plain 572b
->new_from_context() inlines Build.PL, which means that if a Build script elects to terminate early, that causes the perl environment further up the chain to exit as well. For example, Params::Classify 0.007 has an "exit 0" at the end of it's Build.PL, which ensures that if ->new_from_context is ever called on that module, it will exit early. Since Build.PL is touted as a "script", I think it should be treated as such as much as possible, and we should not let the product of a Build.PL pollute the environment above that, whether that's M::B, CPANPLUS, or whatever.
Download (untitled) / with headers
text/plain 530b
The partner to this bug in Params::Classify is here; https://rt.cpan.org/Ticket/Display.html?id=49351 For now, I am using the following workaround. I hope it doesnt have any other unintended side-effects :-) ## hackery for https://rt.cpan.org/Ticket/Display.html?id=49350 ## and https://rt.cpan.org/Ticket/Display.html?id=49351 my $old_new_from_context = \&Module::Build::Base::new_from_context; local *Module::Build::Base::new_from_context = sub { local *CORE::GLOBAL::exit = sub { return @_ }; $old_new_from_context->(@_); };
Download (untitled) / with headers
text/plain 367b
new_from_context as implemented is really no longer necessary. After discussion on #toolchain with ewilhelm, our thought is to deprecate new_from_context and add a load_from_build_pl() class method that runs Build.PL in a separate process and then resumes it the same way the Build script would. (Lowering severity to "normal" as this is not a release-blocking bug)
Download (untitled) / with headers
text/plain 448b
On Tue Sep 01 22:50:42 2009, DAGOLDEN wrote: Show quoted text
> new_from_context as implemented is really no longer necessary. After > discussion on #toolchain with ewilhelm, our thought is to deprecate > new_from_context
Thanks... so if new_from_context is depreciating, I guess CPANPLUS should stop using it. Would you be willing to file an RT over there for this? You'd probably be better at explaining how to upgrade that code than I... Cheers, Tyler
Subject: Re: [rt.cpan.org #49350] new_from_context inlines 'Build.PL'
Date: Wed, 2 Sep 2009 05:33:26 -0400
To: bug-Module-Build [...] rt.cpan.org
From: David Golden <dagolden [...] cpan.org>
Download (untitled) / with headers
text/plain 411b
On Tue, Sep 1, 2009 at 11:50 PM, CRAKRJACK via RT<bug-Module-Build@rt.cpan.org> wrote: Show quoted text
> Thanks... so if new_from_context is depreciating, I guess CPANPLUS > should stop using it. Would you be willing to file an RT over there for > this? You'd probably be better at explaining how to upgrade that code > than I...
CPANPLUS::Dist::Build no longer uses new_from_context (since about version 0.08 in March 2009).
Subject: Re: [rt.cpan.org #49350] new_from_context inlines 'Build.PL'
Date: Wed, 2 Sep 2009 08:47:06 -0500
To: bug-Module-Build [...] rt.cpan.org
From: Ken Williams <kwilliams [...] cpan.org>
Download (untitled) / with headers
text/plain 630b
On Tue, Sep 1, 2009 at 9:50 PM, David Golden via RT<bug-Module-Build@rt.cpan.org> wrote: Show quoted text
>       Queue: Module-Build >  Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=49350 > > > new_from_context as implemented is really no longer necessary.  After > discussion on #toolchain with ewilhelm, our thought is to deprecate > new_from_context and add a load_from_build_pl() class method that runs > Build.PL in a separate process and then resumes it the same way the > Build script would.
Seems like a good idea. I'd even be in favor of keeping the method name the same and just swapping out the implementation. -Ken
Subject: Re: [rt.cpan.org #49350] new_from_context inlines 'Build.PL'
Date: Wed, 2 Sep 2009 10:53:35 -0400
To: bug-Module-Build [...] rt.cpan.org
From: David Golden <dagolden [...] cpan.org>
Download (untitled) / with headers
text/plain 532b
On Wed, Sep 2, 2009 at 9:47 AM, Ken Williams via RT<bug-Module-Build@rt.cpan.org> wrote: Show quoted text
> Seems like a good idea.  I'd even be in favor of keeping the method > name the same and just swapping out the implementation.
We we're being cautious because of a scary comments you put in about why it needed to happen that way. (Which we think are now stale given other implementation changes, particularly around preserving things added to @INC.) So if you support the change, I'm all for changing the guts of new_from_context. David
Subject: Re: [rt.cpan.org #49350] new_from_context inlines 'Build.PL'
Date: Wed, 2 Sep 2009 11:20:50 -0500
To: bug-Module-Build [...] rt.cpan.org
From: Ken Williams <kwilliams [...] cpan.org>
Download (untitled) / with headers
text/plain 494b
On Wed, Sep 2, 2009 at 9:54 AM, David Golden via RT<bug-Module-Build@rt.cpan.org> wrote: Show quoted text
> We we're being cautious because of a scary comments you put in about > why it needed to happen that way.  (Which we think are now stale given > other implementation changes, particularly around preserving things > added to @INC.)
Reading that comment, I'm not sure what I meant by "make sure that the environment is the same". All the cases I can think of should be fine. So I say go for it. -Ken
Patched in trunk


This service is sponsored and maintained by Best Practical Solutions and runs on Perl.org infrastructure.

Please report any issues with rt.cpan.org to rt-cpan-admin@bestpractical.com.