Bug #46633 for ExtUtils-MakeMaker: 6.52 causes link errors in Time::HiRes on VMS

Tue Jun 02 20:30:17 2009 cberry [...] cpan.org - Ticket created

Subject:

6.52 causes link errors in Time::HiRes on VMS

After 6.52 came into blead, the blead build broke on VMS with a pile of linker errors when it got to the Time::HiRes extension. I've attached a diff of the generated dsscrip.mms for Time::HiRes comparing 6.50 to 6.52. The most obvious problem is right here: @@ -293,9 +296,6 @@ PERLTYPE = # Time::HiRes might depend on some other libraries: # See ExtUtils::Liblist for details # -EXTRALIBS = -LDLOADLIBS = [--]DBGPerlShr.exe/Share -BSLOADLIBS = where a very important library has gone missing. The odd thing is none of the other extensions in core have this problem. They all still have the line that looks like: LDLOADLIBS = [--]DBGPerlShr.exe/Share The only likely suspect I can see that makes Time::HiRes different is that it passes LIBS to WriteMakefile(). I will have a look at Liblist and see what's changed -- for now this is just a report.

Subject:

vms_650_652_diff.txt

Message body is not shown because it is too large.

Tue Jun 02 21:34:39 2009 schwern [...] pobox.com - Correspondence added

