Created attachment 175836 [details] [patch] remove hard-coded lib path for stage-qa The hard-coded LD_LIBRARY_PATH=${LOCALBASE}/lib causes problems when a library is installed both by ports and base. Just one example: devel/jsoncpp ... ====> Running Q/A tests (stage-qa) Error: /usr/local/lib/libjsoncpp.so.1.7.7 is linked to /usr/local/lib/libc++.so.1 from devel/libc++ but it is not declared as a dependency Warning: you need LIB_DEPENDS+=libc++.so:devel/libc++ I don't see a reason why it might be needed at the moment, so my first patch attempt just removes it.
There must be some reason for it, I'm asking about it in bug #195203 to the patch's submitter.
The original procedure in question was: > lst_immediate_deps() { > LD_LIBRARY_PATH=${LOCALBASE}/lib ldd -a "$1" | \ > awk 'BEGIN {section=0}; /^\// {section++} /^[^\/]/ {if(section<=1) print}' | \ > sed -e 's/.*=> //' -e 's/ .*//' >} The current lines (after Mathieu modification): > $(LD_LIBRARY_PATH=${LOCALBASE}/lib ldd -a "${STAGEDIR}${file}" | \ > awk '\ > BEGIN {section=0}\ > /^\// {section++}\ > !/^\// && section<=1 && ($3 ~ "^'${PREFIX}'" || $3 ~ "^'${LOCALBASE}'") {print $3}') lst_immediate_deps() finds all immediate dependencies of the given elf. LD_LIBRARY_PATH=${LOCALBASE}/lib was added to merely ensure ldd looks there. So if it looks there by default it shouldn't make any difference at all. Calling code was selecting only the libraries that are under ${LOCALBASE}. So, again, LD_LIBRARY_PATH shouldn't matter if ldd looks there by default. How does the output of ldd command differ on your system with and without LD_LIBRARY_PATH=${LOCALBASE}/lib? Or does it differ? On my system /usr/local/lib/libjsoncpp.so.1.7.7 only depends on the base shared libraries: > $ ldd /usr/local/lib/libjsoncpp.so.1.7.7 > /usr/local/lib/libjsoncpp.so.1.7.7: > libthr.so.3 => /lib/libthr.so.3 (0x801639000) > libc++.so.1 => /usr/lib/libc++.so.1 (0x80185d000) > libcxxrt.so.1 => /lib/libcxxrt.so.1 (0x801b1c000) > libm.so.5 => /lib/libm.so.5 (0x801d39000) > libgcc_s.so.1 => /lib/libgcc_s.so.1 (0x801f62000) > libc.so.7 => /lib/libc.so.7 (0x800822000) I believe the difference is caused by being built locally (portupgrade) vs. in poudriere. Environment, the presence of preinstalled packages, impacts this. I think it prints this warning properly for you: > Warning: you need LIB_DEPENDS+=libc++.so:devel/libc++ The real problem is the volatility of dependency being base vs. localbase triggered by local vs. poudriere build. I am not sure what the solution of this problem couldbe, b ut it has nothing to do with LD_LIBRARY_PATH here.
(In reply to yuri from comment #2) Thanks for providing the background for why LD_LIBRARY_PATH was added. By the way, here's the difference (you should get the same if the libc++ port is installed): % ldd /usr/local/lib/libjsoncpp.so > /tmp/base ; env LD_LIBRARY_PATH=/usr/local/lib ldd /usr/local/lib/libjsoncpp.so > /tmp/ports ; diff -u /tmp/base /tmp/ports > --- /tmp/base 2016-10-18 14:50:37.247449000 -0600 > +++ /tmp/ports 2016-10-18 14:50:37.263341000 -0600 > @@ -1,7 +1,8 @@ > /usr/local/lib/libjsoncpp.so: > libthr.so.3 => /lib/libthr.so.3 (0x801639000) > - libc++.so.1 => /usr/lib/libc++.so.1 (0x80185d000) > - libcxxrt.so.1 => /lib/libcxxrt.so.1 (0x801b1c000) > - libm.so.5 => /lib/libm.so.5 (0x801d39000) > - libgcc_s.so.1 => /lib/libgcc_s.so.1 (0x801f62000) > + libc++.so.1 => /usr/local/lib/libc++.so.1 (0x80185d000) > + libcxxrt.so.1 => /lib/libcxxrt.so.1 (0x801b1d000) > + libm.so.5 => /lib/libm.so.5 (0x801d3a000) > + libgcc_s.so.1 => /lib/libgcc_s.so.1 (0x801f63000) > libc.so.7 => /lib/libc.so.7 (0x800822000) > + librt.so.1 => /usr/lib/librt.so.1 (0x802171000) Pretty straightforward... the path specified in LD_LIBRARY_PATH is searched first causing it to find /usr/local/lib/libc++.so.1 (and its dependencies) before /usr/lib/libc++.so.1 If we remove LD_LIBRARY_PATH from qa.sh, we'll just use the normal system search path... which is the right thing for stage-qa to do. Since you said it was added just to get ldd to look in /usr/local/lib, then it's definitely not needed. The system looks there by default already (try 'ldconfig -r | grep search'). rpath and setuid-ness can change the lib search behavior a bit, but neither of those are involved in this case. This same problem can happen whether you build with poudriere or not. It may be that if you build jsoncpp using poudriere, devel/libc++ might not be installed in your jail. In that case, of course you won't see the problem. I believe you could try doing a bulk poudriere build with devel/libc++ and devel/jsoncpp if you really want to see the problem in poudriere. In any case, inside or outside the jail, I believe it's safe (and correct) to remove the LD_LIBRARY_PATH.
I see. Then your patch is correct. Just commit it.
I don't have /usr/local/lib/libc++.so.1 installed. Shared libs can be linked by the full path, or by the name alone. Not sure how this is chosen during build, but in the former case the preinstalled libs may influence the path it is linked with. That's how, I think, volatility comes into being. I can't recall which package I observed this on.
(In reply to yuri from comment #5) The actual library that is "chosen during the build" depends on -L, -rpath and (as you mentioned) possibly libraries specified with an absolute path. Since we're talking about dynamic linking, however, that's only half of the "linking" picture. At run time (or when you run ldd), the second part of the linking job is finished off and the library file that is chosen can be influenced by a number of things, such as: - whether a run time "hint" was supplied at build time (-rpath) - LD_PRELOAD & LD_LIBRARY_PATH (if not setuid binaries unless you're root) - the ldconfig system search path With all the variables involved, there is more than one way to fool stage-qa so that it doesn't necessarily find undocumented library dependencies. Given the default system configuration, removing LD_LIBRARY_PATH from qa.sh removes one reasonably likely potential false positive source, and leaving it there doesn't help find undeclared dependencies at all.
Agreed!
A commit references this bug: Author: mat Date: Tue Oct 25 11:44:19 UTC 2016 New revision: 424616 URL: https://svnweb.freebsd.org/changeset/ports/424616 Log: Do not force a lookup in LOCALBASE/lib for shared libraries. ldd should work correctly if rpath is set, and this adds false positives. PR: 213547 Submitted by: John Hein Sponsored by: Absolight Changes: head/Mk/Scripts/qa.sh
r424616 fixed this but didn't automatically close the PR. Closing it now.