This is a patch to let env-update call ldconfig when it is found. Please review. Reproducible: Always
Created attachment 391320 [details, diff] portage-prefix-ldconfig.patch
Created attachment 391322 [details, diff] portage-prefix-ldconfig.patch
Where's that thing checking whether or not $EPREFIX/{usr,}sbin/ldconfig exists? I for sure don't have it.
Created attachment 393208 [details, diff] portage-prefix-ldconfig.patch (In reply to Fabian Groffen from comment #3) > Where's that thing checking whether or not $EPREFIX/{usr,}sbin/ldconfig > exists? I for sure don't have it. Sorry, I forgot to add the tests. Patch updated.
I hate to be complaining this much, but does it make sense to check existence of the file /after/ you checked whether it is executable? I can imagine: a) the access check returns false when the file doesn't exist, rendering the existence check pointless b) the access check throwing an error when the file doesn't exist, making the existence check useful when applied before the access check.
(In reply to Fabian Groffen from comment #5) > I hate to be complaining this much, but does it make sense to check > existence of the file /after/ you checked whether it is executable? > > I can imagine: > a) the access check returns false when the file doesn't exist, rendering the > existence check pointless > b) the access check throwing an error when the file doesn't exist, making > the existence check useful when applied before the access check. I think os.path.isfile check is to guard against sbin/ldconfig being a directory (with X_OK bit). Actually, I have stolen the code from find_binary of pym/portage/process.py def find_binary(binary): ..... for path in paths: filename = _os.path.join(path, binary) if _os.access(filename, os.X_OK) and _os.path.isfile(filename): return filename return None
Hi Fabian, more thoughts on this? I think the order is reasonable according the comment above.
pushed, should be in next portage version
Hi portage team, Could you please review and cherry-pick commit dde7fb0919d40cc5585e8a603046e7098371ad69 Author: Benda Xu Date: Wed Dec 10 10:45:31 2014 +0900 let env-update call ldconfig if it exists In Prefix, let env-update call ldconfig if it exists. It does not affect prefix-rpath, and handles prefix-libc(RAP) correctly. in the prefix branch to properly call ldconfig in EPREFIX? Thanks a lot! Benda
(In reply to Benda Xu from comment #9) > Hi portage team, > > Could you please review and cherry-pick commit > dde7fb0919d40cc5585e8a603046e7098371ad69 In the context of the above commit, the eprefix variable refers to config['EPREFIX']. However, as discussed in bug #582802, comment #4, we want to use portage.const.EPREFIX in order to correctly handle cross-prefix situations. Consider ROOT support, which existed before EPREFIX or cross-prefix. We've never called ${ROOT}/sbin/ldconfig because it wouldn't be right. Similarly, it wouldn't be right to call ${EPREFIX}/sbin/ldconfig if ${EPREFIX} belongs to a cross-prefix configuration.
(In reply to Zac Medico from comment #10) > (In reply to Benda Xu from comment #9) > > Hi portage team, > > > > Could you please review and cherry-pick commit > > dde7fb0919d40cc5585e8a603046e7098371ad69 > > In the context of the above commit, the eprefix variable refers to > config['EPREFIX']. However, as discussed in bug #582802, comment #4, we want > to use portage.const.EPREFIX in order to correctly handle cross-prefix > situations. We do want config['EPREFIX'] here (reasoning below). > Consider ROOT support, which existed before EPREFIX or cross-prefix. We've > never called ${ROOT}/sbin/ldconfig because it wouldn't be right. It wouldn't be right because if it is cross-triplet when ROOT!=/ ${ROOT}/sbin/ldconfig cannot be executed on CBUILD. The ROOT=/ ldconfig can be used because ldconfig supports -r option, which essentially does the same as sysroot of gcc/binutils. > Similarly, > it wouldn't be right to call ${EPREFIX}/sbin/ldconfig if ${EPREFIX} belongs > to a cross-prefix configuration. In cross-prefix configuration, ${EPREFIX}/sbin/ldconfig do need to be called, because: 1. cross-prefix is always a native build, i.e. ${EPREFIX}/sbin/ldconfig is always executable if exists. 2. Directories of ${EPREFIX}/etc/ld.so.conf have ${EPREFIX}. ldconfig -r ${EPREFIX} results in double prefix. ldconfig -f ${EPREFIX} leaks portage.config.EPREFIX into the cache. So during cross-prefix, there is *no way* to call portage.config.EPREFIX/sbin/ldconfig to correctly generate ld.so.cache in ${EPREFIX}. 3. Most importantly, in cross-prefix portage.config.EPREFIX may not have glibc but ${EPREFIX} has, and vice versa. In either case, testing against portage.config.EPREFIX ldconfig is incorrect. Therefore, for cross-prefix, when we want to generate ld.so.cache in ${EPREFIX}, ldconfig of ${EPREFIX} should be called.
(In reply to Benda Xu from comment #11) > Therefore, for cross-prefix, when we want to generate ld.so.cache in > ${EPREFIX}, ldconfig of ${EPREFIX} should be called. @redlizard: Are you following this? Sounds like a reasonable rationale. Technically we should use ${EROOT}/sbin/ldconfig, because for absolute paths, config['EPREFIX'] should always be joined with config['ROOT']. Perhaps we are always dealing with ROOT=/ in a cross-prefix configuration, but we should still use EROOT because it's the correct thing to do.
(In reply to Zac Medico from comment #12) > because it's the correct thing to do. It's the correct way to solve absolute paths. @redlizard: I see you're already on the prefix alias.
Created attachment 438066 [details, diff] portage-ldconfig.patch Oh yeah. eroot makes more sense than eprefix.
(In reply to Benda Xu from comment #14) > Created attachment 438066 [details, diff] [details, diff] > portage-ldconfig.patch > > Oh yeah. eroot makes more sense than eprefix. Okay, that's pretty close. However, we never want to use eroot if eprefix is empty, since in that case we might be doing a cross-compile, which means $EROOT/sbin/ldconfig might have been compiled a different target. So, we need something like this: ldconfig = os.path.join(eroot if eprefix else "/", "sbin", "ldconfig")
> 1. cross-prefix is always a native build, i.e. ${EPREFIX}/sbin/ldconfig is always executable if exists. Not true. You can have both config['EPREFIX'] != portage.const.EPREFIX and ${ROOT} != '/' at the same time, and indeed we do use a situation where ${EPREFIX} isn't (yet) executable during the prefix bootstrap. > 2. Directories of ${EPREFIX}/etc/ld.so.conf have ${EPREFIX}. ldconfig -r ${EPREFIX} results in double prefix. ldconfig -f ${EPREFIX} leaks portage.config.EPREFIX into the cache. So during cross-prefix, there is *no way* to call portage.config.EPREFIX/sbin/ldconfig to correctly generate ld.so.cache in ${EPREFIX}. Hm. Good point. We could get most of what we want with Could we use ldconfig -f -C? I haven't tried this, but I *think* ldconfig -R ${ROOT} -f ${EROOT}etc/ld.so.conf -C ${EROOT}etc/ld.so.cache should do what we want? Sorta? > 3. Most importantly, in cross-prefix portage.config.EPREFIX may not have glibc but ${EPREFIX} has, and vice versa. In either case, testing against portage.config.EPREFIX ldconfig is incorrect. This one is extra interesting because it isn't limited to prefix. How do we deal with this issue when building with ROOT? Say I'm cross compiling a glibc ROOT from an uclibc host. Now what do we do with ldconfig?
Something got messed up there, take two. > 1. cross-prefix is always a native build, i.e. > ${EPREFIX}/sbin/ldconfig is always executable if exists. Not true. You can have both config['EPREFIX'] != portage.const.EPREFIX and ${ROOT} != '/' at the same time, and indeed we do use a situation where ${EPREFIX} isn't (yet) executable during the prefix bootstrap. > 2. Directories of ${EPREFIX}/etc/ld.so.conf have ${EPREFIX}. > ldconfig -r ${EPREFIX} results in double prefix. > ldconfig -f ${EPREFIX} leaks portage.config.EPREFIX into the cache. > So during cross-prefix, there is *no way* to call > portage.config.EPREFIX/sbin/ldconfig to correctly generate > ld.so.cache in ${EPREFIX}. Hm. Good point. Could we use ldconfig -f -C? I haven't tried this, but I *think* ldconfig -R ${ROOT} -f ${EROOT}etc/ld.so.conf -C ${EROOT}etc/ld.so.cache should do what we want? Sorta? > 3. Most importantly, in cross-prefix portage.config.EPREFIX may > not have glibc but ${EPREFIX} has, and vice versa. In either case, > testing against portage.config.EPREFIX ldconfig is incorrect. This one is extra interesting because it isn't limited to prefix. How do we deal with this issue when building with ROOT? Say I'm cross compiling a glibc ROOT from an uclibc host. Now what do we do with ldconfig?
(In reply to Ruud Koolen from comment #16) > > 1. cross-prefix is always a native build, i.e. ${EPREFIX}/sbin/ldconfig is always executable if exists. > > Not true. You can have both config['EPREFIX'] != portage.const.EPREFIX and > ${ROOT} != '/' at the same time, and That's nesty, if CBUILD=CHOST and ROOT!=/, call portage.const.EPREFIX/sbin/ldconfig -r ROOT -f EROOT/etc/ld.so.conf -C EROOT/etc/ld.so.cache ? CBUILD!=CHOST call CHOST-ldconfig (bug 152828) > indeed we do use a situation where > ${EPREFIX} isn't (yet) executable during the prefix bootstrap. Which situation to be specific? > > 2. Directories of ${EPREFIX}/etc/ld.so.conf have ${EPREFIX}. ldconfig -r ${EPREFIX} results in double prefix. ldconfig -f ${EPREFIX} leaks portage.config.EPREFIX into the cache. So during cross-prefix, there is *no way* to call portage.config.EPREFIX/sbin/ldconfig to correctly generate ld.so.cache in ${EPREFIX}. > > Hm. Good point. We could get most of what we want with > > Could we use ldconfig -f -C? I haven't tried this, but I *think* ldconfig -R > ${ROOT} -f ${EROOT}etc/ld.so.conf -C ${EROOT}etc/ld.so.cache should do what > we want? Sorta? No, for example. ldconfig is from EPREFIX. Trying it on host /etc/ld.so.conf: ldconfig -v -r / -f /etc/ld.so.conf -C /tmp/ld.so.cache EPREFIX/usr/lib is (wrongly) enumerated. > > 3. Most importantly, in cross-prefix portage.config.EPREFIX may not have glibc but ${EPREFIX} has, and vice versa. In either case, testing against portage.config.EPREFIX ldconfig is incorrect. > > This one is extra interesting because it isn't limited to prefix. > > How do we deal with this issue when building with ROOT? Say I'm cross > compiling a glibc ROOT from an uclibc host. Now what do we do with ldconfig? See point 1, in that case CBUILD!=CHOST.
(In reply to Benda Xu from comment #18) > > That's nesty, if CBUILD=CHOST and ROOT!=/, call > portage.const.EPREFIX/sbin/ldconfig -r ROOT -f EROOT/etc/ld.so.conf -C > EROOT/etc/ld.so.cache ? No I am wrong ldconfig is statically linked. In this case EROOT/sbin/ldconfig -r ROOT should be called.
(In reply to Zac Medico from comment #15) > (In reply to Benda Xu from comment #14) > > Created attachment 438066 [details, diff] [details, diff] [details, diff] > > portage-ldconfig.patch > > > > Oh yeah. eroot makes more sense than eprefix. > > Okay, that's pretty close. However, we never want to use eroot if eprefix is > empty, since in that case we might be doing a cross-compile, which means > $EROOT/sbin/ldconfig might have been compiled a different target. > ldconfig = os.path.join(eroot if eprefix else "/", "sbin", "ldconfig") That is not needed. If ldconfig is for a different target, the next line in the source will catch it: ldconfig = find_binary("%s-ldconfig" % settings["CHOST"]) If ldconfig is for the same target, it can be executed since it is statically linked.
(In reply to Benda Xu from comment #20) > > ldconfig = os.path.join(eroot if eprefix else "/", "sbin", "ldconfig") > > That is not needed. If ldconfig is for a different target, the next line in > the source will catch it: > > ldconfig = find_binary("%s-ldconfig" % settings["CHOST"]) > > If ldconfig is for the same target, it can be executed since it is > statically linked. Okay, then let's make the code select the appropriate ldconfig binary up front, so that by reading the code one can see that we don't have any unintended consequences.
So it should be something like this: if "CHOST" in settings and "CBUILD" in settings and \ settings["CHOST"] != settings["CBUILD"]: ldconfig = find_binary("%s-ldconfig" % settings["CHOST"]) else: ldconfig = os.path.join(eroot, "sbin", "ldconfig")
Hi Zac, (In reply to Zac Medico from comment #22) > So it should be something like this: > > if "CHOST" in settings and "CBUILD" in settings and \ > settings["CHOST"] != settings["CBUILD"]: > ldconfig = find_binary("%s-ldconfig" % settings["CHOST"]) > else: > ldconfig = os.path.join(eroot, "sbin", "ldconfig") Thanks! That's what I want to express.
Where is this ${CHOST}-ldconfig coming from? I don't have any such tools with my cross toolchains.
(In reply to Zac Medico from comment #22) > So it should be something like this: > > if "CHOST" in settings and "CBUILD" in settings and \ > settings["CHOST"] != settings["CBUILD"]: > ldconfig = find_binary("%s-ldconfig" % settings["CHOST"]) > else: > ldconfig = os.path.join(eroot, "sbin", "ldconfig") Hi Zac, will this code snippet be merged to portage git repo?
*** Bug 587510 has been marked as a duplicate of this bug. ***
(In reply to Benda Xu from comment #25) > (In reply to Zac Medico from comment #22) > > So it should be something like this: > > > > if "CHOST" in settings and "CBUILD" in settings and \ > > settings["CHOST"] != settings["CBUILD"]: > > ldconfig = find_binary("%s-ldconfig" % settings["CHOST"]) > > else: > > ldconfig = os.path.join(eroot, "sbin", "ldconfig") > > Hi Zac, will this code snippet be merged to portage git repo? Please update your patch as suggested above. I'd like for Ruud Koolen to give a final ack before we merge it. (In reply to Ruud Koolen from comment #24) > Where is this ${CHOST}-ldconfig coming from? I don't have any such tools > with my cross toolchains. It originates from this commit for bug 152828: https://gitweb.gentoo.org/proj/portage.git/commit/?id=751893b0272561eb9274110a474d5436a7d2bc76 That was nearly a decade ago, so something may have changed since then.
Well, I'm not sure how things worked back then (what glibc version was stable around that time?), but in any case ${CHOST}-ldconfig does not appear to exist on any recent systems. So we'll need to find a different solution for this independent of prefix concerns.
Created attachment 449598 [details, diff] portage-2.3.0-ldconfig-path.patch Hi Zac, I have updated the patch according to your comments. Please consider including it.
(In reply to Benda Xu from comment #29) > Created attachment 449598 [details, diff] [details, diff] > portage-2.3.0-ldconfig-path.patch > > Hi Zac, I have updated the patch according to your comments. Please > consider including it. The patch looks good to me, but I'd feel more comfortable if the commit message described some more of the rationale. Also, I'm not sure what the "does not affect prefix-rpath" part is about. What does that mean exactly?
Created attachment 459004 [details, diff] portage-2.3.0-ldconfig-path.patch
(In reply to Zac Medico from comment #30) > The patch looks good to me, but I'd feel more comfortable if the commit > message described some more of the rationale. Also, I'm not sure what the > "does not affect prefix-rpath" part is about. What does that mean exactly? I am rewording it as env-update should call ldconfig if found in EROOT prefix-standalone has a glibc installed by portage with ldconfig under EROOT/sbin. It should be called during env-update. For prefix-rpath, host glibc is used and env-update should not care about ldconfig. In this case, no ldconfig is in EROOT/sbin and ldconfig is skipped. What do you think?
(In reply to Benda Xu from comment #32) > What do you think? Looks good. I'll merge it this weekend if somebody else doesn't do it first.
This is in the master branch: https://gitweb.gentoo.org/proj/portage.git/commit/?id=1bc49bead14ddd31c94921fe9c3d1972f0737056
(In reply to Zac Medico from comment #34) > This is in the master branch: > > https://gitweb.gentoo.org/proj/portage.git/commit/ > ?id=1bc49bead14ddd31c94921fe9c3d1972f0737056 Thanks Zac.
*** Bug 604952 has been marked as a duplicate of this bug. ***
Fixup for bug 606832: https://gitweb.gentoo.org/proj/portage.git/commit/?id=b3401efea5820c525c57a1b52c38404054593e0a
Thank you Zac, although it is late ;)
