From 250377b3ffc36e4cbce3442e0823a3e470fd8714 Mon Sep 17 00:00:00 2001 From: Dean Moldovan Date: Thu, 21 Sep 2023 13:15:16 +0200 Subject: [PATCH] Fix runtime conflict between embedded and system Python 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. --- changelog.md | 3 +- conanfile.py | 37 ++++++++++++-------- core/conanfile.py | 63 ++++++++++++++++++++++++++++++++--- core/test_package/test.py | 15 +++++++++ test_package/baseline/test.py | 15 +++++++++ 5 files changed, 115 insertions(+), 18 deletions(-) diff --git a/changelog.md b/changelog.md index 9d61672..895c273 100644 --- a/changelog.md +++ b/changelog.md @@ -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. diff --git a/conanfile.py b/conanfile.py index 3b63257..72c8af0 100644 --- a/conanfile.py +++ b/conanfile.py @@ -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" @@ -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}", @@ -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, @@ -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) @@ -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) diff --git a/core/conanfile.py b/core/conanfile.py index 414389e..8b4c86a 100644 --- a/core/conanfile.py +++ b/core/conanfile.py @@ -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", @@ -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 @@ -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}") @@ -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") @@ -249,6 +302,7 @@ 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 @@ -256,6 +310,7 @@ def package(self): 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). diff --git a/core/test_package/test.py b/core/test_package/test.py index 1b6d28e..078df7f 100644 --- a/core/test_package/test.py +++ b/core/test_package/test.py @@ -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) diff --git a/test_package/baseline/test.py b/test_package/baseline/test.py index 1b6d28e..078df7f 100644 --- a/test_package/baseline/test.py +++ b/test_package/baseline/test.py @@ -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)