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

Use Development.Module to be compatible with static Python interpreter #2195

Merged
merged 2 commits into from
Nov 12, 2021

Conversation

Helveg
Copy link
Contributor

@Helveg Helveg commented Oct 22, 2021

The current CMake build requires Python's shared library and header files to be available (libpython.so and Python.h). These aren't always available, and appearantly NEST builds perfectly fine without them.

Let's drop this requirement (one line change from find_package(Python3 Development) to Development.Module) so that NEST can also build and be deployed to environments where Python is built in static mode.

Many non-Ubuntu non-system Python installations do not have these shared libraries. The most relevant use cases where we bump into this issue is on lightweight containers and the manylinux wheel images where these files have been intentionally removed.

(And another one, often relevant to me and 15% of Python users, is that pyenv also builds without shared libraries ;) )

@terhorstd terhorstd added I: No breaking change Previously written code will work as before, no one should note anything changing (aside the fix) S: High Should be handled next T: Maintenance Work to keep up the quality of the code and documentation. labels Oct 28, 2021
@jougs
Copy link
Contributor

jougs commented Oct 28, 2021

I am confused by the description 🤯

Either the word "but" requires an "is available" or similar at the end of the first paragraph, or it needs to be re-formulated altogether. Can you please unconfuse me?!

@Helveg
Copy link
Contributor Author

Helveg commented Oct 29, 2021

Better?

Copy link
Contributor

@steffengraber steffengraber left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The first test on my laptop was positive. Now I tried it in a Docker image and unfortunately I got the following error message:

CMake Error at /usr/share/cmake-3.16/Modules/FindPackageHandleStandardArgs.cmake:146 (message):
Could NOT find Python (missing: Development.Module) (found suitable version
"3.8.10", minimum required is "3.8")
Call Stack (most recent call first):
/usr/share/cmake-3.16/Modules/FindPackageHandleStandardArgs.cmake:393 (_FPHSA_FAILURE_MESSAGE)
/usr/share/cmake-3.16/Modules/FindPython/Support.cmake:2214 (find_package_handle_standard_args)
/usr/share/cmake-3.16/Modules/FindPython.cmake:304 (include)
cmake/ProcessOptions.cmake:395 (find_package)
CMakeLists.txt:130 (nest_process_with_python)

Any idea?

@Helveg
Copy link
Contributor Author

Helveg commented Oct 29, 2021

Can you try a newer version of CMake? Perhaps we'll have to hike the minimum CMake version. From the doc it seems added in 3.18:

New in version 3.18:

  • Development.Module: search for artifacts for Python 3 module developments.
  • Development.Embed: search for artifacts for Python 3 embedding developments.

Up-to-date CMake is available everywhere through pip install cmake 😈

@steffengraber
Copy link
Contributor

I will try, but it is cmake 3.16 the default version in Ubuntu Focal, the latest LTS version.
If this is the problem, I think many users will encounter the problem.

@jougs
Copy link
Contributor

jougs commented Oct 29, 2021

Technically, I'm always in favor of minimalizing things, but @steffengraber's concern is a very valid one.

Would it be possible to ship (and for cmake versions < 3.18 use) a newer version of FindPython3.cmake and the corresponding support file?

@Helveg
Copy link
Contributor Author

Helveg commented Oct 29, 2021

Emit an error to enforce CMake 3.18+. This would only be a deal breaker for Ubuntu users who haven't obtained Ubuntu software outside of apt-install yet:

  • By default non-power users are also prompted to use a virtualenv -> recommend to pip install cmake into venv in the error. We can probably baby-proof this sharp corner by mentioning pip install cmake in the venv creation instructions.
  • Fall back to Development and emit a warning.
  • They should be using the nest wheels (soon ™️ )

What do you think?

@steffengraber
Copy link
Contributor

pip install cmake works.

@jougs
Copy link
Contributor

jougs commented Nov 3, 2021

@terhorstd and I have discussed this and are in favor of a switch based on the CMake version available. I.e.,

  • Fall back to Development and emit a warning.

@Helveg
Copy link
Contributor Author

Helveg commented Nov 8, 2021

@jougs ready for review

Copy link
Contributor

@jougs jougs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sweet!

@jougs
Copy link
Contributor

jougs commented Nov 12, 2021

@steffengraber: could you please re-review (and possibly merge)? Thanks!

Copy link
Contributor

@steffengraber steffengraber left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have tested it in a clean Ubuntu 20.04 installation and found no problems. 👍

@jougs jougs merged commit 971cb4a into nest:master Nov 12, 2021
@Helveg Helveg deleted the python-abi-build branch November 12, 2021 10:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
I: No breaking change Previously written code will work as before, no one should note anything changing (aside the fix) S: High Should be handled next T: Maintenance Work to keep up the quality of the code and documentation.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants