parrot commented 15 years ago

Unfortunately I have to find out, that the created binaries of parrot contains rpath. According to the Fedora packaging guidelines binaries should not contain rpaths. Instead a separate ld.so.conf file should be used.

I was able to remove the rpaths with chrpath but this is not the preferred way to do this.

Originally http://trac.parrot.org/parrot/ticket/476 by JSchmitt

parrot commented 15 years ago

Trac commenter: JSchmitt

This is a problem for Debian as well, and apparently SuSE also (see ticket #474). The patch attached to #474 isn't a general solution, so we'll probably want some way to disable rpath at configuration time.

ghost commented 15 years ago

Disabling rpath in general would be a bad plan. It would make it harder for ordinary non-root users to build and install a version of parrot into any directory other than the directories searched by ld.so. Disabling rpath_blib (as is done in TT #474) is a bad idea -- how is the parrot built during the build process supposed to find libparrot.so?

What would likely be sufficient would be to leave rpath_blib as is, and change config/inter/libparrot.pm to skip setting up rpath_lib (the version that will be used on the installed parrot) if 'libdir' is /usr/lib. (Ideally, I suppose, it should skip it for any directory already searched by ld.so, but that would require parsing the output of ldconfig or some other such tool, and I don't know how standard any of that is.)

parrot commented 15 years ago

3782 byte attachment from JSchmitt at http://trac.parrot.org/parrot/raw-attachment/ticket/476/config_no_rpath.diff

+++ parrot-1.0.0/config/gen/makefiles/json.in
@@ -9,6 +9,8 @@
 PGE_DIR  := ../../compilers/pge
 TGE_DIR  := ../../compilers/tge

+export LD_LIBRARY_PATH := ../../blib/lib
+
 # the default target
 all: JSON.pbc

--- parrot-1.0.0.orig/config/gen/makefiles/nqp.in
+++ parrot-1.0.0/config/gen/makefiles/nqp.in
@@ -9,6 +9,8 @@
 PARROT_LIBRARY := @build_dir@/runtime/parrot/library
 PGE_LIBRARY    := @build_dir@/runtime/parrot/library/PGE

+export LD_LIBRARY_PATH := ../../blib/lib
+
 all: nqp.pbc

 SOURCES := \
--- parrot-1.0.0.orig/config/gen/makefiles/pct.in
+++ parrot-1.0.0/config/gen/makefiles/pct.in
@@ -11,6 +11,8 @@
 PGE_DIR  := ../../compilers/pge
 TEST_DIR := ../../t/compilers/pct

+export LD_LIBRARY_PATH := ../../blib/lib
+
 # Where to put things
 PARROT_LIBRARY := ../../runtime/parrot/library

--- parrot-1.0.0.orig/config/gen/makefiles/pge.in
+++ parrot-1.0.0/config/gen/makefiles/pge.in
@@ -9,6 +9,8 @@

 TOOL_DIR := ../..

+export LD_LIBRARY_PATH := ../../blib/lib
+
 # Where to put things
 PARROT_LIBRARY := ../../runtime/parrot/library

--- parrot-1.0.0.orig/config/gen/makefiles/root.in
+++ parrot-1.0.0/config/gen/makefiles/root.in
@@ -534,6 +534,8 @@
 .pbc$(O) : # suffix rule (limited support)
    $(PARROT) -o $@ $<

+# This sets up proper internal dynamic linking in case we don't set an rpath
+export LD_LIBRARY_PATH = $(CUR_DIR)/blib/lib

 ###############################################################################
 #
@@ -750,7 +752,7 @@

 runtime/parrot/include/config.fpmc : myconfig config_lib.pasm $(MINIPARROT)
    @echo Invoking Parrot to generate runtime/parrot/include/config.fpmc --cross your fingers
-   $(MINIPARROT) config_lib.pasm > $@
-   LD_LIBRARY_PATH=$(CUR_DIR)/blib/lib $(MINIPARROT) config_lib.pasm > $@

  $(PARROT) : $(SRC_DIR)/main$(O) $(GEN_HEADERS) $(LIBPARROT) CFLAGS \
     lib/Parrot/OpLib/core.pm $(SRC_DIR)/parrot_config$(O) \
  --- parrot-1.0.0.orig/config/gen/makefiles/tge.in
  +++ parrot-1.0.0/config/gen/makefiles/tge.in
  @@ -6,6 +6,8 @@
  RM_F     := @rm_f@
  PARROT   := ../../parrot@exe@

+export LD_LIBRARY_PATH := ../../blib/lib
+
 # Where to put things
 PARROT_LIBRARY    := ../../runtime/parrot/library
 PERL6GRAMMAR      := $(PARROT_LIBRARY)/PGE/Perl6Grammar.pbc
--- parrot-1.0.0.orig/config/inter/libparrot.pm
+++ parrot-1.0.0/config/inter/libparrot.pm
@@ -34,6 +34,7 @@
 sub runstep {
     my ( $self, $conf ) = @_;
     my $parrot_is_shared = $conf->options->get('parrot_is_shared');
-    my $site_install = $conf->options->get('site_install');

   $parrot_is_shared = integrate(
       $conf->data->get('parrot_is_shared'),
  @@ -98,7 +99,8 @@
   # Set -rpath (or equivalent) for executables to find the
   # shared libparrot in the build directory.
   $conf->data->set(
-        rpath_blib => ( $parrot_is_shared && $conf->data->get('rpath') )
-        rpath_blib => ( $parrot_is_shared && $conf->data->get('rpath') 
-           && !$site_install )
       ? $conf->data->get('rpath')
           . $conf->data->get('build_dir')
           . $conf->data->get('slash')
  @@ -109,7 +111,8 @@
   # Set -rpath (or equivalent) for the installed executables to find the
   # installed shared libparrot.
   $conf->data->set(
-        rpath_lib => ( $parrot_is_shared && $conf->data->get('rpath') )
-        rpath_lib => ( $parrot_is_shared && $conf->data->get('rpath')
-                       && !$site_install )
       ? $conf->data->get('rpath')
           . $conf->data->get('libdir')
       : ''
  --- parrot-1.0.0.orig/lib/Parrot/Configure/Options/Conf/Shared.pm
  +++ parrot-1.0.0/lib/Parrot/Configure/Options/Conf/Shared.pm
  @@ -62,6 +62,7 @@
   sbindir
   sharedstatedir
   silent
-    site_install
   sysconfdir
   test
   verbose

parrot commented 15 years ago

3784 byte attachment from JSchmitt at http://trac.parrot.org/parrot/raw-attachment/ticket/476/parrot-1.0.0-norpath.diff

+++ parrot-1.0.0/config/gen/makefiles/json.in
@@ -9,6 +9,8 @@
 PGE_DIR  := ../../compilers/pge
 TGE_DIR  := ../../compilers/tge

+export LD_LIBRARY_PATH := ../../blib/lib
+
 # the default target
 all: JSON.pbc

--- parrot-1.0.0.orig/config/gen/makefiles/nqp.in
+++ parrot-1.0.0/config/gen/makefiles/nqp.in
@@ -9,6 +9,8 @@
 PARROT_LIBRARY := @build_dir@/runtime/parrot/library
 PGE_LIBRARY    := @build_dir@/runtime/parrot/library/PGE

+export LD_LIBRARY_PATH := ../../blib/lib
+
 all: nqp.pbc

 SOURCES := \
--- parrot-1.0.0.orig/config/gen/makefiles/pct.in
+++ parrot-1.0.0/config/gen/makefiles/pct.in
@@ -11,6 +11,8 @@
 PGE_DIR  := ../../compilers/pge
 TEST_DIR := ../../t/compilers/pct

+export LD_LIBRARY_PATH := ../../blib/lib
+
 # Where to put things
 PARROT_LIBRARY := ../../runtime/parrot/library

--- parrot-1.0.0.orig/config/gen/makefiles/pge.in
+++ parrot-1.0.0/config/gen/makefiles/pge.in
@@ -9,6 +9,8 @@

 TOOL_DIR := ../..

+export LD_LIBRARY_PATH := ../../blib/lib
+
 # Where to put things
 PARROT_LIBRARY := ../../runtime/parrot/library

--- parrot-1.0.0.orig/config/gen/makefiles/root.in
+++ parrot-1.0.0/config/gen/makefiles/root.in
@@ -534,6 +534,8 @@
 .pbc$(O) : # suffix rule (limited support)
    $(PARROT) -o $@ $<

+# This sets up proper internal dynamic linking in case we don't set an rpath
+export LD_LIBRARY_PATH = $(CUR_DIR)/blib/lib

 ###############################################################################
 #
@@ -750,7 +752,7 @@

 runtime/parrot/include/config.fpmc : myconfig config_lib.pasm $(MINIPARROT)
    @echo Invoking Parrot to generate runtime/parrot/include/config.fpmc --cross your fingers
-   $(MINIPARROT) config_lib.pasm > $@
-   LD_LIBRARY_PATH=$(CUR_DIR)/blib/lib $(MINIPARROT) config_lib.pasm > $@

  $(PARROT) : $(SRC_DIR)/main$(O) $(GEN_HEADERS) $(LIBPARROT) CFLAGS \
     lib/Parrot/OpLib/core.pm $(SRC_DIR)/parrot_config$(O) \
  --- parrot-1.0.0.orig/config/gen/makefiles/tge.in
  +++ parrot-1.0.0/config/gen/makefiles/tge.in
  @@ -6,6 +6,8 @@
  RM_F     := @rm_f@
  PARROT   := ../../parrot@exe@

+export LD_LIBRARY_PATH := ../../blib/lib
+
 # Where to put things
 PARROT_LIBRARY    := ../../runtime/parrot/library
 PERL6GRAMMAR      := $(PARROT_LIBRARY)/PGE/Perl6Grammar.pbc
--- parrot-1.0.0.orig/config/inter/libparrot.pm
+++ parrot-1.0.0/config/inter/libparrot.pm
@@ -34,6 +34,7 @@
 sub runstep {
     my ( $self, $conf ) = @_;
     my $parrot_is_shared = $conf->options->get('parrot_is_shared');
-    my $site_install = $conf->options->get('disable-rpath');

   $parrot_is_shared = integrate(
       $conf->data->get('parrot_is_shared'),
  @@ -98,7 +99,8 @@
   # Set -rpath (or equivalent) for executables to find the
   # shared libparrot in the build directory.
   $conf->data->set(
-        rpath_blib => ( $parrot_is_shared && $conf->data->get('rpath') )
-        rpath_blib => ( $parrot_is_shared && $conf->data->get('rpath') 
-           && !$site_install )
       ? $conf->data->get('rpath')
           . $conf->data->get('build_dir')
           . $conf->data->get('slash')
  @@ -109,7 +111,8 @@
   # Set -rpath (or equivalent) for the installed executables to find the
   # installed shared libparrot.
   $conf->data->set(
-        rpath_lib => ( $parrot_is_shared && $conf->data->get('rpath') )
-        rpath_lib => ( $parrot_is_shared && $conf->data->get('rpath')
-                       && !$site_install )
       ? $conf->data->get('rpath')
           . $conf->data->get('libdir')
       : ''
  --- parrot-1.0.0.orig/lib/Parrot/Configure/Options/Conf/Shared.pm
  +++ parrot-1.0.0/lib/Parrot/Configure/Options/Conf/Shared.pm
  @@ -62,6 +62,7 @@
   sbindir
   sharedstatedir
   silent
-    disable-rpath
   sysconfdir
   test
   verbose

parrot commented 15 years ago

Trac commenter: JSchmitt

I want to suggest the introduction of the configure flag which allow the user to dicide about the generation of rpaths.

parrot commented 15 years ago

Trac commenter: JSchmitt

<p>

Config option --site_install to disable rpath

parrot commented 15 years ago

Trac commenter: JSchmitt

I've just attached a patch with a --site_install option to disable rpath when building for distros. It's not enabled by default so that testers and developers can proceed as usual. Note that I haven't patched all the compiler Makefile generators, only the ones I needed to build using the standard Configure.pl, but it should be trivial to do so.

parrot commented 15 years ago

Trac commenter: JSchmitt

Can you attached this file in a propper downloadable format?

parrot commented 15 years ago

Trac commenter: JSchmitt

Replying to JSchmitt:

Can you attached this file in a propper downloadable format?

It's the link at the bottom of the page showing the diff. Here it is in case you can't get to it:

https://trac.parrot.org/parrot/raw-attachment/ticket/476/config_no_rpath.diff

parrot commented 15 years ago

Trac commenter: JSchmitt

Thank you, the patch works well. But I want to suggest, that you should rename the option into --disable-rpath, so you know directly what task this option has.

parrot commented 15 years ago

Trac commenter: JSchmitt

Replying to JSchmitt:

Thank you, the patch works well. But I want to suggest, that you should rename the option into --disable-rpath, so you know directly what task this option has.

Sure, I'm fine with that. I picked site_install because I thought there might be things other than disabling rpath that would be important for doing this sort of install. This lets us have a single option for these things, although since I don't know of any currently using disable-rpath is fine. If someone with commit privs wants me to re-make the patch using this as the option name please let me know.

parrot commented 15 years ago

Trac commenter: JSchmitt

<p>

Patch to implement a --disable-rpath option

parrot commented 15 years ago

Trac commenter: JSchmitt

I have added a patch where I have renamed the option into --disable-rpath

jkeenan commented 15 years ago

Replying to JSchmitt:

I have added a patch where I have renamed the option into --disable-rpath

Patch, if adopted, will need a brief description of the function of the --disable-rpath option in an appropriate part of the usage message found in print_help() lib/Parrot/Configure/Options/Conf.pm.

--disable-rpath should also be placed alphabetically within the list in lib/Parrot/Configure/Options/Conf/Shared.pm.

Thank you very much.
kid51

ghost commented 15 years ago

Please don't apply this patch. There are several problems.

1. The syntax export LD_LIBRARY_PATH = ../../blib/lib is not portable syntax for a makefile.

2. The command LD_LIBRARY_PATH = ../../blib/lib unconditionally overwrites the user's setting of LD_LIBRARY_PATH. That's bad, particularly if the user had deliberately set LD_LIBRARY_PATH to something useful on that particular system.

3. The environment variable LD_LIBRARY_PATH goes by different names on different systems.

4. The disabling of rpath for the build library seems to me to be wholly unnecessary. I gather the desire is to avoid setting rpath in the *installed* versions of the utilities, but I don't understand at all why it would be necessary to avoid setting it for the build versions.

parrot commented 15 years ago

Trac commenter: JSchmitt

Replying to doughera:

Please don't apply this patch. There are several problems. 1. The syntax export LD_LIBRARY_PATH = ../../blib/lib is not portable syntax for a makefile.

I decided to do this because it seemed simplest, although I am admittedly biased towards GNU make. I can move the LD_LIBRARY_PATH = ... in front of each call to $(PARROT) in the makefile instead, which should be portable. I'll post a patch with that sometime soon unless someone beats me to it.

2. The command LD_LIBRARY_PATH = ../../blib/lib unconditionally overwrites the user's setting of LD_LIBRARY_PATH. That's bad, particularly if the user had deliberately set LD_LIBRARY_PATH to something useful on that particular system.

This is indeed an issue. Setting LD_LIBRARY_PATH = $(LD_LIBRARY_PATH):../../blib/lib would trivially solve this.

3. The environment variable LD_LIBRARY_PATH goes by different names on different systems.

Porters for those systems who are interested in building without rpath should make the necessary fixes for their system. For linux systems this is the correct environment variable to use. Furthermore, it's silly to block this patch for a non-default configure option that's important to the major linux distros. Other systems won't be affected by this unless they want to be.

4. The disabling of rpath for the build library seems to me to be wholly unnecessary. I gather the desire is to avoid setting rpath in the *installed* versions of the utilities, but I don't understand at all why it would be necessary to avoid setting it for the build versions.

Do you have an alternative suggestion then? Every Linux distro that I've seen so far (including Debian/Ubuntu, Redhat/Fedora, and SuSE) needs for rpath to be removed, and there is no need to require it to be set for the build process.

ghost commented 15 years ago

1269 byte attachment from doughera at http://trac.parrot.org/parrot/raw-attachment/ticket/476/rpath-lib-patch.txt

+++ parrot-andy/config/inter/libparrot.pm   2009-03-23 23:49:57.118939305 -0400
@@ -107,13 +107,21 @@
     );

Set -rpath (or equivalent) for the installed executables to find the

-    # installed shared libparrot.
-    $conf->data->set(
-        rpath_lib => ( $parrot_is_shared && $conf->data->get('rpath') )
-        ? $conf->data->get('rpath')
-            . $conf->data->get('libdir')
-        : ''
-    );
-    # installed shared libparrot.  Don't set rpath if the library
-    # directory is going to be searched by ld.so anyway.  For now,
-    # hard-code those directories as qw(/lib /usr/lib /usr/lib64).
-    if ($parrot_is_shared && $
-         conf->data->get('rpath') && 
-         $conf->data->get('libdir') ne '/lib' &&
-         $conf->data->get('libdir') ne '/usr/lib' &&
-         $conf->data->get('libdir') ne '/usr/lib64') {
-        $conf->data->set( rpath_lib => 
-            $conf->data->get('rpath') . $conf->data->get('libdir')
-        );
-    }
-    else { 
-        $conf->data->set( rpath_lib => "");
-    }

   unless ( defined( $conf->data->get('libparrot_ldflags') ) ) {
       $conf->data->set(libparrot_ldflags =>

ghost commented 15 years ago

I appreciate that some Linux distributions have a policy against using rpath. (I do not understand precisely why they have such a policy, but I also am not out to try to change it.) I also appreciate that you simply want to fix this minor problem and get on with other things.

However, the suggested patch leaves behind a little bit of a maintenance mess in parrot's Configure and Makefile, and I think it would be nice to try to clean it up first.

Although your needs are specific to parrot on Linux, I can easily imagine other packagers wishing to use the same functionality, hence my request to try to make it more portable and general. This is part of my overall bias that, to the extent reasonable, things in Configure.pl ought to be done in ways that are general and extensible.

The whole thing would not be an issue if Configure.pl offered you a way to specify arbitrary Configure variables at the command line, something like

perl Configure.pl --with-rpath_lib=""

Alas it doesn't. A patch to allow it to do so would be *very* welcome. (But it would also be a *lot* of work; I do not expect anyone to actually implement it.)

Instead you have proposed adding a specific Configure.pl flag, to be added to Configure.pl's existing hodge-podge of conflicting command-line flag styles. Again, that pre-existing hodge-podge is neither your problem nor your responsibility. However, it does give some of the background for why I am resistant to simply adding more.

I also think you have missed the important distinction between rpath_blib (which is used only during the build process) and rpath_lib (which is used for the installed executables). Surely the RedHat (and SuSE and Debian) policies have to do with not using rpath in the executables that are actually installed, not with the executables that are built and used merely as part of the build process. I understand that you *can* get rid of rpath during the build process and replace it by the LD_LIBRARY_PATH settings. I just don't see why you have to.

Fortunately, as I have already discussed in my reply 2 days ago, a much easier and more general solution already exists: Simply have config/inter/libparrot.pm skip the rpath_lib stuff if the library directory is one which is going to be searched by ld.so anyway. A simple test of '/lib' or '/usr/lib' (or '/usr/lib64', if it exists) should suffice. I would think this would work for *BSD and Solaris as well as Linux, without any environment variable fiddling or Makefile patching. If it doesn't work for some reason, then it would make sense to consider some more complicated fix.

I've thrown together a proof-of-concept patch to show exactly what I mean. I'm sure it could be gussied up a bit, and I haven't tested it, but this is my alternative suggestion.

ghost commented 15 years ago

Patch to skip unnecessary rpath_lib setting

chromatic commented 15 years ago

How about we skip rpath for installable Parrots, figuring that anyone who installs a libparrot to somewhere that ld doesn't see can figure out how to make it work? I assume that we can pass the appropriate "look in blib/lib to find libparrot" flag when compiling the installable files, and that installing them won't hurt anything. If that's not the case, then ignore this comment.

The only time we really need to use rpath is when linking an application we don't intend to install -- when building a Parrot to run from the checkout directory. Am I right?

coke commented 15 years ago

I appear to need rpath to link a binary I do intend to install (for OS X). The installed binary is linked against the libs in the build dir otherwise.

ghost commented 15 years ago

Replying to chromatic:

How about we skip rpath for installable Parrots, figuring that anyone who installs a libparrot to somewhere that ld doesn't see can figure out how to make it work?

Actually that's the exact situation for which I created the rpath_lib stuff in the first place. Using rpath in that situation is a very natural and useful way to make it just work.

I assume that we can pass the appropriate "look in blib/lib to find libparrot" flag when compiling the installable files, and that installing them won't hurt anything. If that's not the case, then ignore this comment.

Alas, the executable can "remember" the build directory then.

The only time we really need to use rpath is when linking an application we don't intend to install -- when building a Parrot to run from the checkout directory. Am I right?

That certainly is one time when rpath is very useful. (Not strictly necessary, but very convenient and useful.) I don't understand why it would be a problem, and said so repeatedly above, but gravity claims it is, even for things we don't intend to install. I can't imagine why, and think there must be some miscommunication somewhere.

ghost commented 15 years ago

Replying to coke:

I appear to need rpath to link a binary I do intend to install (for OS X). The installed binary is linked against the libs in the build dir otherwise.

I didn't think the OS X rpath stuff worked at all. If you have found a set of commands that will finally make it installable on OS X with a shared libparrot, I'll take a look and see what the Configure system can do to make that work. If it means my patch above is wrong, then so be it. Alas, I don't have any OS X experience.

parrot commented 15 years ago

Trac commenter: JSchmitt

Replying to doughera:

That certainly is one time when rpath is very useful. (Not strictly necessary, but very convenient and useful.) I don't understand why it would be a problem, and said so repeatedly above, but gravity claims it is, even for things we don't intend to install. I can't imagine why, and think there must be some miscommunication somewhere.

Yes, there is indeed a miscommunication. I don't think there's a problem using rpath for things that aren't intended to be installed. That's why I named the original config option "site_install" in order to explicitly denote that. I also do think it's useful to use rpath for non-installed parrot, which is why I made it a configuration option rather than a blanket removal of rpath use from the build system. I only want to be able to build parrot without using rpath for systemwide installs as by distro packages.

parrot commented 15 years ago

Trac commenter: JSchmitt

Replying to chromatic:

How about we skip rpath for installable Parrots, figuring that anyone who installs a libparrot to somewhere that ld doesn't see can figure out how to make it work? I assume that we can pass the appropriate "look in blib/lib to find libparrot" flag when compiling the installable files, and that installing them won't hurt anything. If that's not the case, then ignore this comment. The only time we really need to use rpath is when linking an application we don't intend to install -- when building a Parrot to run from the checkout directory. Am I right?

This is exactly what the patch I wrote should be doing. If it's not then I did something very wrong and I'd love to hear what so I can fix it. The goal was to explicitly tell the configure script that you're building an installable parrot (using the site_install option, although that name kinda sucks) and so it takes the appropriate action to disable rpath. Otherwise, the build system works as before.

allisonrandal commented 15 years ago

You've all almost got it. Parrot already does custom linking for installable_parrot. When you set the prefix to "/usr", it sets rpath in installable_parrot to "/usr/lib". Which is silly when "/usr/lib" is already part of the library path.

We can't know which library paths the user will have set up on their system, so selecting by path isn't a good solution.

a) Add a --disable-rpath option that only disables the rpath option on installable_parrot (so the build parrot continues working as-is). This means only setting the 'rpath_lib' option to blank, but leaving the 'rpath_blib' option configured as usual. (Yes, those were terrible names for the options, but let's not rename them at the moment.)

b) Don't set LD_LIBRARY_PATH in the Parrot build process. If the user requested --disable-rpath, then they can take the extra steps to set LD_LIBRARY_PATH or their platform equivalent. The documentation and command-line help for the --disable-rpath option should instruct the user to do this.

c) Follow Jim Keenan's recommendations on documentation and sort order.

d) Add configure tests to make sure the --disable-rpath option has the desired result.

Allison

ghost commented 15 years ago

Replying to gravity:

This is exactly what the patch I wrote should be doing. If it's not then I did something very wrong and I'd love to hear what so I can fix it.

Your patch is *also* removing rpath from the build process for things that are not being installed -- look at the parts changing rpath_blib. That's what I mean about the distinction between rpath_lib and rpath_blib. I understand you want to change rpath_lib. I don't understand why your patch changes rpath_blib.

ghost commented 15 years ago

Replying to allison:

You've all almost got it. Parrot already does custom linking for installable_parrot. When you set the prefix to "/usr", it sets rpath in installable_parrot to "/usr/lib". Which is silly when "/usr/lib" is already part of the library path.

Silly, but harmless, I would think. Still I do understand it runs afoul of some rigid Linux packaging system guidelines.

We can't know which library paths the user will have set up on their system, so selecting by path isn't a good solution.

True, it's not ideal, and we don't want to get in the business of parsing ldconfig output. Another inconsistently-named hand-parsed and hand-tested Configure option isn't ideal either, but the difference between the two approaches isn't that big a deal to me, as long as I don't have to do it. Both are working around a fundamental weakness in Configure.pl, namely that the user can't directly set rpath_lib from the Configure.pl command line.

The bigger issue for me is the rpath_lib vs. rpath_blib distinction. All 4 of my numbered objections above are specific to rpath_blib. If we leave rpath_blib alone, they are automatically rendered moot. I primarily intended my patch as a proof-of-concept patch that could demonstrate that rpath_blib didn't need any changes at all. My verbal descriptions of the differences between rpath_blib and rpath_lib have apparently not been sufficiently clear. Accordingly, I thought a patch might be clearer. That's all.

ghost commented 15 years ago

4178 byte attachment from doughera at http://trac.parrot.org/parrot/raw-attachment/ticket/476/rpath_lib-2.patch

--- parrot-current/config/init/defaults.pm  2009-03-21 21:02:56.000000000 -0400
+++ parrot-andy/config/init/defaults.pm 2009-03-25 00:10:33.701938937 -0400
@@ -129,6 +129,21 @@
         # -shared for GNU ld.
         ld_load_flags => $Config{lddlflags},
-        # Set -rpath (or equivalent) to append to $linkflags (and
-        # $ldflags) for executables to find the shared libparrot in
-        # the build directory.
-        # This is often something like -R $builddir/blib/lib
-        # (This is initialized to undef so that later parts of
-        # Configure.pl can distinguish between the variable being
-        # unset and being set (on the command line) to the empty
-        # string.)
-        rpath_blib => undef,
  +
-        # Set -rpath (or equivalent) for the installed executables to
-        # find the installed shared libparrot.  
-        # This is often something like -R /usr/local/lib
- ```
     rpath_lib => undef,

+ libs => $Config{libs},

   cc_inc     => "-I./include",

diff -r -u parrot-current/config/inter/libparrot.pm parrot-andy/config/inter/libparrot.pm --- parrot-current/config/inter/libparrot.pm 2009-03-21 21:02:56.000000000 -0400 +++ parrot-andy/config/inter/libparrot.pm 2009-03-25 00:49:53.213944808 -0400 @@ -97,22 +97,30 @@

Set -rpath (or equivalent) for executables to find the

shared libparrot in the build directory.

$conf->data->set(
rpath_blib => ( $parrot_is_shared && $conf->data->get('rpath') )
? $conf->data->get('rpath')
. $conf->data->get('build_dir')
. $conf->data->get('slash')
. $conf->data->get('blib_dir')
: ''
);
Respect a hint file or command-line override.
my $rpath_blib = $conf->option_or_data('rpath_blib');
if (!defined($rpath_blib)) {
$rpath_blib = ( $parrot_is_shared && $conf->data->get('rpath') )
? $conf->data->get('rpath')
. $conf->data->get('build_dir')
. $conf->data->get('slash')
. $conf->data->get('blib_dir')
: '';
}

Set -rpath (or equivalent) for the installed executables to find the

installed shared libparrot.
my $rpath_lib = $conf->option_or_data('rpath_lib');
if (!defined($rpath_lib)) {
$rpath_lib = ( $parrot_is_shared && $conf->data->get('rpath') )
? $conf->data->get('rpath')
. $conf->data->get('libdir')
: '';
} + $conf->data->set(
rpath_lib => ( $parrot_is_shared && $conf->data->get('rpath') )
? $conf->data->get('rpath')
. $conf->data->get('libdir')
: ''
rpath_blib => $rpath_blib,
```
 rpath_lib  => $rpath_lib
```
);

unless ( defined( $conf->data->get('libparrot_ldflags') ) ) { diff -r -u parrot-current/lib/Parrot/Configure/Options/Conf/Shared.pm parrot-andy/lib/Parrot/Configure/Options/Conf/Shared.pm --- parrot-current/lib/Parrot/Configure/Options/Conf/Shared.pm 2009-03-21 21:02:48.000000000 -0400 +++ parrot-andy/lib/Parrot/Configure/Options/Conf/Shared.pm 2009-03-25 00:17:38.418939204 -0400 @@ -58,6 +58,7 @@ parrot_is_shared prefix profile
rpath_lib sbindir sharedstatedir silent diff -r -u parrot-current/lib/Parrot/Configure/Options/Conf.pm parrot-andy/lib/Parrot/Configure/Options/Conf.pm --- parrot-current/lib/Parrot/Configure/Options/Conf.pm 2009-03-21 21:02:48.000000000 -0400 +++ parrot-andy/lib/Parrot/Configure/Options/Conf.pm 2009-03-25 00:18:04.834939494 -0400 @@ -133,6 +133,9 @@ [/usr/local] --exec-prefix=EPREFIX Install architecture-dependent files in EPREFIX [PREFIX]
--rpath_lib=RPATH add RPATH into link flags to tell the
installed parrot how to find the installed
```
                     version of libparrot.so (if appropriate).
```
By default, make install' will install all the files in /usr/local/bin', `/usr/local/lib' etc. You can specify

ghost commented 15 years ago

I've attached a patch which allows you to set rpath_lib directly to whatever is needed, including setting it to the empty string. Thus to meet the original needs of this ticket, you would type

     perl Configure.pl --rpath_lib=""

(i.e. set it to the empty string). This has the advantage of being a more general solution -- should users on other platforms find Configure's built-in guesses for how to set rpath to be incorrect, this gives them a workaround way to specify the correct flags. (I didn't quite go all all the way towards implementing a general way to specify Configure.pl variables, but I was tempted.)

ghost commented 15 years ago

A patch to allow specifying rpath_lib on the Configure.pl command line

allisonrandal commented 15 years ago

We have an immediate use case for --disable-rpath, and it's a standard option in the Linux world. We don't have a use case for setting rpath_lib directly, since it's already set when the user sets the installation paths.

We can always add direct rpath_lib setting later if/when someone has a specific need for it.

allisonrandal commented 15 years ago

Turns out, the option has to disable rpath_blib too, otherwise dynamic PMCs and ops are built with the blib/lib rpath.

Committed in r37823. Resolving ticket.

ghost commented 15 years ago

Replying to allison:

Turns out, the option has to disable rpath_blib too, otherwise dynamic PMCs and ops are built with the blib/lib rpath.

That sounds to me like there's a bug in the build system, then. In no case should installed files depend on the build directory. This merely papers over that bug for the case of Linux packagers.

Committed in r37823. Resolving ticket.

I'll let you decide whether to reopen this ticket, or resolve it, open a new ticket for the dynpmc linking bug, and refer back to here to unapply the rpath_blib bits once that new ticket it resolved.

ghost commented 15 years ago

I created a new ticket TT #540 specifically about the dynamic PMCs incorrectly linking to the build path.

ghost commented 15 years ago

Now that TT #540 is fixed, the part of this patch that disables rpath_blib can be reverted.

allisonrandal commented 15 years ago

The --disable-rpath option isn't a workaround for a bug, it's a needed feature for building Linux packages. And, it's not much use unless it disables -rpath everywhere. (We could add --disable-rpath and --really-disable-rpath options, but that'd be a bit silly since the only time you need the option is when you *really* need it.)

No changes and reclosing ticket.

ghost commented 15 years ago

Replying to allison:

The --disable-rpath option isn't a workaround for a bug, it's a needed feature for building Linux packages.

No, you are misunderstanding me. The disable-rpath option currently does *two* things: it disables rpath_blib (which is used only in the build directory) and rpath_lib (which is used for the installables). The bug in TT #540 is that dynamic PMCS intended to be installed were linking to things in the build directory. The disable-rpath option worked around that bug by disabling rpath_blib. Now that the bug in TT #540 is fixed, the disable-rpath option no longer has to disable rpath_blib. Phrased differently, disabling rpath_blib should never be necessary because nothing linked with rpath_blib should ever be installed.

And, it's not much use unless it disables -rpath everywhere.

No. As I have tried to explain over and over again, but apparently to no avail, disabling rpath_blib should be unnecessary. Yes, I understand the packager can work around it by using LD_LIBRARY_PATH (or equivalent). But that work-around would not be required if parrot didn't disable rpath_blib in the first place.

coke commented 15 years ago

doughera's thread throughout this ticket of rpath_lib vs. rpath_blib is never addressed. Re-opening ticket.

jkeenan commented 13 years ago

Andy,

This is another ticket where, if you could shed some light on the current status of the issues, it would be appreciated.

Thank you very much.

kid51

ghost commented 13 years ago

Replying to jkeenan:

Andy, This is another ticket where, if you could shed some light on the current status of the issues, it would be appreciated.

Nothing has changed.

parrot / parrot

Binaries should not contain rpath #471

Set -rpath (or equivalent) for the installed executables to find the

Set -rpath (or equivalent) for executables to find the

shared libparrot in the build directory.

Respect a hint file or command-line override.

Set -rpath (or equivalent) for the installed executables to find the

installed shared libparrot.