conan-io / conan

Conan - The open-source C and C++ package manager
https://conan.io
MIT License
8.25k stars 980 forks source link

[bug] Error when detecting CPython version when included via CMake #16952

Open wawanbreton opened 1 month ago

wawanbreton commented 1 month ago

Describe the bug

Hi, I'm trying to consume the cpython/3.12.2 package for my project, which is Python binding.

My conanfile.py contains the following:

def requirements(self):
    self.requires("cpython/3.12.2")

def configure(self):
    self.options["cpython"].shared = True

And also the classic CMake generator/build

Now my CMakeLists.txt contains:

cmake_minimum_required(VERSION 3.20)
find_package(Python REQUIRED)
...

Building CPython 2.12.2 works like a charm, and the FindPython.cmake file and associated are properly generated. But when I run my conan build ., I get the following error:

...
-- Conan: Including build module from '/home/erwan/.conan2/p/b/cpyth65d041356dc8d/p/lib/cmake/use_conan_python.cmake'
-- Found Python: /home/erwan/.conan2/p/b/cpyth65d041356dc8d/p/lib/cmake/../../bin/python3.12 (found version "3.12.3") found components: Interpreter 
CMake Error at /home/erwan/.conan2/p/b/cpyth65d041356dc8d/p/lib/cmake/use_conan_python.cmake:18 (message):
  CMake detected wrong cpython version - this is likely a bug with the
  cpython Conan package
Call Stack (most recent call first):
  build/Release/generators/FindPython.cmake:38 (include)
  CMakeLists.txt:6 (find_package)

-- Configuring incomplete, errors occurred!

ERROR: conanfile.py (pyarcus/5.4.0-alpha.0): Error in build() method, line 155
        cmake.configure()
        ConanException: Error 1 while executing

Apparently, when trying to detect the Python version, it finds Python 3.12.3 which is the one installed on my system. I indeed have the following:

