From 979e81e74c57c53859c9f474ee2855be52539f03 Mon Sep 17 00:00:00 2001 From: Dean Moldovan Date: Thu, 13 Jun 2024 15:51:35 +0200 Subject: [PATCH] Fix build issues due to IDEs overriding the `Python_EXECUTABLE` path Recent versions of CLion and some VScode extensions has started implicitly passing `-DPython_EXECUTABLE=` when configuring CMake. This ends up overriding `find_package(Python)` with bad results. The fix is to no allow this by `FORCE` setting `Python_EXECUTABLE`. See the changelog entry and code comments for more details on the issue and fix. --- changelog.md | 4 ++++ conanfile.py | 4 ++-- core/conanfile.py | 2 +- core/embedded_python-core.cmake | 16 ++++++++++------ core/test_package/conanfile.py | 9 ++++++++- embedded_python.cmake | 4 ++-- test_package/conanfile.py | 3 +++ 7 files changed, 30 insertions(+), 12 deletions(-) diff --git a/changelog.md b/changelog.md index 9fa8819..03fbe5b 100644 --- a/changelog.md +++ b/changelog.md @@ -1,5 +1,9 @@ # Changelog +## v1.9.1 | In development + +- Fixed an issue where calling CMake with `-DPython_EXECUTABLE=` 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. + ## v1.9.0 | 2024-05-03 - Added support for Conan v2. diff --git a/conanfile.py b/conanfile.py index 80f6ad5..f379d89 100644 --- a/conanfile.py +++ b/conanfile.py @@ -10,7 +10,7 @@ # noinspection PyUnresolvedReferences class EmbeddedPython(ConanFile): name = "embedded_python" - version = "1.9.0" # of the Conan package, `embedded_python-core:version` is the Python version + version = "1.9.1" # of the Conan package, `embedded_python-core:version` is the Python version license = "PSFL" description = "Embedded distribution of Python" topics = "embedded", "python" @@ -35,7 +35,7 @@ class EmbeddedPython(ConanFile): exports_sources = "embedded_python.cmake" def requirements(self): - self.requires(f"embedded_python-core/1.3.0@{self.user}/{self.channel}") + self.requires(f"embedded_python-core/1.3.1@{self.user}/{self.channel}") @property def pyversion(self): diff --git a/core/conanfile.py b/core/conanfile.py index a733f9c..d13063a 100644 --- a/core/conanfile.py +++ b/core/conanfile.py @@ -13,7 +13,7 @@ # noinspection PyUnresolvedReferences class EmbeddedPythonCore(ConanFile): name = "embedded_python-core" - version = "1.3.0" # of the Conan package, `options.version` is the Python version + version = "1.3.1" # of the Conan package, `options.version` is the Python version license = "PSFL" description = "The core embedded Python (no extra pip packages)" topics = "embedded", "python" diff --git a/core/embedded_python-core.cmake b/core/embedded_python-core.cmake index 08c791e..448ebf8 100644 --- a/core/embedded_python-core.cmake +++ b/core/embedded_python-core.cmake @@ -1,12 +1,16 @@ include_guard(GLOBAL) -# A hint for `find_package(Python)` -set(Python_ROOT_DIR "${CMAKE_CURRENT_LIST_DIR}/embedded_python" CACHE STRING "") - -if(WIN32) # Extra hint to speed up the find (not needed for correctness) - set(Python_EXECUTABLE "${Python_ROOT_DIR}/python.exe" CACHE STRING "") +# `find_package(Python)` supports specifying `Python_EXECUTABLE` to short-circuit the search. +# See: https://cmake.org/cmake/help/latest/module/FindPython.html#artifacts-specification +# When this variable is specified, all other hints are ignored. Note that we `FORCE` set the +# variables. Otherwise, the values could be hijacked via earlier `set(CACHE)` calls or via +# `-D` flags on the command line (some IDEs do this implicitly). For projects that embed +# Python, it's important that there are no traces of system Python in the build. +set(Python_ROOT_DIR "${CMAKE_CURRENT_LIST_DIR}/embedded_python" CACHE STRING "" FORCE) +if(WIN32) + set(Python_EXECUTABLE "${Python_ROOT_DIR}/python.exe" CACHE STRING "" FORCE) else() - set(Python_EXECUTABLE "${Python_ROOT_DIR}/bin/python3" CACHE STRING "") + set(Python_EXECUTABLE "${Python_ROOT_DIR}/python3" CACHE STRING "" FORCE) endif() find_package(Python ${self.pyversion} EXACT REQUIRED GLOBAL COMPONENTS Interpreter Development) diff --git a/core/test_package/conanfile.py b/core/test_package/conanfile.py index 57f26ac..4872959 100644 --- a/core/test_package/conanfile.py +++ b/core/test_package/conanfile.py @@ -32,7 +32,14 @@ def build(self): embedded_python_tools.symlink_import(self, dst="bin/python") cmake = CMake(self) - cmake.configure(variables={"EXPECTED_PYTHON_CORE_PATH": self._core_package_path.as_posix()}) + cmake.configure( + variables={ + # To test that we find the correct prefix for `Python_EXECUTABLE` + "EXPECTED_PYTHON_CORE_PATH": self._core_package_path.as_posix(), + # We specify the wrong exe here (system Python) to test that we do ignore it + "Python_EXECUTABLE": sys.executable, + } + ) cmake.build() @property diff --git a/embedded_python.cmake b/embedded_python.cmake index d9f2b96..33d2104 100644 --- a/embedded_python.cmake +++ b/embedded_python.cmake @@ -11,7 +11,7 @@ # since those are already provided by `core`. if(WIN32) - set(EmbeddedPython_EXECUTABLE "${CMAKE_CURRENT_LIST_DIR}/embedded_python/python.exe") + set(EmbeddedPython_EXECUTABLE "${CMAKE_CURRENT_LIST_DIR}/embedded_python/python.exe" CACHE STRING "" FORCE) else() - set(EmbeddedPython_EXECUTABLE "${CMAKE_CURRENT_LIST_DIR}/embedded_python/bin/python3") + set(EmbeddedPython_EXECUTABLE "${CMAKE_CURRENT_LIST_DIR}/embedded_python/python3" CACHE STRING "" FORCE) endif() diff --git a/test_package/conanfile.py b/test_package/conanfile.py index fffdcf6..82d967c 100644 --- a/test_package/conanfile.py +++ b/test_package/conanfile.py @@ -64,8 +64,11 @@ def build(self): cmake = CMake(self) cmake.configure( variables={ + # To test that we find the correct prefix for `Python_EXECUTABLE` "EXPECTED_PYTHON_CORE_PATH": self._core_package_path.as_posix(), "EXPECTED_PYTHON_PATH": self._package_path.as_posix(), + # We specify the wrong exe here (system Python) to test that we do ignore it + "Python_EXECUTABLE": sys.executable, } ) cmake.build()