Ticket #854 (closed cage: fixed)

Opened 13 years ago

Last modified 11 years ago

config/inter/progs.pm: Figure out why -libpath: needs to be removed from $linkflags and $ldflags

Reported by: jkeenan Owned by: jkeenan
Priority: minor Milestone:
Component: configure Version: 1.3.0
Severity: low Keywords:
Cc: Language:
Patch status: Platform:

Description

This ticket is being transferred from  RT #43174. It holds a June 2007 ticketization by Paul T Cochrane of two inline comments found in config/inter/progs.pm.

Original description: In the file config/inter/progs.pm, there are two comments making the point that it is not understood why the string '-libpath:\S+' needs to be removed from the $linkflags and $ldflags variables. Why does it need to be removed? It doesn't seem from the code that anything nspecial needs to happen, but some more understanding would be nice.

Here is the relevant code from config/inter/progs.pm:

    $linkflags = $conf->data->get('linkflags');
    $linkflags =~ s/-libpath:\S+//g;    # RT#43174 No idea why.
    $linkflags = integrate( $linkflags, $conf->options->get('linkflags') );
    $linkflags = prompt( "And flags for your linker?", $linkflags ) if $ask;
    $conf->data->set( linkflags => $linkflags );

    $ldflags = $conf->data->get('ldflags');
    $ldflags =~ s/-libpath:\S+//g;      # RT#43174 No idea why.
    $ldflags = integrate( $ldflags, $conf->options->get('ldflags') );
    $ldflags = prompt( "And your $ld flags for building shared libraries?", $ldflags )        if $ask;
    $conf->data->set( ldflags => $ldflags );

The Parrot::Configure object's linkflags and ldflags settings are originally set in config/init/defaults.pm, where each is picked up from the Perl 5 %Config settings for ldflags and possibly modified later in that same module to account for 64-bit platform differences.

So, to resolve the question in this ticket, we would have to ask:

1. Under what circumstances would the Perl 5 $Config{ldflags} contain a string matching qr/-libpath:\S+/?

2. What is the harm caused by leaving a string matching that pattern in Parrot::Configure's linkflags or ldflags attributes?

Now, I don't claim to know the answers to either question. But Internet searching suggests that in this context libpath is something encountered mainly on AIX. See, for example  this page from the Linux Documentation Project or  this one from Apache. Let me quote from the latter:

On UNIX platforms you must ensure that LIBPATH environment variable 
is set properly to pick up the shared libraries at runtime. (UNIX 
gurus will understand here that LIBPATH actually translates to 
LD_LIBRARY_PATH on Solaris and Linux, SHLIB_PATH on HP-UX, 
DYLD_LIBRARY_PATH on Mac OS X, and stays as LIBPATH on AIX).

So I suspect that this regex substitution was introduced by someone familiar with AIX.

But I'm just speculating. Can anyone offer something better?

Thank you very much.
kid51

Attachments

inter.progs.patch Download (0.8 KB) - added by jkeenan 11 years ago.
Patch which ceases stripping of libpath from linkflags and ldflags

Change History

in reply to: ↑ description   Changed 11 years ago by jkeenan

  • status changed from new to assigned
  • owner set to jkeenan

Replying to jkeenan:

Reviewing this old ticket today, I tried to answer this question:

2. What is the harm caused by leaving a string matching that pattern in Parrot::Configure's linkflags or ldflags attributes?

... by simply commenting out the lines where the Trac references appeared:

$ svn diff config/inter/progs.pm 
Index: config/inter/progs.pm
===================================================================
--- config/inter/progs.pm       (revision 48718)
+++ config/inter/progs.pm       (working copy)
@@ -103,13 +103,13 @@
     $conf->debug("\nccflags: $ccflags\n");
 
     $linkflags = $conf->data->get('linkflags');
-    $linkflags =~ s/-libpath:\S+//g;    # TT #854: No idea why.
+#    $linkflags =~ s/-libpath:\S+//g;    # TT #854: No idea why.
     $linkflags = integrate( $linkflags, $conf->options->get('linkflags') );
     $linkflags = prompt( "And flags for your linker?", $linkflags ) if $ask;
     $conf->data->set( linkflags => $linkflags );
 
     $ldflags = $conf->data->get('ldflags');
-    $ldflags =~ s/-libpath:\S+//g;      # TT #854: No idea why.
+#    $ldflags =~ s/-libpath:\S+//g;      # TT #854: No idea why.
     $ldflags = integrate( $ldflags, $conf->options->get('ldflags') );
     $ldflags = prompt( "And your $ld flags for building shared libraries?", $ldflags )
         if $ask;

I configured both before and after applying this patch and, after the second run, diff-ed lib/Parrot/Config/Generated.pm. There were no meaningful differences between 'before' and 'after' on either Linux/i386 or Darwin/PPC. make test had no failures on either platform after the change.

If we could get this patch tested on other platforms and demonstrate no harm, then we could close this ticket.

Thank you very much.

kid51

Changed 11 years ago by jkeenan

Patch which ceases stripping of libpath from linkflags and ldflags

  Changed 11 years ago by doughera

On Sun, 29 Aug 2010, Parrot wrote:



