conan-io / conan-center-index

Recipes for the ConanCenter repository
https://conan.io/center
MIT License
970 stars 1.78k forks source link

[package] CPython: Misc recipe improvements #23151

Open Ahajha opened 8 months ago

Ahajha commented 8 months ago

I'm making this issue to serve as a TODO list of things that I believe need to be done in the CPython recipe:

Other notes:

Ahajha commented 8 months ago

Regarding conf info and environment variables:

Currently, we have the following defined: Env: PATH, PYTHON, PYTHONHOME, PYTHON_ROOT Conf: user.cpython:python, user.cpython:pythonhome, user.cpython:module_requires_pythonhome, user.cpython:python_root

Env:

Conf:

I'm unsure of the use of the env_vars option for Conan 2.0, that kind of option seemed more of a hack around there being no way to opt out of the variables. Now, Conan 2.0 has an opt-in method of using env vars, so I don't think this option should be used in Conan 2.0.

Ahajha commented 8 months ago

Regarding transparent FindPython integration, I've played with a few ideas:

  1. If we make the CMake find modules Python and Python3 (one has to be CONFIG, I don't think there's a way to make aliases as of yet), then we can set some hint variables and then include("${CMAKE_ROOT}/Modules/FindPython3"). This gets us decently far, but doesn't seem to work on Windows in Debug mode(problem, was known before and unsure how to fix), and would likely require linking the created targets with cpython::XXX (likely not a problem, assuming I'm allowed to change those targets within CMake).
  2. Include a definition of Python3_add_library. This gets us further, but it becomes difficult to manage the various targets that FindPython exposes, for example, Python3::SABIModule vs Python3::Module. This would be a longer implementation, and would lock us into latest the version of FindPython, instead of relying on CMake's version.

I think (1) is the ideal route, but I need to figure out why Debug mode on Windows isn't working. Maybe it's a bug in CMake, unsure.

valgur commented 8 months ago

For FindPython it's sufficient to add cpython as a tool_requires and a VirtualBuildEnv. I think that's not too much to ask from the package consumers.

Ahajha commented 8 months ago

That seems to work with Release mode on Windows, but not Debug (seems to really want to pick any other version on the system). I suppose it's less of an issue if it's in the build context, since that will usually be release mode, but I would like it to work without issue.

Ahajha commented 7 months ago

Update on the find_package issue regarding Windows Debug mode: I don't think it will be possible to just do nothing on our side and have it work, but I can get it to work in the test package (and consequently in other recipes) by defining just Python3_EXECUTABLE and Python3_LIBRARY in CMake to the .exe and .lib file, respectively. Going back to idea 1 from above, this should now be possible.

Ahajha commented 7 months ago

Regarding options that might not work:

All of these options default to on, so I tried disabling all of them, then reverted the ones that caused test failures.

For Windows:

3 options are removed for Windows: with_curses, with_gdbm, with_nis. The remaining 4 work, except that a small patch needs to be made: In _msvc_excluded_projects, the discarded bz2 project should be changed to _bz2.

No notably changes were made to the build system at any point, so I only tested 3.10.14.

For non-Windows platforms:

(Tested on Ubuntu 22.04)

The build system for modules changed significantly between 3.10 and 3.12, with 3.11 being in a transition state, so I tested all 3. Pre-3.10 not much changed. (Also to note, so far I don't see much change between 3.12 and 3.13.)

All options work in all versions. Some options, however, will not correctly apply if the respective system package is installed, for example with_curses.

Honorable mention to with_nis, which currently defaults to off since the requisite nsl package does not exist in CCI. However, this module is being removed in 3.13, so I don't think there's a point trying to enable it at this point, as it ends up picking up a system package for now.

Edit: After removing libncurses-dev and other dev packages (unsure why they were installed, but besides the point), all options now work in all versions. So some of them are affected by the system packages, ideally they shouldn't be.