| Summary: | sys-libs/musl: expand ldconfig support | ||
|---|---|---|---|
| Product: | Gentoo Linux | Reporter: | Felix Janda <felix.janda> |
| Component: | [OLD] Core system | Assignee: | Anthony Basile <blueness> |
| Status: | RESOLVED FIXED | ||
| Severity: | normal | CC: | lu_zero, toolchain, ttilley |
| Priority: | Normal | ||
| Version: | unspecified | ||
| Hardware: | All | ||
| OS: | Linux | ||
| Whiteboard: | |||
| Package list: | Runtime testing required: | --- | |
| Bug Depends on: | |||
| Bug Blocks: | 430702 | ||
| Attachments: |
Proposed patch against ldconfig-0.1
Proposed patch against ldconfig-0.1 |
||
|
Description
Felix Janda
2015-03-29 22:54:55 UTC
Created attachment 400100 [details, diff]
Proposed patch against ldconfig-0.1
- Let ldconfig also read files in /lib/ld.so.conf.d/*
- Give /etc/ld-musl-$LDSO_ARCH.path the right permissions
- If there are multiple /lib/ld-musl-*.so.1 create a path file for any of them
- There should not be comments in /etc/ld-musl-$LDSO_PATH.path
- Don't error out for the unimplemented options (This needs some more thought.)
The patched ldconfig creates a functional path file at least on my system.
(In reply to Felix Janda from comment #1) > Created attachment 400100 [details, diff] [details, diff] > Proposed patch against ldconfig-0.1 > > - Let ldconfig also read files in /lib/ld.so.conf.d/* > - Give /etc/ld-musl-$LDSO_ARCH.path the right permissions > - If there are multiple /lib/ld-musl-*.so.1 create a path file for any of > them > - There should not be comments in /etc/ld-musl-$LDSO_PATH.path > - Don't error out for the unimplemented options (This needs some more > thought.) > > The patched ldconfig creates a functional path file at least on my system. Does this fix the issue I was having, namely that my ldconfig would break a system? (I thought of something like this but ran out of time to work on it.) If so, I'm wondering if we shouldn't just bundle this with the musl ebuild. I'm going to push out musl-1.1.8 without ldconfig to build the stage tarballs. I'll then test and if this works, I'll push out 1.1.8-r1 ~arch with this. Comments? + X=$(mktemp --tmpdir=/tmp ${LDSO_NAME}.XXXXXX) + for d in $drs; do + echo $d >> $X + done + mv $X $ETC_LDSO_PATH + chmod 644 $ETC_LDSO_PATH Yes, without explicitely setting the permissions, they would be 600 and dynamic linking breaks for all non-root users. Actually, I now think it's better to set the right permissions before moving in order to have an atomic upgrade. In addition, in order to get the path for libgcc, parsing the files in /etc/ld.conf.d/ is necessary. Please test! (In reply to Felix Janda from comment #3) > Yes, without explicitely setting the permissions, they would be 600 and > dynamic linking breaks for all non-root users. Actually, I now think it's > better to set the right permissions before moving in order to have an atomic > upgrade. I'm not sure the permissions was everything because I was doing this as root in a chroot env. But I could have been missing something. > > In addition, in order to get the path for libgcc, parsing the files in > /etc/ld.conf.d/ is necessary. Yes > > Please test! musl 1.1.8 brings in a servious fix so I think we need more time to test this in the wild before I begin bundling it with the musl ebuild. Created attachment 400488 [details, diff]
Proposed patch against ldconfig-0.1
A new version with a shorter diff against ldconfig-0.1:
* Don't complete ignore -f option
* Read from /etc/ld.so.conf.d/*
* Get correct dynamic linker from /bin/bash using readelf.
* Set correct permissions for the temporary file before moving it
It is safe to assume that bash is dynamically linker against musl?
(In reply to Felix Janda from comment #5) > Created attachment 400488 [details, diff] [details, diff] > Proposed patch against ldconfig-0.1 > > A new version with a shorter diff against ldconfig-0.1: > * Don't complete ignore -f option > * Read from /etc/ld.so.conf.d/* > * Get correct dynamic linker from /bin/bash using readelf. > * Set correct permissions for the temporary file before moving it > > It is safe to assume that bash is dynamically linker against musl? LDSO_PATH=$(readelf -l /bin/bash | grep -o '\/lib\/ld-musl-.*\.so\.1') 1) The system may not have bash. In all likelihood it will, but it might be the case that some POSIX purest wants just dash for instance. 2) We can't use /bin/sh since it might be a sym link to a busybox which might be statically linked. In this case $LDSO_PATH will be an empty variable and badness will ensue. 3) This approache creates a dependency on binutils which is probably okay, but you might want to be carefull. Sometimes I build little images where i eventually remove binutils/gcc (among other stuff). Of course when you do you might say, no need to run ldconfig after that, but it'll still be on the system and will produce an empty $LDSO_PATH variable again. I'm still feeling safer with my original LDSO_PATH=$(ls /lib/ld-musl-*.so.1) (In reply to Anthony Basile from comment #6). > > I'm still feeling safer with my original LDSO_PATH=$(ls /lib/ld-musl-*.so.1) Okay I pushed this without bump. I did switch to LDSO_PATH=$(ls /lib/ld-musl-*.so.1) but I also have LDSO_PATH=$(readelf -l /bin/bash | grep -o '\/lib\/ld-musl-.*\.so\.1') in there but commented out, for testing. I'm going to do a full rebuild of @system in a chroot on amd64 to see how this goes. (In reply to Felix Janda from comment #5) > * Read from /etc/ld.so.conf.d/* Actually my original script had that with expanded(). This is redundant: for f in $LDSO_CONF.d/*; do drs="$drs $(read_ldso_conf "$f")" done I'm testing that now. Another point to note is that effectively this script is `ldconfig -X` since it does not update sym links to the most recent .so's, and of course doesn't deal with -N + -X logic. This is sufficient for portage, but it may not be what the user expects. So i'm going to change the getopts to print out an warning if -X is missing, not if its present. (In reply to Anthony Basile from comment #8) > (In reply to Felix Janda from comment #5) > > > * Read from /etc/ld.so.conf.d/* > > Actually my original script had that with expanded(). This is redundant: > > for f in $LDSO_CONF.d/*; do > drs="$drs $(read_ldso_conf "$f")" > done > > I'm testing that now. If you remove this, hell will break loose because libgcc_s.so will not be found anymore. (Unless I understand you wrong.) This is very likely exactly the problem you had with the original script. Also, when including an ldconfig with the musl ebuild we make it tailored for the $LDSO_ARCH. (In reply to Felix Janda from comment #10) > (In reply to Anthony Basile from comment #8) > > (In reply to Felix Janda from comment #5) > > > > > * Read from /etc/ld.so.conf.d/* > > > > Actually my original script had that with expanded(). This is redundant: > > > > for f in $LDSO_CONF.d/*; do > > drs="$drs $(read_ldso_conf "$f")" > > done > > > > I'm testing that now. > > If you remove this, hell will break loose because libgcc_s.so will not be > found anymore. (Unless I understand you wrong.) This is very likely exactly > the problem you had with the original script. The problem is that env-update is not including a line that looks like include ld.so.conf.d/*.conf when generating the ld.so.conf file. This is the problem, not the ldconfig script as written. Notice what this does: next=0 for l in $line; do if [[ $next == 1 ]]; then next=0 drs=$(expand $l $drs) elif [[ $l == "include" ]]; then next=1 else if [[ -d $l ]]; then repeated $l $drs && continue drs+=" $l " fi fi done When "include" is encountered, it skips it and set 'next=1' and then on the next token runs expand() which reads ld.so.conf.d/*.conf. If that line were in the ld.so.conf file, it would work but right now env-update is not adding it. So this tells me something needs to be fixed in env-update, not the ldconfig script. Here's what happens when you run it manually: yellow ~ # cat /etc/ld.so.confy /lib /usr/lib /usr/local/lib include ld.so.conf.d/*.conf yellow ~ # ls /etc/ld.so.conf.d/ 05binutils.conf 05gcc-x86_64-gentoo-linux-musl.conf yellow ~ # ldconfig yellow ~ # cat /etc/ld-musl-x86_64.path /lib /usr/lib /usr/local/lib /usr/x86_64-gentoo-linux-musl/lib /usr/lib/gcc/x86_64-gentoo-linux-musl/4.8.4 as expected. However, if you run env-update, here's what the ld.so.conf file looks like: yellow ~ # env-update >>> Regenerating /etc/ld.so.cache... yellow ~ # cat /etc/ld.so.conf # ld.so.conf autogenerated by env-update; make all changes to # contents of /etc/env.d directory /lib /usr/lib /usr/local/lib Note the include ld.so.conf.d/*.conf was removed. Running ldconfig now misses stuff the stuff from ld.so.conf.d/*.conf and we get the failure. Adding for f in $LDSO_CONF.d/*; do drs="$drs $(read_ldso_conf "$f")" done is wrong because the include line point to something other than $LDSO_CONF.d/*. We must reproduce the original ldconfig as best possible so that we don't break standards and confuse users. (In reply to Anthony Basile from comment #12) > is wrong because the include line point to something other than > $LDSO_CONF.d/*. We must reproduce the original ldconfig as best possible so > that we don't break standards and confuse users. sorry that should read "the include line *may* point to something other than" Thanks for explaining. (I somehow had the impression /etc/ld.so.conf.d was standard.) I agree that this seems to be a problem of env-update. Usually there is 00glibc generated by sys-libs/glibc/files/eblits/src_install.eblit with the line 'LDPATH="include ld.so.conf.d/*.conf'" but other tools like gcc-config also assume that ld.so.conf.d/*.conf are included. So the 'LDPATH="include ld.so.conf.d/*.conf'" should better be in 00basic. (In reply to Felix Janda from comment #14) not every C library supports ld.so.conf.d, so each C lib has to opt into it. putting it into 00basic would be wrong. gcc-config & binutils-config are explicitly designed to support both the dir and the flat file. if the dir exists, they assume the system supports it. this isn't a bug on their part. I see, e.g. uclibc's ldconfig does not support include statements. So we should create an /etc/env.d/00musl. (Or rip out the include statement handling to be more like uclibc...) Small fix: --- a/ldconfig-0.1 2015-04-06 16:35:30.000000000 -0200 +++ b/ldconfig-0.1 2015-04-06 16:35:39.217604221 -0200 @@ -29,7 +29,7 @@ echo "Invalid option: -$opt" >&2 exit 1 ;; - n|N|X|C) + n|N|X|C|p) echo "Unimplemented option: -$opt" >&2 exit 1 ;; (In reply to Felix Janda from comment #16) > I see, e.g. uclibc's ldconfig does not support include statements. > > So we should create an /etc/env.d/00musl. > (Or rip out the include statement handling to be more like uclibc...) I went with this option. I like the include option. This is now in the tree with 1.1.8-r1 and 9999. I also coalesced sys-app/getent with musl. I think it was a bad idea to break it out into its own packages. Please test and reopen this bug if there's some issue. (In reply to Anthony Basile from comment #19) getent is also a dedicated script in uClibc. we could think about creating a common project on github or something for it. (In reply to SpanKY from comment #20) > (In reply to Anthony Basile from comment #19) > > getent is also a dedicated script in uClibc. we could think about creating > a common project on github or something for it. yep that's where i stole i from. i did break out a copy which i'm hosting on my dev space so moving it to github might be a good idea, to give it more public exposure now that its being used in two libc's. i still think keeping it as a separate package on the gentoo tree is excessive. its a whole package for one file. so in musl-1.1.8-r1 its being distributed in ${FILESDIR} and i plan to eventually mask/remove sys-apps/getent. if we do put it on github, we can just add the url to the SRC_URI. unless you think keeping it a separate package is a good idea, I'll eventually remove sys-apps/getent. (In reply to Anthony Basile from comment #21) i just worry about forking. maybe fetch it directly from uClibc's git ? (In reply to SpanKY from comment #22) > (In reply to Anthony Basile from comment #21) > > i just worry about forking. maybe fetch it directly from uClibc's git ? yeah i agree about the forking. i'll come up with something where we stay in sync with uclibc automatically. |