>  > 2.  What is the harm caused by leaving a string matching that pattern in
>  Parrot::Configure's `linkflags` or `ldflags` attributes?

In perl5, for Windows, I believe it is set to point to the directory where 
the shared perl library is stored.  Since parrot just grabs the flags from 
the perl installation, but doesn't actually need to link against libperl, 
it does seem appropriate to remove that string.  I doubt it does any 
*harm*, but it doesn't do any good either.

-- 
    Andy Dougherty		doughera@lafayette.edu

follow-up: ↓ 4   Changed 11 years ago by Paul C. Anagnostopoulos

I checked out a fresh Parrot and applied this patch on my 32-bit Windows XP machine. I did a make and a make test and all tests were successful.

in reply to: ↑ 3 ; follow-up: ↓ 5   Changed 11 years ago by jkeenan

  • status changed from assigned to closed
  • resolution set to fixed

Replying to Paul C. Anagnostopoulos:

I checked out a fresh Parrot and applied this patch on my 32-bit Windows XP machine. I did a make and a make test and all tests were successful.

Paul,

Thanks for taking the time to test this.

I applied the change in r48758. I hereby declare this 3+-years-old ticket closed.

kid51

in reply to: ↑ 4   Changed 11 years ago by doughera

Replying to jkeenan:

Replying to Paul C. Anagnostopoulos:

I checked out a fresh Parrot and applied this patch on my 32-bit Windows XP machine. I did a make and a make test and all tests were successful.

Paul, Thanks for taking the time to test this. I applied the change in r48758. I hereby declare this 3+-years-old ticket closed.

Hang on -- passing tests is good, but it's not the complete story.

Paul: Can you please check the differences in the 'myconfig' file generated before and after this patch? Specifically, did the original contain '-libpath:'? If so, does it point somewhere that parrot should be looking? I still think this patch is wrong and the original should have been retained (albeit probably with an explanatory comment!).

follow-up: ↓ 7   Changed 11 years ago by Paul C. Anagnostopoulos

Okay, so I started with the current release, ran configure, and saved myconfig, config_lib.pir, and lib/parrot/config/generated.pm. Then I reverse-applied the patch to put the two -libpath lines back in progs.pm. Then I did a realclean and configured again.

The only differences in those three files are the configuration dates.

in reply to: ↑ 6 ; follow-up: ↓ 8   Changed 11 years ago by doughera

  • status changed from closed to reopened
  • resolution fixed deleted

Replying to Paul C. Anagnostopoulos:

The only differences in those three files are the configuration dates.

Ok, that just means that the patch is irrelevant for your configuration.

Here's what I think is happening: On some builds of Perl for Win32, ldflags includes -libpath. For example, ActiveState's Perl, installed into the default location, has

ldflags='-nologo -nodefaultlib -debug -opt:ref,icf  -libpath:"C:\Perl\lib\CORE"  -machine:x86';

On other builds of Perl for Win32, such as Strawberry Perl, the "-L" flag is used instead, so the code in question in Configure.pl does nothing.

There is normally no reason for parrot to be linking against libraries stored in C:\Perl\lib\CORE, so it makes sense for parrot's Configure.pl to remove the entry -libpath:"C:\Perl\lib\CORE .

I think the patch should be reverted, and an explanatory comment added instead.

in reply to: ↑ 7   Changed 11 years ago by jkeenan

Replying to doughera:

I think the patch should be reverted, and an explanatory comment added instead.

Reverted in r48769. Does the explanatory comment work for you (both/all of you)? Please let me know so that I can re-close the ticket before Tuesday.

Thank you very much.

kid51

  Changed 11 years ago by jkeenan

  • status changed from reopened to new

  Changed 11 years ago by jkeenan

  • status changed from new to assigned

  Changed 11 years ago by doughera

On Fri, 3 Sep 2010, Parrot wrote:

>  Reverted in r48769.  Does the explanatory comment work for you (both/all
>  of you)?  Please let me know so that I can re-close the ticket before
>  Tuesday.

The comment's actually backwards.  Strawberry Perl doesn't use -libpath.
Maybe just something like the following (sorry to email, but I don't have 
an easy way to get to trac at the moment).  Not that it really matters, 
however, since it's all explained in the ticket.

Index: config/inter/progs.pm
===================================================================
--- config/inter/progs.pm	(revision 48769)
+++ config/inter/progs.pm	(working copy)
@@ -103,8 +103,8 @@
     $conf->debug("\nccflags: $ccflags\n");
 
     $linkflags = $conf->data->get('linkflags');
-    # The substitution below has no effect on most platforms but may be needed
-    # on Win32 with Strawberry Perl and other OSes.  See TT #854.
+    # Remove the path to the Perl library (from Win32 config).
+    # See TT #854.
     $linkflags =~ s/-libpath:\S+//g;
     $linkflags = integrate( $linkflags, $conf->options->get('linkflags') );
     $linkflags = prompt( "And flags for your linker?", $linkflags ) if $ask;


-- 
    Andy Dougherty		doughera@lafayette.edu

  Changed 11 years ago by jkeenan

  • status changed from assigned to closed
  • resolution set to fixed

Applied in r48772. Re-closing ticket.

Note: See TracTickets for help on using tickets.