rpms / redhat-rpm-config

Clone
Source Code
GIT

#55 %set_build_flags: add LT_SYS_LIBRARY_PATH
Merged 3 years ago by ignatenkobrain. Opened 4 years ago by praiskup.
rpms/ praiskup/redhat-rpm-config LT_SYS_LIBRARY_PATH  into  master

file modified
+6 -3 
@@ -15,9 +15,9 @@ 
 

  `--prefix=/usr`) to adjust the paths to the packaging defaults.
 

  

  As a side effect, this will set the environment variables `CFLAGS`,
 

- `CXXFLAGS`, `FFLAGS`, `FCFLAGS`, and `LDFLAGS`, so they can be used by
 

- makefiles and other build tools.  (However, existing values for this
 

- variables are not overwritten.)
 

+ `CXXFLAGS`, `FFLAGS`, `FCFLAGS`, `LDFLAGS` and `LT_SYS_LIBRARY_PATH`,
 

+ so they can be used by makefiles and other build tools.  (However,
 

+ existing values for these variables are not overwritten.)
 

  

  If your package does not use autoconf, you can still set the same
 

  environment variables using
 
@@ -43,6 +43,9 @@ 
 

    driver.  At the start of the `%build` section, the environment
 

    variable `RPM_LD_FLAGS` is set to this value.
 

  

+ The variable `LT_SYS_LIBRARY_PATH` is defined here to prevent the `libtool`
 

+ script (v2.4.6+) from hardcoding %_libdir into the binaries' RPATH.
 

+ 

  These RPM macros do not alter shell environment variables.
 

  

  For some other build tools separate mechanisms exist:

file modified
+3 -1 
@@ -52,12 +52,14 @@ 
 

  # variables CFLAGS, CXXFLAGS, FFLAGS, FCFLAGS, LDFLAGS if they have
 

  # not been set already.  RPM_OPT_FLAGS and RPM_LD_FLAGS have already
 

  # been set implicitly at the start of the %%build section.
 

+ # LT_SYS_LIBRARY_PATH is used by libtool script.
 

  %set_build_flags \
 

    CFLAGS="${CFLAGS:-%{build_cflags}}" ; export CFLAGS ; \
 

    CXXFLAGS="${CXXFLAGS:-%{build_cxxflags}}" ; export CXXFLAGS ; \
 

    FFLAGS="${FFLAGS:-%{build_fflags}}" ; export FFLAGS ; \
 

    FCFLAGS="${FCFLAGS:-%{build_fflags}}" ; export FCFLAGS ; \
 

-   LDFLAGS="${LDFLAGS:-%{build_ldflags}}" ; export LDFLAGS
 

+   LDFLAGS="${LDFLAGS:-%{build_ldflags}}" ; export LDFLAGS ; \
 

+   LT_SYS_LIBRARY_PATH="${LT_SYS_LIBRARY_PATH:-%_libdir:}" ; export LT_SYS_LIBRARY_PATH
 

  

  # Internal-only.  Do not use.  Expand a variable and strip the flags
 

  # not suitable to extension builders.

The libtool script/macros (new enough, v2.4.6+) honor this
variable in cases where it isn't possible to detect the
system-wide default library path. It is e.g. able to parse
/etc/ld.so.* configuration, but there's no info about /usr/lib64.

So to not force everybody to do:

%configure LT_SYS_LIBRARY_PATH=...

... rather set this system-wide. Note that this is low-risk
change; older libtool doesn't use this variable, and really no
other tool should.

to me, this belongs to %configure and not to %set_build_flags

I don't know. @fweimer do you agree with @ignatenkobrain on that point?

FTR, there also exists a system-wide hack for older libtool scripts:
https://bugzilla.redhat.com/show_bug.cgi?id=1171106

rebased onto fc5cc06abf1c4b250f3cdb83dc894c4ee87297d8
4 years ago

As %set_build_flags can be (and is quite often) used outside of the context of %configure, I have to agree with @ignatenkobrain. Certainly it's just a random environment variable and shouldn't cause issues, but it doesn't seem like it belongs with the build flags.

(And while looking into this, I realize that%cmake should probably use %set_build_flags instead of the similar but slightly different thing it uses currently.)

rebased onto 53690d2aef656331be79b43505b3bf0a5d954c32
4 years ago

I don't disagree with Igor here except that in %configure there are no variables set these days so I was not sure. Moved to %configure, thanks for taking a look.

Is LT_SYS_LIBRARY_PATH an environment variable, or something that is processed by the autoconf script?

It is environment variable which is processed by ./configure script (not by autoconf itself). Not sure whether it answers your question... When the variable is set during ./configure phase, the built ./libtool has different defaults for sys_lib_dlsearch_path.

Well, plus the LT_SYS_LIBRARY_PATH is also env variable accepted by ./libtool script (at the time of ./libtool run, it overrides the default set by ./configure).

That said, even if the package doesn't run %configure but uses libtool, it can be useful to have that variable set in environment.

If it is an environment variable picked up by libtool, setting LT_SYS_LIBRARY_PATH in %set_build_flags seems the correct thing to do.

Would you please add a comment that LT_SYS_LIBRARY_PATH affects libtool to the macros file, and also update buildflags.md. Thanks.

Ok, I tried that. PTAL

rebased onto 3284ed0cc7473016649f2cad5edccf21bea79f18
4 years ago

rebased onto bc3609f4e70710e9d9ab1eb3cad25dcb1a02eb00
4 years ago

Maybe this? … not to hardcode %_libdir into the RPATH attribute of ELF binaries

Either “The LT_SYS_LIBRARY_PATH variable is …” or “LT_SYS_LIBRARY_PATH is …”.

Technically neither are quite correct. If we're going to be that picky:

The variable `LT_SYS_LIBRARY_PATH` is defined here to prevent the `libtool` script (v2.4.6+) from hardcoding %_libdir into the binaries' RPATH.

Thanks for comments! Updated.

rebased onto 5accde152871ca6f32c6e4064b1e0627208817c4
4 years ago

rebased onto e1fd36899628c4a96e6778c2cbfe5af5644c242e
4 years ago

Gently ping. May I do something about this?

Is there an extra colon after %_libdir?

Yes, intentionally - per libtool docs:

 -- Variable: LT_SYS_LIBRARY_PATH
     Libtool has heuristics for the system search path for
     runtime-loaded libraries.  If the guessed default does not match
     the setup of the host system, this variable can be used to modify
     that path list, as follows ('LT_SYS_LIBRARY_PATH' is a
     colon-delimited list like 'PATH'):
        * 'path:' The heuristically determined paths will be appened
          after the trailing colon;

Is anyone against merging this?

Well, it looks harmless enough.

The commit message doesn't really explain the problem that this is supposed to solves though, considering that we've gotten along all these years without such a thing. I'd prefer having the case briefly explained in the commit message, rather than having to chase through originating PR to mailing list archive, all url's which can go away in the future.

Other than that, no objections I guess.

@praiskup would you mind updating commit message to address @pmatilai concern please?

Thanks!

rebased onto 4e8c34475230098ef206538e34bbe8975e427eae
3 years ago

Sorry, I forgot about this. Updated, PTAL

rebased onto e345575
3 years ago

Pull-Request has been merged by ignatenkobrain
3 years ago
Changes Summary 2
+6 -3
file changed
buildflags.md
+3 -1
file changed
macros
Powered by Pagure 5.13.3
Documentation • File an Issue • About this Instance • SSH Hostkey/Fingerprint
© Red Hat, Inc. and others.