Bug #23735 for Module-Install: Patch against CPAN.pm batch jobs going into recursion

Thu Nov 30 23:03:30 2006 ANDK [...] cpan.org - Ticket created

Subject:

Patch against CPAN.pm batch jobs going into recursion

M:I tries to detect if it is running under CPAN.pm by looking for a lock file. But there are circumstances when this goes wrong: when the user sets a different cpan HOME directory and when CPAN.pm is run in batch mode. CPAN.pm will be setting the environment variable PERL5_CPAN_IS_RUNNING and I have a patch to teach M:I using it (without breaking current heuristic). The patch is on CPAN in ANDK/patches/Module-Install-0.64-ANDK-01.patch Thanks!

Mon Mar 05 11:45:29 2007 ANDK [...] cpan.org - Correspondence added

I asked Prof Warnock about his opinion and he said I should simply set PERL5_CPANPLUS_IS_RUNNING. What Du you think? For CPAN.pm users this is quite a critical bug because everything goes wrong as soon as Autoinstall sends CPAN.pm into recursion and the next process starts to ask the same questions, fails at the same prerequisites, starts cleaning up the build directory. etc. I have the impression more and more people are using Autoinstall ... so what are the plans for this?

Mon Mar 05 11:45:32 2007 ANDK [...] cpan.org - Status changed from 'new' to 'open'

Tue Mar 06 04:57:14 2007 adamk [...] cpan.org - Correspondence added

Actually, PERL5_CPANPLUS_IS_RUNNING has turned out to be almost entirely useless. Instead, CPANPLUS is moving to the new PERL5_CPANPLUS_IS_EXECUTING environment variable, which contains the rel2abs of the Makefile.PL or Build.PL. In the CPAN.pm case. you should set PERL5_CPAN_IS_EXECUTING to the rel2abs of the Makefile.PL. This will 1) Allow new version of Module::Install to detect the presense of a parent CPAN.pm correctly 2) Allow CPAN.pm to detect that it has been invoked recursively, and handle that (right)? Adam K

Wed Mar 07 04:56:22 2007 kane [...] cpan.org - Correspondence added

From:

KANE@cpan.org

On Mon Mar 05 11:45:29 2007, ANDK wrote:

Show quoted text

> I asked Prof Warnock about his opinion and he said I should simply set > PERL5_CPANPLUS_IS_RUNNING. What Du you think?

Argh, please no, don't do that. From the docs of CPANPLUS::Backend: ENVIRONMENT When "CPANPLUS::Backend" is loaded, which is necessary for just about every <CPANPLUS> operation, the environment variable "PERL5_CPAN- PLUS_IS_RUNNING" is set to the current process id. This information might be useful somehow to spawned processes. This is what that ENV var promises. 1.88_78 now sets it blindly. CPANPLUS may not even be on the box. It certainly may not be running, and you're setting it to '1' which is certainly not the process id... And you *certainly* don't want to 'kill -9' that to stop CPANPLUS... Please don't set ENV vars that aren't owned by your module -- it'll do more damage than good. Thanks, Jos

Sun Aug 19 00:47:09 2007 mschwern [...] cpan.org - Correspondence added

Ping! Andres, I think Adam and Kane's comments happened during the RT email blackout. You should read them if you haven't.

Thu Aug 23 01:54:11 2007 andreas.koenig.7os6VVqR [...] franz.ak.mind.de - Correspondence added