Subject:	Re: [rt.cpan.org #46633] 6.52 causes link errors in Time::HiRes on VMS
Date:	Tue, 02 Jun 2009 18:34:16 -0700
To:	bug-ExtUtils-MakeMaker [...] rt.cpan.org
From:	Michael G Schwern <schwern [...] pobox.com>

Craig A. Berry via RT wrote: Show quoted text

> The only likely suspect I can see that makes Time::HiRes different is > that it passes LIBS to WriteMakefile(). > > I will have a look at Liblist and see what's changed -- for now this is > just a report.

This change is the prime suspect. http://github.com/schwern/extutils-makemaker/commit/6963c4e0a03f3f6399f2964e7ca4f3e187255c3b Liblist hasn't changed since September. -- 52. Not allowed to yell "Take that Cobra" at the rifle range. -- The 213 Things Skippy Is No Longer Allowed To Do In The U.S. Army http://skippyslist.com/list/

Tue Jun 02 21:34:40 2009 The RT System itself - Status changed from 'new' to 'open'

Tue Jun 02 23:32:14 2009 craigberry [...] mac.com - Correspondence added

Subject:	Re: [rt.cpan.org #46633] 6.52 causes link errors in Time::HiRes on VMS
Date:	Tue, 02 Jun 2009 22:31:55 -0500
To:	bug-ExtUtils-MakeMaker [...] rt.cpan.org
From:	"Craig A. Berry" <craigberry [...] mac.com>

On Jun 2, 2009, at 8:34 PM, Michael G Schwern via RT wrote: Show quoted text

> <URL: https://rt.cpan.org/Ticket/Display.html?id=46633 > > > Craig A. Berry via RT wrote:

>> The only likely suspect I can see that makes Time::HiRes different is >> that it passes LIBS to WriteMakefile(). >> >> I will have a look at Liblist and see what's changed -- for now >> this is >> just a report.

> > This change is the prime suspect. > http://github.com/schwern/extutils-makemaker/commit/6963c4e0a03f3f6399f2964e7ca4f3e187255c3b

Maybe, though from reading code (not yet watching in the debugger), I think a more likely suspect is: <http://perl5.git.perl.org/perl.git/blobdiff/20f91e418dfa8bdf6cf78614bfebebc28a7613ee..5bdf71cc9217764029fb9a321295118de3bb8e72:/lib/ExtUtils/MM_Any.pm Show quoted text

>

For some reason I can't find a proper pointer to that change at github.com. In this section here in MM_Any.pm: + foreach my $libs ( @{$self->{LIBS}} ){ + $libs =~ s/^\s*(.*\S)\s*$/$1/; # remove leading and trailing whitespace + my(@libs) = $self->extliblist($libs); + if ($libs[0] or $libs[1] or $libs[2]){ + # LD_RUN_PATH now computed by ExtUtils::Liblist + ($self->{EXTRALIBS}, $self->{BSLOADLIBS}, + $self->{LDLOADLIBS}, $self->{LD_RUN_PATH}) = @libs; + last; + } + } it sure looks to me like $self->{LDLOADLIBS} will be overwritten with the third element of @libs regardless of whether that element exists as long as any of the first three elements exists and is true. Similar could be said of EXTRALIBS, BSLOADLIBS, or LD_RUN_PATH. Unless extliblist is guaranteed to return exactly the right number of values in the right places. Show quoted text

________________________________________ Craig A. Berry mailto:craigberry@mac.com "... getting out of a sonnet is much more difficult than getting in." Brad Leithauser

Tue Jun 02 23:59:58 2009 schwern [...] pobox.com - Correspondence added

Subject:	Re: [rt.cpan.org #46633] 6.52 causes link errors in Time::HiRes on VMS
Date:	Tue, 02 Jun 2009 20:59:30 -0700
To:	bug-ExtUtils-MakeMaker [...] rt.cpan.org
From:	Michael G Schwern <schwern [...] pobox.com>

craigberry@mac.com via RT wrote: Show quoted text

> For some reason I can't find a proper pointer to that change at > github.com. > > In this section here in MM_Any.pm: > > + foreach my $libs ( @{$self->{LIBS}} ){ > + $libs =~ s/^\s*(.*\S)\s*$/$1/; # remove leading and trailing > whitespace > + my(@libs) = $self->extliblist($libs); > + if ($libs[0] or $libs[1] or $libs[2]){ > + # LD_RUN_PATH now computed by ExtUtils::Liblist > + ($self->{EXTRALIBS}, $self->{BSLOADLIBS}, > + $self->{LDLOADLIBS}, $self->{LD_RUN_PATH}) = @libs; > + last; > + } > + } > > it sure looks to me like $self->{LDLOADLIBS} will be overwritten with > the third element of @libs regardless of whether that element exists > as long as any of the first three elements exists and is true. > Similar could be said of EXTRALIBS, BSLOADLIBS, or LD_RUN_PATH. > Unless extliblist is guaranteed to return exactly the right number of > values in the right places.

That was last touched by ecb0efd8 and it should have just been a code move out of MM_Unix->init_others() and into MM_Any->init_others. Its possible that subtly changed the order of initialization. -- The mind is a terrible thing, and it must be stopped.

Wed Jun 03 09:21:20 2009 craigberry [...] mac.com - Correspondence added

Subject:	Re: [rt.cpan.org #46633] 6.52 causes link errors in Time::HiRes on VMS
Date:	Wed, 03 Jun 2009 08:20:23 -0500
To:	bug-ExtUtils-MakeMaker [...] rt.cpan.org
From:	"Craig A. Berry" <craigberry [...] mac.com>

On Jun 2, 2009, at 10:59 PM, Michael G Schwern via RT wrote: Show quoted text

> > craigberry@mac.com via RT wrote:

>> >> In this section here in MM_Any.pm: >> >> + foreach my $libs ( @{$self->{LIBS}} ){ >> + $libs =~ s/^\s*(.*\S)\s*$/$1/; # remove leading and trailing >> whitespace >> + my(@libs) = $self->extliblist($libs); >> + if ($libs[0] or $libs[1] or $libs[2]){ >> + # LD_RUN_PATH now computed by ExtUtils::Liblist >> + ($self->{EXTRALIBS}, $self->{BSLOADLIBS}, >> + $self->{LDLOADLIBS}, $self->{LD_RUN_PATH}) = @libs; >> + last; >> + } >> + } >> >>

Show quoted text

> That was last touched by ecb0efd8 and it should have just been a > code move out > of MM_Unix->init_others() and into MM_Any->init_others. Its > possible that > subtly changed the order of initialization.

Here's what the debugger says right before that code executes: ExtUtils::MM_Any::init_others(../../lib/ExtUtils/MM_Any.pm:1785): 1785: foreach my $libs ( @{$self->{LIBS}} ){ DB<9> x $self->{LIBS} 0 ARRAY(0x7b2f60) empty array which I guess shouldn't be a surprise since an empty array is exactly what the Makefile.PL passes in for LIB, except apparently there's an expectation that standard libraries still get appended or prepended or something. But here we never execute the loop because the array is empty. FWIW, extliblist does seem to return the right things even when passed an empty array: DB<13> x $self->extliblist() 0 '' 1 '' 2 '[--]DBGPerlShr.exe/Share' 3 '' Gotta go, but will look further tonight. Show quoted text

________________________________________ Craig A. Berry mailto:craigberry@mac.com "... getting out of a sonnet is much more difficult than getting in." Brad Leithauser

Wed Jun 03 20:53:13 2009 craigberry [...] mac.com - Correspondence added

Subject:	Re: [rt.cpan.org #46633] 6.52 causes link errors in Time::HiRes on VMS
Date:	Wed, 03 Jun 2009 19:52:51 -0500
To:	bug-ExtUtils-MakeMaker [...] rt.cpan.org
From:	"Craig A. Berry" <craigberry [...] mac.com>

On Jun 3, 2009, at 8:21 AM, craigberry@mac.com via RT wrote: Show quoted text

> <URL: https://rt.cpan.org/Ticket/Display.html?id=46633 > > > > On Jun 2, 2009, at 10:59 PM, Michael G Schwern via RT wrote: >

>> >> craigberry@mac.com via RT wrote:

>>> >>> In this section here in MM_Any.pm: >>> >>> + foreach my $libs ( @{$self->{LIBS}} ){ >>> + $libs =~ s/^\s*(.*\S)\s*$/$1/; # remove leading and >>> trailing >>> whitespace >>> + my(@libs) = $self->extliblist($libs); >>> + if ($libs[0] or $libs[1] or $libs[2]){ >>> + # LD_RUN_PATH now computed by ExtUtils::Liblist >>> + ($self->{EXTRALIBS}, $self->{BSLOADLIBS}, >>> + $self->{LDLOADLIBS}, $self->{LD_RUN_PATH}) = @libs; >>> + last; >>> + } >>> + } >>> >>>

>

>> That was last touched by ecb0efd8 and it should have just been a >> code move out >> of MM_Unix->init_others() and into MM_Any->init_others. Its >> possible that >> subtly changed the order of initialization.

> > > Here's what the debugger says right before that code executes: > > ExtUtils::MM_Any::init_others(../../lib/ExtUtils/MM_Any.pm:1785): > 1785: foreach my $libs ( @{$self->{LIBS}} ){ > DB<9> x $self->{LIBS} > 0 ARRAY(0x7b2f60) > empty array > > > which I guess shouldn't be a surprise since an empty array is exactly > what the Makefile.PL passes in for LIB, except apparently there's an > expectation that standard libraries still get appended or prepended or > something. But here we never execute the loop because the array is > empty.

The code wasn't just moved, it was refactored. The old code checked not only for the whole array being undef but also for element zero being undef and forced it to an array with one zero-length element in both cases. With the attached patch the new code now does that as well. With this, Time::HiRes now builds fine in blead on VMS. Show quoted text

________________________________________ Craig A. Berry mailto:craigberry@mac.com "... getting out of a sonnet is much more difficult than getting in." Brad Leithauser

Message body is not shown because sender requested not to inline it.

Thu Jun 04 13:45:58 2009 craigberry [...] mac.com - Correspondence added

CC:	cberry [...] cpan.org
Subject:	Re: [rt.cpan.org #46633] 6.52 causes link errors in Time::HiRes on VMS
Date:	Thu, 04 Jun 2009 12:45:15 -0500
To:	bug-ExtUtils-MakeMaker [...] rt.cpan.org
From:	"Craig A. Berry" <craigberry [...] mac.com>

On Jun 3, 2009, at 7:53 PM, craigberry@mac.com via RT wrote: Show quoted text

> The code wasn't just moved, it was refactored. The old code checked > not only for the whole array being undef but also for element zero > being undef and forced it to an array with one zero-length element in > both cases. With the attached patch the new code now does that as > well. With this, Time::HiRes now builds fine in blead on VMS.

Well, that patch generated a warning in one of the tests, so instead of revising the new code, the patch attached here just replaces it with the old code, which is easier to read anyway. With this in blead, the build goes fine on both Mac OS X and VMS, and there are no new test failures as a result of it. Show quoted text

________________________________________ Craig A. Berry mailto:craigberry@mac.com "... getting out of a sonnet is much more difficult than getting in." Brad Leithauser

Message body is not shown because sender requested not to inline it.

Sun Jun 07 21:37:40 2009 mschwern [...] cpan.org - Correspondence added

Thanks for digging this out! I've fixed it in d7a7ee105de60f87f8fd3c03cd95fb99383129f6 Instead of reverting I've kept the table format. I find it much easier to see what's going on and all the logic is gathered together. "unless @{$self->{LIBS}} && defined $self->{LIBS}[0];" is a bit mind mending. I also pulled it out into its own method and wrote a test for it so it won't happen again!

Sun Jun 07 21:37:42 2009 mschwern [...] cpan.org - Status changed from 'open' to 'resolved'