Skip to content

Commit

Permalink
Fix runtime conflict between embedded 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 Oct 2, 2023
1 parent 3b01d0d commit 250377b
Show file tree
Hide file tree
Showing 5 changed files with 115 additions and 18 deletions.
3 changes: 2 additions & 1 deletion changelog.md
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
# Changelog

## v1.8.1 | In development
## v1.8.1 | 2023-10-02

- 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
37 changes: 24 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,26 @@ 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. `-E -s`
gets us close enough to isolated mode without breaking the installation of old packages.
"""
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 +191,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 +201,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
63 changes: 59 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,57 @@ 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.
Note that `._pth == isolated_mode` is only the case when running Python via the `python(3)`
executable. When embedding into an application executable, the `._pth` file is not relevant.
Isolated mode is set via the C API: https://docs.python.org/3/c-api/init_config.html While
embedding in the app is the primary use case, running the `python(3)` exe is also useful
for various build and runtime tasks. It's important to maintain isolated mode in all cases
to avoid obscure, hard-to-debug issues.
Finally, both `-core` and regular variants of this recipe will have the `._pth` file in the
package. All installed `pip` packages work correctly at runtime in isolated mode. However,
some older packages cannot be installed in isolated mode (they are using outdated `setup.py`
conventions). For this reason, we temporarily delete the `._pth` file and fall back to
partial isolation while installing `pip` packages. See `_build_bootstrap()` for details.
"""
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 +302,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 250377b

Please sign in to comment.