$ /home/erwan/.conan2/p/b/cpyth65d041356dc8d/p/lib/cmake/../../bin/python3.12
Python 3.12.3 (main, Jul 31 2024, 17:43:48) [GCC 13.2.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> 
$ LD_LIBRARY_PATH=/home/erwan/.conan2/p/b/cpyth65d041356dc8d/p/lib/ /home/erwan/.conan2/p/b/cpyth65d041356dc8d/p/lib/cmake/../../bin/python3.12
Python 3.12.2 (main, Sep  6 2024, 16:01:31) [GCC 13.2.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>>

I can then use the LD_LIBRARY_PATH trick to start the build, and this time the detection works properly. But this really sound like a dirty hack :smiley:

Do you think this is something about my environment, or is there indeed a bug somewhere ? In this case, I can try to provide a fix, but I don't know what part of the process is faulty.

How to reproduce it

The provided bug description should allow to reproduce it. Otherwise, I can make a minimal projet.

memsharded commented 1 month ago

Hi @wawanbreton

Thanks very much for your report. This seems indeed an issue in the cpython package recipe in ConanCenter, not necessarily in Conan itself. I'll check it, and if true I will transfer the ticket to the conan-center-index repo.

valgur commented 1 month ago

Related:

Adding self.tool_requires("cpython/<host_version>") and a VirtualBuildEnv(self).generate() will likely fix your issue, though.

memsharded commented 1 month ago

Hi @wawanbreton

The reason it is failing is because being a shared-library, it is not found in the runtime paths. Conan by default generates in the conanrun.sh information the necessary variables for runtime paths information, but the issue is doing a find_package(Python that happens to internally rely on the actual execution of the Python executable.

Then it means that the runtime information is needed also at build time (the build-time information contains the runtime paths of the tool_requires, so they can be used in the build, but not the runtime information of requires which in theory shouldn't be needed at build time).

This can be modeled in different ways with Conan, for example:

from conan import ConanFile
from conan.tools.cmake import CMake, cmake_layout
from conan.tools.env import VirtualRunEnv

class Pkt(ConanFile):
    settings = "os", "arch", "compiler", "build_type"
    generators = "CMakeToolchain", "CMakeDeps"
    requires = "cpython/3.12.2"

    def configure(self):
        self.options["cpython"].shared = True

    def layout(self):
        cmake_layout(self)

    def generate(self):
        VirtualRunEnv(self).generate(scope="build")

    def build(self):
        cmake = CMake(self)
        cmake.configure()
        cmake.build()

It can be forced with VirtualRunEnv(self).generate(scope="build") to also source the conanrun.sh runtime-environment script also from the conanbuild.sh, and then everything works.

Please try that and let us know if this makes sense.

wawanbreton commented 1 month ago

Hi, and thank you very much for you answers. Sorry for the delay in answering, I have been a bit sick.

So, setting VirtualRunEnv(self).generate(scope="build") does allow the Python detection to work ! This is a very acceptable solution for me, as it does not involve using a customized recipe, or something hacky/dirty.

On the other hand, I am probably not the only one who will run into this issue, in this case I don't have a very exotic setup. But maybe other people can find this discussion, and then the solution for it ? I also see that the VirtualRunEnv documentation does not list what scope values you can actually give, and for what purpose. Maybe that could be improved ? I'm merely suggesting, do what you want with it :)

Thanks again for you help !

memsharded commented 1 month ago

Thanks for the feedback!

yes, lets leave it open, at least some improvements to the docs could be made.

The thing is that "runtime" shared dependencies for "build time" are intrinsically complex, they don't have a great solution, and Conan can in fact do even more than some build systems can do by themselves. Still, it is very common for tools used at build-time to be statically linked to avoid these problems.

But yes, at least some docs can be added to explain this, thanks again!

Ahajha commented 1 month ago

Just chiming in - I'm glad that sanity check code I added actually caught something :)

memsharded commented 1 month ago

Interestingly, I was trying this in Windows and in Windows it seems to work. It happens that the recipe contains something like:

if self.options.env_vars:
            bindir = os.path.join(self.package_folder, "bin")
            self.runenv_info.append_path("PATH", bindir)
            self.buildenv_info.append_path("PATH", bindir)

Which is Windows location for shared libs, but it doesn't contain the equivalent for Linux LD_LIBRARY_PATH

memsharded commented 1 month ago

Which means that if I add:

diff --git a/recipes/cpython/all/conanfile.py b/recipes/cpython/all/conanfile.py
index c98c3381c1..d7eb719bd9 100644
--- a/recipes/cpython/all/conanfile.py
+++ b/recipes/cpython/all/conanfile.py
@@ -889,6 +889,9 @@ class CPythonConan(ConanFile):
             bindir = os.path.join(self.package_folder, "bin")
             self.runenv_info.append_path("PATH", bindir)
             self.buildenv_info.append_path("PATH", bindir)
+            if self.options.shared:  # So the find_package(Python) works
+                self.buildenv_info.append_path("LD_LIBRARY_PATH", os.path.join(self.package_folder, "lib"))
+                self.buildenv_info.append_path("DYLD_LIBRARY_PATH", os.path.join(self.package_folder, "lib"))

to the cpython recipe, now the above works without issues. I think I will propose this as a change to the recipe in ConanCenter, but please tell me if you have any feedback or concern.

Thanks for your feedback!

memsharded commented 1 month ago

I am reviewing the recipe, I have a quick question, for @wawanbreton, but also @Ahajha

What is the main use you want to do of the python conan package? You want to embed it as a library into your application, or do you want to use Python as a tool in your build process?

From what I see the recipe doesn't look intended to link Python as a library, because it doesn't contain includedirs.

Ahajha commented 1 month ago

@memsharded It does have includedirs - they're near the top of package_info(). I originally started working on the recipe in order to work on a project which included C++ extension modules and embedding Python, by necessity I believe Python would need to be available to run - both of these use cases are tested in the test package (and were there before I touched them). I am no longer working on that project though - so the answer for me in a weird way is sort of "what could have been". However my guess is that most people using this package are using Python as a build tool, with second place going to native extension modules.

wawanbreton commented 1 month ago

What is the main use you want to do of the python conan package? You want to embed it as a library into your application, or do you want to use Python as a tool in your build process?

In our case, it is even trickier :smiley: we have an application that is made of a few sub-projects. Some of them are C++ libraries, one is a C++ executable that runs in background, but our main front-end is a pure Python app. Thus we also have bindings to call the C++ libraries from the front-end. The bindings use Python as a shared library, but our main project embeds the actual Python interpreter, built by Conan.