Skip to content

Commit

Permalink
Fix runtime conflict between bootstrap and system Python
Browse files Browse the repository at this point in the history
The issue was that our embedded Python could pick up extra Python
packages installed in the user's home directory. Python adds user
paths by default unless instructed otherwise. The `._pth` file puts
it into isolated mode. See the docstrings of `_isolate()` for all
the details.
  • Loading branch information
dean0x7d committed Sep 22, 2023
1 parent db0621f commit 8682c6e
Show file tree
Hide file tree
Showing 5 changed files with 100 additions and 17 deletions.
1 change: 1 addition & 0 deletions changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

## v1.8.1 | In development

- Fixed packaging and runtime errors caused by conflicts with an incompatible system Python installation or `pip` packages installed in the user's home directory. The embedded Python now always run in isolated mode regardless of command line flags.
- Fixed packaging error on Windows when the Conan cache path contains spaces.
- Fixed Python include dirs being added twice (didn't cause any issues, just noise on the command line).
- Fixed `openssl` v3 mistakenly being enabled for Python 3.10. While 3.10 has preliminary support for `openssl` v3, Python 3.11 is the real minimum requirement for full support.
Expand Down
36 changes: 23 additions & 13 deletions conanfile.py
Original file line number Diff line number Diff line change
Expand Up @@ -128,8 +128,10 @@ def _build_bootstrap(self):
bootstrap = pathlib.Path(self.build_folder) / "bootstrap"
files.copy(self, "*", src=self.core_pkg / "embedded_python", dst=bootstrap)

if self.settings.os == "Windows":
# Deleting the ._pth file restores regular (non-embedded) module path rules
# Deleting the ._pth file restores regular (non-embedded) module path rules
if self.settings.os != "Windows":
os.remove(bootstrap / f"python{self.short_pyversion}._pth")
else:
os.remove(bootstrap / f"python{self.int_pyversion}._pth")
# Moving files to the `DLLs` folder restores non-embedded folder structure
dlls = bootstrap / "DLLs"
Expand All @@ -138,7 +140,7 @@ def _build_bootstrap(self):
file.rename(dlls / file.name)
# We need pip to install packages
files.download(self, "https://bootstrap.pypa.io/get-pip.py", filename="get-pip.py")
self.run(f"{self.bootstrap_py_exe} get-pip.py")
self._run_bootstrap_py("get-pip.py")

specs = [
f"pip=={self.options.pip_version}",
Expand All @@ -147,12 +149,25 @@ def _build_bootstrap(self):
f"pip-licenses=={self.options.pip_licenses_version}",
]
options = "--no-warn-script-location --upgrade"
self.run(f"{self.bootstrap_py_exe} -m pip install {options} {' '.join(specs)}")
self._run_bootstrap_py(f"-m pip install {options} {' '.join(specs)}")

def _run_bootstrap_py(self, command, **kwargs):
"""Run `command` with the Python created by `_build_bootstrap`
While we do need to mostly restore regular module path rules for the bootstrap, we still
don't want to get conflicts with packages installed in the user's home directory. We can
disable those via env variable. Again, this is only for bootstrapping. The final package
will be fully isolated via the `._pth` file.
Here, we can't use `-I` because that also removes the current script directory from the
path which is a problem for older packages with outdated `setup.py` conventions.
"""
self.run(f"{self.bootstrap_py_exe} -E -s {command}", **kwargs)

def _gather_licenses(self, license_folder):
"""Gather licenses for all packages using our bootstrap environment"""
self.run(
f"{self.bootstrap_py_exe} -m piplicenses --python={self.package_py_exe}"
self._run_bootstrap_py(
f"-m piplicenses --python={self.package_py_exe}"
" --with-system --from=mixed --format=plain-vertical"
" --with-license-file --no-license-path --output-file=package_licenses.txt",
cwd=license_folder,
Expand All @@ -175,12 +190,6 @@ def build(self):
def package(self):
files.copy(self, "embedded_python.cmake", src=self.build_folder, dst=self.package_folder)
files.copy(self, "embedded_python*", src=self.core_pkg, dst=self.package_folder)
prefix = pathlib.Path(self.package_folder, "embedded_python")
if self.settings.os == "Windows":
# Enable site-packages, i.e. additional non-system packages
target = prefix / f"python{self.int_pyversion}._pth"
files.replace_in_file(self, target, "#import site", "import site")

license_folder = pathlib.Path(self.package_folder, "licenses")
files.copy(self, "LICENSE.txt", src=self.core_pkg / "licenses", dst=license_folder)

Expand All @@ -191,8 +200,9 @@ def package(self):
requirements = self._make_requirements_file(
extra_packages=[f"setuptools=={self.options.setuptools_version}"]
)
prefix = pathlib.Path(self.package_folder, "embedded_python")
options = f'--no-deps --ignore-installed --no-warn-script-location --prefix "{prefix}"'
self.run(f"{self.bootstrap_py_exe} -m pip install {options} -r {requirements}")
self._run_bootstrap_py(f"-m pip install {options} -r {requirements}")
self._gather_licenses(license_folder)
self._gather_packages(license_folder)

Expand Down
50 changes: 46 additions & 4 deletions core/conanfile.py
Original file line number Diff line number Diff line change
Expand Up @@ -96,9 +96,11 @@ def generate(self):
url = f"https://github.com/python/cpython/archive/v{self.pyversion}.tar.gz"
files.get(self, url, strip_root=True)

tc = AutotoolsToolchain(self, prefix=pathlib.Path(self.package_folder, "embedded_python"))
prefix = pathlib.Path(self.package_folder, "embedded_python")
tc = AutotoolsToolchain(self, prefix=prefix)
openssl_path = self.dependencies["openssl"].package_folder
tc.configure_args += [
f"--bindir={prefix}", # see `_isolate()` for the reason why we override this path
"--enable-shared",
"--without-static-libpython",
"--disable-test-modules",
Expand All @@ -114,7 +116,7 @@ def generate(self):
# package. Unlike RUNPATH, RPATH takes precedence over LD_LIBRARY_PATH.
if self.settings.os == "Linux":
deps.environment.append(
"LDFLAGS", [r"-Wl,-rpath='\$\$ORIGIN/../lib'", "-Wl,--disable-new-dtags"]
"LDFLAGS", [r"-Wl,-rpath='\$\$ORIGIN/lib'", "-Wl,--disable-new-dtags"]
)

# Statically linking CPython with OpenSSL requires a bit of extra care. See the discussion
Expand Down Expand Up @@ -151,14 +153,14 @@ def _patch_libpython_path(self, dst):
if self.settings.os != "Macos":
return

exe = dst / f"bin/python{self.short_pyversion}"
exe = dst / f"python{self.short_pyversion}"
buffer = io.StringIO()
self.run(f"otool -L {exe}", output=buffer)
lines = buffer.getvalue().strip().split("\n")[1:]
libraries = [line.split()[0] for line in lines]
hardcoded_libraries = [lib for lib in libraries if lib.startswith(str(dst))]
for lib in hardcoded_libraries:
relocatable_library = lib.replace(str(dst), "@executable_path/..")
relocatable_library = lib.replace(str(dst), "@executable_path")
self.output.info(f"Patching {exe}, replace {lib} with {relocatable_library}")
self.run(f"install_name_tool -change {lib} {relocatable_library} {exe}")

Expand Down Expand Up @@ -230,6 +232,44 @@ def is_landmark(filepath):
elif path.is_dir() and path.name not in keep_lib_dirs:
shutil.rmtree(path)

def _isolate(self, prefix):
"""Isolate this embedded environment from any other Python installations
Creating a `._pth` file puts Python into isolated mode: it will ignore any `PYTHON*`
env variables or additional packages installed in the users home directory. Only
the paths listed in the `._pth` file will be in `sys.path` on startup.
There's an extra quirk that complicates things on non-Windows systems. The `._pth` file
must be in the same directory as the real (non-symlink) executable, but it also must be
in the home/prefix directory. Usually, the executable is in `prefix/bin`. This forces us
to move the executable to `prefix` (this is done in `generate()`). To avoid issues with
established Unix Python conventions, we put symlinks back into `prefix/bin`. This is not
an issue on Windows since it already has `bin == prefix` by default.
"""
if self.settings.os == "Windows":
paths = [
f"python{self.int_pyversion}.zip",
".",
"Lib/site-packages",
]
# `.pth` file must be next to the main `.dll` and use the same name.
with open(prefix / f"python{self.int_pyversion}._pth", "w") as f:
f.write("\n".join(paths))
else:
paths = [
f"lib/python{self.int_pyversion}.zip",
f"lib/python{self.short_pyversion}",
f"lib/python{self.short_pyversion}/lib-dynload",
f"lib/python{self.short_pyversion}/site-packages",
]
# `.pth` file must be next to real (non-symlink) executable and use the same name.
with open(prefix / f"python{self.short_pyversion}._pth", "w") as f:
f.write("\n".join(paths))

py_exe = f"python{self.short_pyversion}"
os.symlink(f"../{py_exe}", prefix / f"bin/{py_exe}")
os.symlink(f"../{py_exe}", prefix / f"bin/python3")

def package(self):
src = self.build_folder
dst = pathlib.Path(self.package_folder, "embedded_python")
Expand All @@ -249,13 +289,15 @@ def package(self):
files.rmdir(self, "tmp")
files.rm(self, "dev.msi", dst)

self._isolate(dst)
files.copy(self, "LICENSE.txt", src=dst, dst=license_folder)
else:
from conan.tools.gnu import Autotools

autotools = Autotools(self)
autotools.install(args=["DESTDIR=''"]) # already handled by AutotoolsToolchain prefix
self._patch_libpython_path(dst)
self._isolate(dst)

# Give write permissions, otherwise end-user projects won't be able to re-import
# the shared libraries (re-import happens on subsequent `conan install` runs).
Expand Down
15 changes: 15 additions & 0 deletions core/test_package/test.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,18 @@
import zlib

print("All optional Python features are importable")

import sys
import site

if sys.version_info[:2] >= (3, 11):
assert sys.flags.isolated == 1
assert sys.flags.ignore_environment == 1
assert not site.ENABLE_USER_SITE

print("sys.path:")
for p in sys.path:
print("-", p)

# The environment is isolated so only internal paths should be here
assert all("embedded_python" in p for p in sys.path)
15 changes: 15 additions & 0 deletions test_package/baseline/test.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,18 @@
import zlib

print("All optional Python features are importable")

import sys
import site

if sys.version_info[:2] >= (3, 11):
assert sys.flags.isolated == 1
assert sys.flags.ignore_environment == 1
assert not site.ENABLE_USER_SITE

print("sys.path:")
for p in sys.path:
print("-", p)

# The environment is isolated so only internal paths should be here
assert all("embedded_python" in p for p in sys.path)

0 comments on commit 8682c6e

Please sign in to comment.