Skip to content

Commit

Permalink
Provide an alternative to embedded_python_tools.symlink_import()
Browse files Browse the repository at this point in the history
Until now, we've primarily been symlinking the Python dir into the build
folder. However, that has a couple of issues:
1. Occasionally the symlink goes wrong and needs to be deleted manually.
   This is especially problematic on Windows.
2. The `conanfile.py` syntax is surprising. Even more so with Conan v2
   where it requires a manual `sys.path.append()` to work.

`symlink_import()` was essentially creating a symlink from `bin/python`
to `<conan_package_path>/embedded_python`. The project executable would
point `PyConfig::home` to `bin/python`.

This commit provides an alternative that simply writes that directory
path to a file called `bin/.embedded_python.home`. The executable can
read that file on startup and point `PyConfig::home` there.

For now, both methods are valid. If the home file works out, we can
deprecate `symlink_import()` and remove it down the line.
  • Loading branch information
dean0x7d committed Jun 17, 2024
1 parent e261343 commit 2e205a5
Show file tree
Hide file tree
Showing 7 changed files with 72 additions and 55 deletions.
3 changes: 2 additions & 1 deletion changelog.md
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
# Changelog

## v1.9.1 | In development
## v1.9.1 | 2024-06-17

- Fixed an issue where calling CMake with `-DPython_EXECUTABLE=<system_python>` created conflicts with the embedded Python (either a loud version error, or silently passing the wrong library paths). Some IDEs would pass this flag implicitly and it would hijack the `find_package(Python)` call used internally by this recipe. Now, we specifically protect against this since there should be no traces of system Python in a project that wishes to embed it.
- Provided an alternative to `embedded_python_tools.symlink_import()`. For dev builds, it's now possible to point `PyConfig::home` to the contents of `bin/.embedded_python(-core).home` to avoid needing to copy the entire Python environment into the build tree every time the project is reconfigured.

## v1.9.0 | 2024-05-03

Expand Down
11 changes: 7 additions & 4 deletions core/conanfile.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,8 @@ class EmbeddedPythonCore(ConanFile):
default_options = {
"zip_stdlib": "stored",
}
exports_sources = "embedded_python_tools.py", "embedded_python-core.cmake"
exports_sources = "embedded_python_tools.py", "embedded_python*.cmake"
package_type = "shared-library"

def validate(self):
minimum_python = "3.11.5"
Expand Down Expand Up @@ -277,7 +278,7 @@ def _isolate(self, prefix):
def package(self):
src = self.build_folder
dst = pathlib.Path(self.package_folder, "embedded_python")
files.copy(self, "embedded_python-core.cmake", src, dst=self.package_folder)
files.copy(self, "embedded_python*.cmake", src, dst=self.package_folder)
files.copy(self, "embedded_python_tools.py", src, dst=self.package_folder)
license_folder = pathlib.Path(self.package_folder, "licenses")

Expand Down Expand Up @@ -321,8 +322,10 @@ def package(self):

def package_info(self):
self.env_info.PYTHONPATH.append(self.package_folder)
self.cpp_info.set_property("cmake_build_modules", ["embedded_python-core.cmake"])
self.cpp_info.build_modules = ["embedded_python-core.cmake"]
self.cpp_info.set_property(
"cmake_build_modules", ["embedded_python-core.cmake", "embedded_python-tools.cmake"]
)
self.cpp_info.build_modules = ["embedded_python-core.cmake", "embedded_python-tools.cmake"]
prefix = pathlib.Path(self.package_folder) / "embedded_python"
self.cpp_info.includedirs = [str(prefix / "include")]
if self.settings.os == "Windows":
Expand Down
15 changes: 15 additions & 0 deletions core/embedded_python-tools.cmake
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
include_guard(DIRECTORY)

# For development, we want avoid copying all of Python's `lib` and `site-packages` into our
# build tree every time we re-configure the project. Instead, we can point `PyConfig::home`
# to the contents of this file to gain access to all the Python packages.
# For release/deployment, the entire `Python_ROOT_DIR` should be copied into the app's `bin`
# folder and `PyConfig::home` should point to that.
function(embedded_python_generate_home_file filename content)
if(DEFINED CMAKE_RUNTIME_OUTPUT_DIRECTORY)
set(filename ${CMAKE_RUNTIME_OUTPUT_DIRECTORY}/${filename})
endif()
file(GENERATE OUTPUT ${filename} CONTENT "${content}")
endfunction()

