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

Request PYTHON_INCLUDE_DIR and PYTHON_LIBRARY variables (or equivalent) #231

Closed
ramcdona opened this issue Aug 10, 2021 · 4 comments
Closed
Assignees
Labels
feature request New feature or request to improve the current logic

Comments

@ramcdona
Copy link

CMake's FindPythonLibs.cmake searches standard install locations to find a Python installation to build against when compiling C/C++ to work with Python. This generally means it finds the default system-installed Python.

Since setup-python installs specific requested versions of Python to a non-standard location, CMake won't ever find them.

It would be great if setup-python set variables (like pythonLocation) that point at the library and header files needed to build C/C++ against a specific Python. In CMake terms, these are PYTHON_INCLUDE_DIR and PYTHON_LIBRARY, but other names could be used.

The CMake scripts FindPythonLibs, FindPythonInterp, and the new FindPython2 and FindPython3 set some other variables that might also be worth adding to the list.

Finally, it would be great if all these variables were clearly documented and their existence was obvious to a user of setup-python.

@nikita-bykov nikita-bykov self-assigned this Nov 12, 2021
@nikita-bykov nikita-bykov added the feature request New feature or request to improve the current logic label Nov 12, 2021
@nikita-bykov nikita-bykov removed their assignment Nov 12, 2021
@dsame dsame self-assigned this Jun 21, 2022
@dsame dsame mentioned this issue Jun 22, 2022
@dsame
Copy link
Contributor

dsame commented Jun 22, 2022

hello @ramcdona , i created the PR to set the hints FindPython, FindPython2 and FindPython3

I hesitate we should set PYTHON_INCLUDE_DIR and PYTHON_LIBRARY variables for deprecated FindPythonLibs module - setting these variables potentially can override the default search based on ROOT_DIR. Any thougts?

@IvanZosimov
Copy link
Contributor

📟 Just a gentle ping, @ramcdona

@ramcdona
Copy link
Author

ramcdona commented Jul 7, 2022

I'm sorry, I thought I was included on this review by mistake. As a GH Actions user (not developer), I don't know how to test this change in my build system to see if it resolves my problems.

Obviously, my goal is for FindPythonXXX to 'just work' when using setup-python to set a particular version.

CMake 3.12 (the version that deprecates FindPythonLibs) is pretty new in CMake world. While the CMake community is getting better at using newer versions, it still lags. This site lists what CMake version can be expected by default with most Linux distributions.

The GH Actions Ubuntu 18.04 has CMake 3.23.2, so that isn't a problem. It looks like the oldest Windows and MacOS environments also support 3.22.2.

In my own program, I only recently updated my CMAKE_MINIMUM_REQUIRED to 3.1. I have had need to support RH 7 -- which is terribly slow to progress.

Your fix (setting CMake specific variables) should make for a pretty magical experience for CMake 3.12+ users. However, it does not help someone using simple makefiles or a hand-rolled build setup. Imagine someone setting up a compiler and wanting to know what to set the -I and -L paths to. They will have to implement their own CMake-like FindPython3- like system to convert pythonLocation into the information they need.

So, to me, having variables that contain the information implied by PYTHON_INCLUDE_DIR and PYTHON_LIBRARY is a good general purpose solution (independent of CMake). If they had generic (non-CMake aligned) names -- and those names were documented -- then a CMake user could access them and pass them in their GH Actions via -DPYTHON_INCLUDE_DIR=$pythonIncludeDir and -DPYTHON_LIBRARY=$pythonLibrary.

So, my initial request was for setup-python to set some variables that could reasonably be expected by anyone who needs to compile against the desired version of Python. If those variables were clearly documented, said user could probably figure out how to make things work for their build system.

Your solution goes above and beyond for CMake users (very cool) and is almost magical enough to not even need documentation (though the setup-python docs should be mentioned that it is expected to magically work with FindPython3).

I am certainly not the grand architect of GitHub Actions -- so I won't claim to dictate what is the right philosophical direction here.

Either way, it would be great to add some documentation (perhaps here) to inform users about these additional variables and their intended consequences. While you're at it, documenting the consequences of setup-python with "e.g." (which is perhaps an incomplete list) is fairly non-satisfying.

With this setting, the action will add/update environment variables (e.g. PATH, PKG_CONFIG_PATH, pythonLocation) for python to just work out of the box.

If that is a complete list, then OK, but it 'feels' like an incomplete list -- and the user has no way to find a complete list (including what they are expected to contain or mean). A bulleted list of all variables set by setup-python would be much appreciated.

@dsame
Copy link
Contributor

dsame commented Aug 2, 2022

@ramcdona currently we have python tool python3-config installed that does exactly that you have requested and it is well known and documented tool.

python3-config --includes produces the output
-I/Library/Frameworks/Python.framework/Versions/3.8/include/python3.8 -I/Library/Frameworks/Python.framework/Versions/3.8/include/python3.8 and can be used for gcc builds as

gcc `python3-config --includes` ...

This is a preferable way to get the python configuration and we should avoid to mimic the same functionality because of possible unpredictable interferences.

The setting the hints for cmake though was a good idea and we already implemented it, thank you.

@dsame dsame closed this as completed Aug 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request New feature or request to improve the current logic
Projects
None yet
Development

No branches or pull requests

4 participants