Add an rpath to bin/python3 on glibc platforms to fix #619 #621
Conversation
Even though the Python interpreter no longer needs libpython3.x.so, it turns out some extension modules (incorrectly) do, and miraculously, allowing them to find libpython3.x.so doesn't actually break things, for reasons detailed in the comment. So set an rpath that allows libpython3.x.so to be loaded if needed by some other library, even though we won't use that ourselves. We are already doing this on musl; do it on glibc too. Note that this change does not risk users who want to make bin/python3 setuid or setcap (e.g. astral-sh#576); while the rpath is presumably ignored for privileged binaries, there is no error message, and the binary launches fine, and _because_ we do not need the rpath in order for the interpreter to work, everything (except these misbuilt extension modules) works.
5d1e377 to
f794daa
Compare
| # usually is thanks to gdb, vim, etc.), because libpython is in | ||
| # the system lib directory, as well as the behavior in practice | ||
| # on conda-forge miniconda and probably other Conda-family | ||
| # Python distributions, which too set an rpath. |
There was a problem hiding this comment.
don't think so, or I'm not seeing it? "which, also, set an rpath."
Thanks for the style feedback, I may or may not roll it into an update of this file depending on how builds go :) My overall instinct is that there's enough going on that I want things like this highly visible to the next person to think "can I change this" but yeah I could have done that differently.
(Also, oops, should have tagged this linux, doing that now before I forget just in case we have to rebuild)
There was a problem hiding this comment.
Ah I see. I got confused by the phrasing haha. Definitely understand following up separately.
| # problematic, i.e., we could load the shared library from the wrong location if | ||
| # `LD_LIBRARY_PATH` is set, but there's not a clear alternative at this time. The | ||
| # long term solution is probably to statically link to libpython instead. | ||
| # Although we are statically linking libpython, some extension |
There was a problem hiding this comment.
The column wrap on this is pretty intense given the amount of content and the presence of longer lines in the file, but 🤷♀️
Overall, this feels a little more long-form than I'd personally do here when you could just have a brief version and put the rest it in the pull request, but I don't feel strongly about the styling here.
| # version-specific library), because there is no particular | ||
| # speedup/benefit in statically linking libpython into | ||
| # libpython3.so, and we'd just be shipping a third copy of the | ||
| # libpython code. Therefore we use the old logic for that and |
There was a problem hiding this comment.
"old logic" here feels confusing in a comment, old compared to when? It seems like this will go stale.
Even though the Python interpreter no longer needs libpython3.x.so, it turns out some extension modules (incorrectly) do, and miraculously, allowing them to find libpython3.x.so doesn't actually break things, for reasons detailed in the comment. So set an rpath that allows libpython3.x.so to be loaded if needed by some other library, even though we won't use that ourselves. We are already doing this on musl; do it on glibc too.
Note that this change does not risk users who want to make bin/python3 setuid or setcap (e.g. #576); while the rpath is presumably ignored for privileged binaries, there is no error message, and the binary launches fine, and because we do not need the rpath in order for the interpreter to work, everything (except these misbuilt extension modules) works.