-
-
Notifications
You must be signed in to change notification settings - Fork 244
POC: Add a relocatable build flag #861
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
63daa7f to
c2e856d
Compare
c2e856d to
d715fa3
Compare
geofft
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately I think there's a good amount of stuff missing compared to our current implementation, and this is a hairy enough spot that I don't think we should have any regressions in 3.15 compared to what we do on older versions.
On glibc/Linux, we add a DT_NEEDED for bin/python to $ORIGIN/../lib/libpython3.x.so.1.0 (and for lib/libpython3.so to $ORIGIN/libpython3.x.so.1.0) , to avoid issues with LD_LIBRARY_PATH taking precedence. I'm relatively sure this is annoying to do without a separate patchelf step, but maybe it can be done.
We also set an DT_RPATH for bin/python on Linux (required on musl, compatibility on glibc). This is not a DT_RUNPATH, so this needs -Wl,--disable-new-dtags.
I don't see either of those in the Makefile diff, in addition to the other things noted in the comments.
If you really wanted, you could start by upstreaming this gated to macOS only, which might be a little less code to write.
Also speaking of platform support, the dynamic linkers on the BSDs and Solaris works much like glibc, so most of our Linux support should apply there too in the version that gets upstreamed.
In either case, I would error out if --enable-relocatable is passed on another platform.
| Author: Zanie Blue <[email protected]> | ||
| Date: Fri Nov 21 15:44:42 2025 -0600 | ||
|
|
||
| Set linker arguments when `--enabled-relocatable` is used |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| Set linker arguments when `--enabled-relocatable` is used | |
| Set linker arguments when `--enable-relocatable` is used |
| -change /install/lib/${LIBPYTHON_SHARED_LIBRARY_BASENAME} @executable_path/${LIBPYTHON_SHARED_LIBRARY_BASENAME} \ | ||
| ${ROOT}/out/python/install/lib/${LIBPYTHON_SHARED_LIBRARY_BASENAME} | ||
|
|
||
| # We also normalize /tools/deps/lib/libz.1.dylib to the system location. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How come you didn't need to preserve this?
| # At the moment, python3 and libpython don't have shared-library | ||
| # dependencies, but at some point we will want to run this for | ||
| # them too. | ||
| for module in ${ROOT}/out/python/install/lib/python*/lib-dynload/*.so; do | ||
| install_name_tool -add_rpath @loader_path/../.. "$module" | ||
| done |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This almost certainly needs to be preserved, this fixes up _tkinter, right? That is, you need this to go into the CPython Makefile rule for building shared-library extension modules (... which we might not be using, to make things more exciting).
| @@ -1025,10 +1025,19 @@ libpython$(LDVERSION).so: $(LIBRARY_OBJS) $(DTRACE_OBJS) | ||
| fi | ||
|
|
||
| libpython3.so: libpython$(LDVERSION).so |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You need to patch the libpython$(LDVERSION).so target too. (LINKFORSHARED is only used when building the interpreter.)
The goal here is to construct two upstreamable patches
--enable-relocatableconfigure flag to gate new behaviorAs a PoC, we apply these patches here for Python 3.15 and skip our post-build relocatability patchelf invocations.