Skip to content

GH-128469: set RPATH on the builddir Python executable #128470

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.

Sign up for GitHub

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

Closed
wants to merge 1 commit into from
Closed

GH-128469: set RPATH on the builddir Python executable #128470

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 12 additions & 5 deletions Makefile.pre.in
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -301,6 +301,8 @@ MACHDEP_OBJS= @MACHDEP_OBJS@
LIBOBJDIR= Python/
LIBOBJS= @LIBOBJS@

BUILDPYTHON_LDFLAGS= @BUILDPYTHON_LDFLAGS@

PYTHON= python$(EXE)
BUILDPYTHON= python$(BUILDEXE)

Expand Down Expand Up @@ -727,7 +729,7 @@ list-targets:
@grep -E '^[A-Za-z][-A-Za-z0-9]+:' Makefile | awk -F : '{print $$1}'

.PHONY: build_all
build_all: check-clean-src check-app-store-compliance $(BUILDPYTHON) platform sharedmods \
build_all: check-clean-src check-app-store-compliance $(BUILDPYTHON) python-bin platform sharedmods \
gdbhooks Programs/_testembed scripts checksharedmods rundsymutil

.PHONY: build_wasm
Expand Down Expand Up @@ -911,9 +913,14 @@ clinic: check-clean-src
clinic-tests: check-clean-src $(srcdir)/Lib/test/clinic.test.c
$(PYTHON_FOR_REGEN) $(srcdir)/Tools/clinic/clinic.py -f $(srcdir)/Lib/test/clinic.test.c

# Build the interpreter
# Build the builddir interpreter (with RPATH to the local libpython)
$(BUILDPYTHON): Programs/python.o $(LINK_PYTHON_DEPS)
$(LINKCC) $(PY_CORE_LDFLAGS) $(LINKFORSHARED) -o $@ Programs/python.o $(LINK_PYTHON_OBJS) $(LIBS) $(MODLIBS) $(SYSLIBS)
$(LINKCC) $(PY_CORE_LDFLAGS) $(LINKFORSHARED) $(BUILDPYTHON_LDFLAGS) -o $@ Programs/python.o $(LINK_PYTHON_OBJS) $(LIBS) $(MODLIBS) $(SYSLIBS)

# Build the interpreter binary to install
.PHONY: python-bin
python-bin: Programs/python.o $(LINK_PYTHON_DEPS) pybuilddir.txt
Copy link
Contributor

@eli-schwartz eli-schwartz Jan 8, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Allowing the software to be runnable from the build directory is an overall great idea. There's two common ways to do it:

  • build / link it twice. Once with an rpath for running uninstalled, once without an rpath for installing.
  • rewrite the binary at install time to remove the unsafe path

This PR implements the former approach, which has the advantage of being simple to implement. The downside is a lack of "purity" -- you don't run tests with what you install, but rather with something you reasonably hope is effectively the same. It also requires relinking, which can be slow if using LTO.

I'm a fan of the latter approach. It's a pity that there's no standard tooling for it. chrpath/patchelf can be used I guess but may not be installed, plus you can end up with slightly odd ELF header reordering that has had occasional very bad bugs (patchelf does a lot of sorcery that chrpath doesn't, and in general extending an rpath rather than truncating it can be risky).

Meson has a pure python, stdlib-only implementation of an ELF rpath rewriter which uses simple truncation, which might help: https://github.com/mesonbuild/meson/blob/master/mesonbuild/scripts/depfixer.py

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not clear to me what problem this issue / PR is trying to solve that isn't addressed by the existing RUNSHARED configuration variable. Can you elaborate?

In any case, doing two links just to address the apparent edge case of simplifying running from the build directory without installing seems overkill and counter to the goal of reproducible builds.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not clear to me what problem this issue / PR is trying to solve that isn't addressed by the existing RUNSHARED configuration variable. Can you elaborate?

Running python directly from the build directory, which is a big part of the development process.

In any case, doing two links just to address the apparent edge case of simplifying running from the build directory without installing seems overkill and counter to the goal of reproducible builds.

Yeah, I understand. With GH-127972 merged it becomes simple to show a warning in getpath if the library is loaded from the system instead of the build directory, so I am gonna go for that approach instead.

$(LINKCC) $(PY_CORE_LDFLAGS) $(LINKFORSHARED) -o `cat pybuilddir.txt`/$(PYTHON) Programs/python.o $(LINK_PYTHON_OBJS) $(LIBS) $(MODLIBS) $(SYSLIBS)

platform: $(PYTHON_FOR_BUILD_DEPS) pybuilddir.txt
$(RUNSHARED) $(PYTHON_FOR_BUILD) -c 'import sys ; from sysconfig import get_platform ; print("%s-%d.%d" % (get_platform(), *sys.version_info[:2]))' >platform
Expand Down Expand Up @@ -2264,7 +2271,7 @@ sharedinstall: all
# Install the interpreter with $(VERSION) affixed
# This goes into $(exec_prefix)
.PHONY: altbininstall
altbininstall: $(BUILDPYTHON) @FRAMEWORKPYTHONW@
altbininstall: python-bin $(BUILDPYTHON) @FRAMEWORKPYTHONW@
@for i in $(BINDIR) $(LIBDIR); \
do \
if test ! -d $(DESTDIR)$$i; then \
Expand All @@ -2274,7 +2281,7 @@ altbininstall: $(BUILDPYTHON) @FRAMEWORKPYTHONW@
fi; \
done
if test "$(PYTHONFRAMEWORKDIR)" = "no-framework" ; then \
$(INSTALL_PROGRAM) $(BUILDPYTHON) $(DESTDIR)$(BINDIR)/python$(LDVERSION)$(EXE); \
$(INSTALL_PROGRAM) `cat pybuilddir.txt`/$(PYTHON) $(DESTDIR)$(BINDIR)/python$(LDVERSION)$(EXE); \
else \
$(INSTALL_PROGRAM) $(STRIPFLAG) Mac/pythonw $(DESTDIR)$(BINDIR)/python$(LDVERSION)$(EXE); \
fi
Expand Down
34 changes: 10 additions & 24 deletions config.sub
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,10 @@
#! /bin/sh
# Configuration validation subroutine script.
# Copyright 1992-2024 Free Software Foundation, Inc.
# Copyright 1992-2023 Free Software Foundation, Inc.