embedded_python_generate_home_file(".embedded_python-core.home" "${Python_ROOT_DIR}")
29 changes: 8 additions & 21 deletions core/test_package/conanfile.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,14 @@
import subprocess
import conan
from conan import ConanFile
from conan.tools.cmake import CMake, CMakeToolchain, cmake_layout
from conan.tools.cmake import CMake, cmake_layout


# noinspection PyUnresolvedReferences
class TestEmbeddedPythonCore(ConanFile):
name = "test_embedded_python"
settings = "os", "compiler", "build_type", "arch"
generators = "CMakeDeps", "VirtualRunEnv"
generators = "CMakeToolchain", "CMakeDeps", "VirtualRunEnv"
test_type = "explicit"

def layout(self):
Expand All @@ -19,18 +19,7 @@ def layout(self):
def requirements(self):
self.requires(self.tested_reference_str)

def generate(self):
build_type = self.settings.build_type.value
tc = CMakeToolchain(self)
tc.variables[f"CMAKE_RUNTIME_OUTPUT_DIRECTORY_{build_type.upper()}"] = "bin"
tc.generate()

def build(self):
sys.path.append(str(self._core_package_path))

import embedded_python_tools

embedded_python_tools.symlink_import(self, dst="bin/python")
cmake = CMake(self)
cmake.configure(
variables={
Expand All @@ -42,20 +31,18 @@ def build(self):
)
cmake.build()

@property
def _py_exe(self):
if self.settings.os == "Windows":
return pathlib.Path(self.build_folder, "bin/python/python.exe")
else:
return pathlib.Path(self.build_folder, "bin/python/bin/python3")

@property
def _core_package_path(self):
if conan.__version__.startswith("2"):
return pathlib.Path(self.dependencies["embedded_python-core"].package_folder)
else:
return pathlib.Path(self.deps_cpp_info["embedded_python-core"].rootpath)

@property
def _py_exe(self):
exe = "python.exe" if sys.platform == "win32" else "python3"
return self._core_package_path / "embedded_python" / exe

def _test_stdlib(self):
"""Ensure that Python runs and built the optional stdlib modules"""
self.run(f'{self._py_exe} -c "import sys; print(sys.version);"')
Expand All @@ -78,7 +65,7 @@ def _test_libpython_path(self):

def _test_embed(self):
"""Ensure that everything is available to compile and link to the embedded Python"""
self.run(pathlib.Path("bin", "test_package"), env="conanrun")
self.run(pathlib.Path(self.cpp.build.bindir, "test_package").absolute(), env="conanrun")

def _test_licenses(self):
"""Ensure that the license file is included"""
Expand Down
18 changes: 17 additions & 1 deletion core/test_package/src/main.cpp
Original file line number Diff line number Diff line change
@@ -1,13 +1,29 @@
#include <Python.h>
#include <iostream>
#include <fstream>
#include <filesystem>

std::string find_python_home(std::filesystem::path bin) {
const auto local_home = bin / "python";
if (std::filesystem::exists(local_home)) {
return local_home.string();
}

auto home_file = bin / ".embedded_python.home";
if (!std::filesystem::exists(home_file)) {
home_file = bin / ".embedded_python-core.home";
}
auto stream = std::ifstream(home_file);
return std::string(std::istreambuf_iterator<char>(stream),
std::istreambuf_iterator<char>());
}

int main(int argc, const char* argv[]) {
auto config = PyConfig{};
PyConfig_InitIsolatedConfig(&config);

const auto bin = std::filesystem::path(argv[0]).parent_path();
const auto python_home = (bin / "python").string();
const auto python_home = find_python_home(bin);
if (auto status = PyConfig_SetBytesString(&config, &config.home, python_home.c_str());
PyStatus_Exception(status)) {
PyConfig_Clear(&config);
Expand Down
12 changes: 9 additions & 3 deletions embedded_python.cmake
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
include_guard(DIRECTORY)

# There is one important thing we want to achieve with the `embedded_python`/`embedded_python-core`
# split: we want to avoid recompiling the world when only the Python environment packages change
# but the version/headers/libs stay the same. To do that we must ensure that everything is built
Expand All @@ -9,9 +11,13 @@
# library. On top of that, `embedded_python.cmake` adds `EmbeddedPython_EXECUTABLE` which is aware
# of the full environment with `pip` packages. Note that we do no provide any include or lib dirs
# since those are already provided by `core`.

set(EmbeddedPython_ROOT_DIR "${CMAKE_CURRENT_LIST_DIR}/embedded_python" CACHE STRING "" FORCE)
if(WIN32)
set(EmbeddedPython_EXECUTABLE "${CMAKE_CURRENT_LIST_DIR}/embedded_python/python.exe" CACHE STRING "" FORCE)
set(EmbeddedPython_EXECUTABLE "${EmbeddedPython_ROOT_DIR}/python.exe" CACHE STRING "" FORCE)
else()
set(EmbeddedPython_EXECUTABLE "${CMAKE_CURRENT_LIST_DIR}/embedded_python/python3" CACHE STRING "" FORCE)
set(EmbeddedPython_EXECUTABLE "${EmbeddedPython_ROOT_DIR}/python3" CACHE STRING "" FORCE)
endif()

# See docstring of `embedded_python_generate_home_file()`. It's up to the user to pick if they
# want to point the `-core` package (no `pip` package) or the full embedded environment.
embedded_python_generate_home_file(".embedded_python.home" "${EmbeddedPython_ROOT_DIR}")
39 changes: 14 additions & 25 deletions test_package/conanfile.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
import subprocess
import conan
from conan import ConanFile
from conan.tools.cmake import CMake, CMakeToolchain, cmake_layout
from conan.tools.cmake import CMake, cmake_layout

project_root = pathlib.Path(__file__).parent

Expand All @@ -17,12 +17,13 @@ def _read_env(name):
class TestEmbeddedPython(ConanFile):
name = "test_embedded_python"
settings = "os", "compiler", "build_type", "arch"
generators = "CMakeDeps", "VirtualRunEnv"
generators = "CMakeToolchain", "CMakeDeps", "VirtualRunEnv"
options = {"env": [None, "ANY"]}
default_options = {
"env": None,
"embedded_python-core/*:version": "3.11.5",
}
package_type = "shared-library"

@property
def _core_package_path(self):
Expand All @@ -38,6 +39,11 @@ def _package_path(self):
else:
return pathlib.Path(self.deps_cpp_info["embedded_python"].rootpath)

@property
def _py_exe(self):
exe = "python.exe" if sys.platform == "win32" else "python3"
return self._package_path / "embedded_python" / exe

def layout(self):
cmake_layout(self)

Expand All @@ -48,19 +54,7 @@ def configure(self):
if self.options.env:
self.options["embedded_python"].packages = _read_env(self.options.env)

def generate(self):
build_type = self.settings.build_type.value
tc = CMakeToolchain(self)
tc.variables[f"CMAKE_RUNTIME_OUTPUT_DIRECTORY_{build_type.upper()}"] = "bin"
tc.generate()

def build(self):
sys.path.append(str(self._package_path))

import embedded_python_tools

embedded_python_tools.symlink_import(self, dst="bin/python")

cmake = CMake(self)
cmake.configure(
variables={
Expand All @@ -75,22 +69,17 @@ def build(self):

def _test_env(self):
"""Ensure that Python runs and finds the installed environment"""
if self.settings.os == "Windows":
python_exe = str(pathlib.Path("./bin/python/python").resolve())
else:
python_exe = str(pathlib.Path("./bin/python/bin/python3").resolve())

self.run(f'{python_exe} -c "import sys; print(sys.version);"')

self.run(f'{self._py_exe} -c "import sys; print(sys.version);"')
name = str(self.options.env) if self.options.env else "baseline"
self.run(f"{python_exe} {project_root / name / 'test.py'}", env="conanrun")
self.run(f"{self._py_exe} {project_root / name / 'test.py'}", env="conanrun")

def _test_libpython_path(self):
if self.settings.os != "Macos":
return

python_exe = str(pathlib.Path("./bin/python/bin/python3").resolve())
p = subprocess.run(["otool", "-L", python_exe], check=True, text=True, capture_output=True)
p = subprocess.run(
["otool", "-L", self._py_exe], check=True, text=True, capture_output=True
)
lines = str(p.stdout).strip().split("\n")[1:]
libraries = [line.split()[0] for line in lines]
candidates = [lib for lib in libraries if "libpython" in lib]
Expand All @@ -101,7 +90,7 @@ def _test_libpython_path(self):

def _test_embed(self):
"""Ensure that everything is available to compile and link to the embedded Python"""
self.run(pathlib.Path("bin", "test_package"), env="conanrun")
self.run(pathlib.Path(self.cpp.build.bindir, "test_package").absolute(), env="conanrun")

def _test_licenses(self):
"""Ensure that the licenses have been gathered"""
Expand Down

0 comments on commit 2e205a5

Please sign in to comment.