CC:	ANDK@cpan.org, KANE@cpan.org, ADAMK@cpan.org
Subject:	Re: [rt.cpan.org #23735] Patch against CPAN.pm batch jobs going into recursion
Date:	Thu, 23 Aug 2007 07:53:11 +0200
To:	bug-Module-Install@rt.cpan.org
From:	andreas.koenig.7os6VVqR@franz.ak.mind.de (Andreas J. Koenig)

Show quoted text

>>>>> On Sun, 19 Aug 2007 00:47:14 -0400, "Michael G Schwern via RT" <bug-Module-Install@rt.cpan.org> said:

Show quoted text

> Andres, I think Adam and Kane's comments happened during the RT email > blackout. You should read them if you haven't.

Thanks Schwern! Indeed I did not receive these postings as mail. I'm sorry that I dispropriated PERL5_CPANPLUS_IS_RUNNING. This decision wasn't done lightly. I waited for 6 months to get this desastrous bug in Module::Install addressed and the only way to fix it was setting this environment variable. Due to the nature of Module::Install it is *never* upgraded, so this particular bug will stay around forever. I changed my code now so that the variable is set to $$. I fear I cannot stop using it for the time being. Maybe all parties should agree on considering this variable being burnt by wrong usage. Please deprecate it. I have not yet fully understood the advice from Adam. Can this find its way into the Module::Install docs? Maybe with coding guidelines how I'm consided to use this variable? I hope this desaster makes it clear that none of us can solve this kind of problems alone. For me it makes no sense why we need 4 environment variables to stop one builder/installer from going wild but if you can paint the bigger picture, I'll be glad to follow your lead. Thanks, -- andreas

Thu Aug 23 09:50:36 2007 adamkennedybackup [...] gmail.com - Correspondence added

Subject:	Re: [rt.cpan.org #23735] Patch against CPAN.pm batch jobs going into recursion
Date:	Thu, 23 Aug 2007 23:49:41 +1000
To:	bug-Module-Install@rt.cpan.org
From:	"Adam Kennedy" <adamkennedybackup@gmail.com>

From my perspective, PERL5_CPANPLUS_IS_RUNNING was broken by design, primary because it doesn't do what it says. It just indicates that at some point above me, CPANPLUS was loaded into memory, not that it is running, and certainly not that CPANPLUS in particular happened to call the M:I Makefile.PL.

As for the behaviour of M:I, what work I have done on it is to try and prevent it doing worse things.

Certainly, setting PERL5_CPANPLUS_IS_RUNNING is now probably a good way to make M:I detect a "legacy" version of a CPAN client.

After several long discussions in the irc.perl.org #toolchain channel, kane and I agreed on a new PERL5_CPANPLUS_IS_EXECUTING=/absolute/path/to/Makefile.PL, which provides the actual functionality that we need.

As for why we need this stuff, it's to allow M:I to have some idea of what it is working with in terms of other members of the toolchain.

Show quoted text

> I hope this desaster makes it
> clear that none of us can solve this kind of problems alone.

I agree here.

I know you hate using IRC, but if you could find a way to join the rest of the toolchain maintainers in irc.perl.org #toolchain, you'll find it's very low traffic, and a higher bandwidth forum might help to avoid this sort of problem in the future.

If we are limited to stretched out disconnected conversations in mail and web forums, it's harder to co-ordinate.

On 23/08/07, (Andreas J. Koenig) via RT <bug-Module-Install@rt.cpan.org> wrote:

Show quoted text

       Queue: Module-Install
Ticket <URL: http://rt.cpan.org/Ticket/Display.html?id=23735 >

>>>>> On Sun, 19 Aug 2007 00:47:14 -0400, "Michael G Schwern via RT" < bug-Module-Install@rt.cpan.org> said:

  > Andres, I think Adam and Kane's comments happened during the RT email
  > blackout.  You should read them if you haven't.

Thanks Schwern! Indeed I did not receive these postings as mail.

I'm sorry that I dispropriated PERL5_CPANPLUS_IS_RUNNING. This
decision wasn't done lightly. I waited for 6 months to get this
desastrous bug in Module::Install addressed and the only way to fix it
was setting this environment variable. Due to the nature of
Module::Install it is *never* upgraded, so this particular bug will
stay around forever. I changed my code now so that the variable is set
to $$. I fear I cannot stop using it for the time being. Maybe all
parties should agree on considering this variable being burnt by wrong
usage. Please deprecate it.

I have not yet fully understood the advice from Adam. Can this find
its way into the Module::Install docs? Maybe with coding guidelines
how I'm consided to use this variable? I hope this desaster makes it
clear that none of us can solve this kind of problems alone. For me it
makes no sense why we need 4 environment variables to stop one
builder/installer from going wild but if you can paint the bigger
picture, I'll be glad to follow your lead.

Thanks,
--
andreas

Mon Sep 17 16:44:10 2007 rt-cpan [...] trout.me.uk - Correspondence added

From:

rt-cpan@trout.me.uk

On Mon Mar 05 11:45:29 2007, ANDK wrote:

Show quoted text

> I asked Prof Warnock about his opinion and he said I should simply set > PERL5_CPANPLUS_IS_RUNNING. What Du you think? > > For CPAN.pm users this is quite a critical bug because everything goes > wrong as soon as Autoinstall sends CPAN.pm into recursion and the next > process starts to ask the same questions, fails at the same > prerequisites, starts cleaning up the build directory. etc. > > I have the impression more and more people are using Autoinstall ... so > what are the plans for this?

Unfortunately, your changes to CPAN.pm are now a problem for CPAN.pm users, at least those of us who expect to be able to test for a modern version of CPAN.pm - any autoinstall-using Makefile.PL that does a requires onto anything that loads CPAN.pm is now broken, since the version checks load CPAN.pm and the autoinstall process then believes that CPANPLUS is already running and as such it shouldn't be doing anything. This is a potentially huge problem from my POV since a lot of Catalyst deployment setups prereq in recent CPAN.pms in order to make their build/install scripts work sensibly, and the installdeps make target on all of these now breaks. Horrible though it is, the only thing I can think of that will avoid this without re-introducing the bug is to check if Module::Install is already loaded and not set the env var if so. I'm currently adding the following stanza - my $no_cpanplus_env = !exists $ENV{PERL5_CPANPLUS_IS_RUNNING}; require CPAN; delete $ENV{PERL5_CPANPLUS_IS_RUNNING} if $no_cpanplus_env; to work around the problem but it really doesn't seem like the right solution to need to layer a workaround on top of your workaround.