# shellcheck disable=SC2006,SC2268 # see below for rationale

# Patched 2024-02-03 to include support for arm64_32 and iOS/tvOS/watchOS simulators
timestamp='2024-01-01'
timestamp='2023-09-19'

# This file is free software; you can redistribute it and/or modify it
# under the terms of the GNU General Public License as published by
Expand Down Expand Up @@ -77,7 +76,7 @@ Report bugs and patches to <config-patches@gnu.org>."
version="\
GNU config.sub ($timestamp)

Copyright 1992-2024 Free Software Foundation, Inc.
Copyright 1992-2023 Free Software Foundation, Inc.

This is free software; see the source for copying conditions. There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE."
Expand Down Expand Up @@ -1128,7 +1127,7 @@ case $cpu-$vendor in
xscale-* | xscalee[bl]-*)
cpu=`echo "$cpu" | sed 's/^xscale/arm/'`
;;
arm64-* | aarch64le-* | arm64_32-*)
arm64-* | aarch64le-*)
cpu=aarch64
;;

Expand Down Expand Up @@ -1223,7 +1222,6 @@ case $cpu-$vendor in
| moxie \
| mt \
| msp430 \
| nanomips* \
| nds32 | nds32le | nds32be \
| nfp \
| nios | nios2 | nios2eb | nios2el \
Expand Down Expand Up @@ -1255,7 +1253,6 @@ case $cpu-$vendor in
| ubicom32 \
| v70 | v850 | v850e | v850e1 | v850es | v850e2 | v850e2v3 \
| vax \
| vc4 \
| visium \
| w65 \
| wasm32 | wasm64 \
Expand Down Expand Up @@ -1600,7 +1597,7 @@ case $cpu-$vendor in
os=
obj=elf
;;
mips*-*|nanomips*-*)
mips*-*)
os=
obj=elf
;;
Expand Down Expand Up @@ -1724,7 +1721,7 @@ fi

case $os in
# Sometimes we do "kernel-libc", so those need to count as OSes.
llvm* | musl* | newlib* | relibc* | uclibc*)
musl* | newlib* | relibc* | uclibc*)
;;
# Likewise for "kernel-abi"
eabi* | gnueabi*)
Expand Down Expand Up @@ -1769,19 +1766,12 @@ case $os in
| onefs* | tirtos* | phoenix* | fuchsia* | redox* | bme* \
| midnightbsd* | amdhsa* | unleashed* | emscripten* | wasi* \
| nsk* | powerunix* | genode* | zvmoe* | qnx* | emx* | zephyr* \
| fiwix* | mlibc* | cos* | mbr* | ironclad* )
| fiwix* | mlibc* | cos* | mbr* )
;;
# This one is extra strict with allowed versions
sco3.2v2 | sco3.2v[4-9]* | sco5v6*)
# Don't forget version if it is 3.2v4 or newer.
;;
# This refers to builds using the UEFI calling convention
# (which depends on the architecture) and PE file format.
# Note that this is both a different calling convention and
# different file format than that of GNU-EFI
# (x86_64-w64-mingw32).
uefi)
;;
none)
;;
kernel* | msvc* )
Expand Down Expand Up @@ -1828,18 +1818,16 @@ esac
# As a final step for OS-related things, validate the OS-kernel combination
# (given a valid OS), if there is a kernel.
case $kernel-$os-$obj in
linux-gnu*- | linux-android*- | linux-dietlibc*- | linux-llvm*- \
| linux-mlibc*- | linux-musl*- | linux-newlib*- \
| linux-relibc*- | linux-uclibc*- )
linux-gnu*- | linux-dietlibc*- | linux-android*- | linux-newlib*- \
| linux-musl*- | linux-relibc*- | linux-uclibc*- | linux-mlibc*- )
;;
uclinux-uclibc*- )
;;
managarm-mlibc*- | managarm-kernel*- )
;;
windows*-msvc*-)
;;
-dietlibc*- | -llvm*- | -mlibc*- | -musl*- | -newlib*- | -relibc*- \
| -uclibc*- )
-dietlibc*- | -newlib*- | -musl*- | -relibc*- | -uclibc*- | -mlibc*- )
# These are just libc implementations, not actual OSes, and thus
# require a kernel.
echo "Invalid configuration '$1': libc '$os' needs explicit kernel." 1>&2
Expand Down Expand Up @@ -1867,8 +1855,6 @@ case $kernel-$os-$obj in
;;
*-eabi*- | *-gnueabi*-)
;;
ios*-simulator- | tvos*-simulator- | watchos*-simulator- )
;;
none--*)
# None (no kernel, i.e. freestanding / bare metal),
# can be paired with an machine code file format
Expand Down
93 changes: 51 additions & 42 deletions configure
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading
Loading