Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Feature request] Export cmake variables corresponding to Python version #2268

Open
SylvainCorlay opened this issue Jun 26, 2020 · 9 comments

Comments

@SylvainCorlay
Copy link
Member

Variables such as PYTHON_VERSION_MAJOR and PYTHON_VERSION_MINOR are marked as INTERNAL.

As a consequence, downstream projects making use of pybind11 in their cmake cannot make use of these variables. It would be very convenient to have access to them!

@henryiii
Copy link
Collaborator

Possibly will be solved as part of #2201.

@henryiii
Copy link
Collaborator

Actually this doesn't mean you can't use them downstream, it just means you shouldn't change them in a configure call (they don't show up in things like ccmake.

@henryiii
Copy link
Collaborator

@SylvainCorlay Unless you have a specific request, I think this should be closed, let me know if there's anything else. In "classic" mode, the variables are exposed; CMake's INTERNAL just means they are not user settable and therefore should not show up in the GUI. In the new mode, you don't have to rely on them at all, as you can do:

find_package(Python COMPONENTS Interpreter Development)
# All Python variables from FindPython usable
find_package(pybind11)

@SylvainCorlay
Copy link
Member Author

@henryiii I don't think this should be marked as closed.

Our use case is that we rely on Pybind11 to find Python and export the cmake flags for downstream packages. In these downstream packages, we need to do different things depending on the Python version. Should we make our own call to find_package instead of relying on pybind11 and the flags passed to pybind11, we may have a different result.

Hence I think that pybind11 should not make these INTERNAL.

@henryiii henryiii reopened this Aug 19, 2020
@henryiii
Copy link
Collaborator

henryiii commented Aug 19, 2020

I still believe the correct setting for this may be "INTERNAL". INTERNAL in CMake means it is not user visible, that is, it is not something you set in the command line, ie, cmake -DPYTHON_VERION=3.7.6 is invalid, it won't show up in the GUI. It does not mean a project that uses pybind11 is not supposed to use these variables; in fact, it is explicitly allowed and recommended that packages do use these (since you cannot otherwise access the PYTHON_ variables; recalling FindPythonInterp/Libs is invalid).

The other possible settings are:

  1. Use a normal, non-INTERNAL set. Parent projects could not use these variables anymore. ❌
  2. Use a CACHE STRING FORCE setting, it shows up in cmake-gui, ccmake, and cmake -L as a configurable option ❌
  3. Use a CACHE STRING FORCE setting with mark_as_advanced ; this slightly hides it in the gui/listings, but it still a settable option
  4. Set in PARENT_SCOPE if not the master project; this is less accessible than CACHE settings, which are truly global. If we were not worried about backward compatibility, then this might be a good option.
  5. Use INTERNAL, which makes it available anywhere in CMake, and doesn't tell users it is editable. (current solution)

Also, the best solution is likely to use:

find_package(Python COMPONENTS Interpreter Development)
message(STATUS "${Python_VERSION}")
...
add_subdirectory(pybind11) # Respects previous call to find_package(Python, does not find or set old variables

@henryiii
Copy link
Collaborator

henryiii commented Aug 19, 2020

To reiterate, the following usage is fully supported:

add_subdirectory(pybind11)
message(STATUS "${PYTHON_VERSION}")

Marking PYTHON_VERSION INTERNAL does not mean that we intend to drop that as if it was not public interface, it is.

@henryiii
Copy link
Collaborator

PS: Changed find_package to add_subdirectory above, as I think that's the one we are talking about; but it's true for both.

@henryiii
Copy link
Collaborator

henryiii commented Aug 19, 2020

In CMake-GUI:

  • BOOL is a checkbox.
  • FILEPATH is a file dialog.
  • PATH is a directory dialog.
  • STRING is an edit box, or a dropdown if you set an optional property
  • INTERNAL does not show up. It also implies FORCE, since there's no way to set it in the GUI.

That's the meaning of this setting; it's a GUI setting.

@SylvainCorlay
Copy link
Member Author

Hum, patching the pybind11 to remove the INTERNAL appears to work for us. Will get back with some more info.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants