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=<system_python>` 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.
lumicks · Jun 13, 2024 · 979e81e · 979e81e
1 parent 0484735
commit 979e81e
Show file tree

Hide file tree

Showing 7 changed files with 30 additions and 12 deletions.
diff --git 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=<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.
+
 ## v1.9.0 | 2024-05-03
 
 - Added support for Conan v2.

diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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()