Missing python-bindings requirements should not lead to a FATAL_ERROR in CMake

Summary

If Python is not found, the python bindings are not activated. If, however, during the setup of the python bindings some python requirement cannot be fulfilled (and also not automatically installed), CMake raises a hard error. This happened, e.g., in a Linux Mint 19 system (based on Ubuntu 18.04) with an unusable pip. Warning are shown and the hard error message indicates how to solve the problem, but for those who do not want to use any python bindings this is annoying, because it is absolutely not needed for the c++ library.

Proposed solution: transform FATA_ERRORs in CMake into WARNING and deactivate the python bindings instead. Maybe collect all these problems in a variable DUNE_PYTHON_BINDINGS_REQUIREMENTS_FOUND that is set to FALSE in any of the current FATAL_ERROR cases.

Related to #275.

Edit

The issue is not a hard error during the installation of the requirements for the python bindings. A problem there leads to a warning only. The issue is a hard error when setting up the internal venv and pip is not available.

added buildsystem python labels

@simon.praetorius: IMHO: There should not be any FATAL_ERROR when checking for the requirements of the Python binding. Instead, if any requirement is not fulfilled, the bindings should be disabled. So, a slightly softer approach would be more appropriate. This could, probably, easily be done by changing some FATAL_ERROR to WARNING + setting the option DUNE_ENABLE_PYTHONBINDINGS=FALSE.

@andreas.dedner: Sounds good to me

@christi:

IMHO: There should not be any FATAL_ERROR when checking for the requirements of the Python binding. Instead, if any requirement is not fulfilled, the bindings should be disabled. So, a slightly softer approach would be more appropriate. This could, probably, easily be done by changing some FATAL_ERROR to WARNING + setting the option DUNE_ENABLE_PYTHONBINDINGS=FALSE.

In that case it would be good if a clear message is printed somewhere, ideally in the summary that python bindings were disable, because requirement xy was not fulfilled.

@andreas.dedner: Current situation:

In DunePythonInstallPackages was have at the moment

  dune_execute_process(COMMAND ${DUNE_PYTHON_VIRTUALENV_EXECUTABLE} -m pip install ...
                        RESULT_VARIABLE DUNE_PYTHON_DEPENDENCIES_FAILED
                        WARNING_MESSAGE "python package requirements could not be installed - possibly connection to the python package index failed"
                      )
  if(DUNE_PYTHON_DEPENDENCIES_FAILED)
    set(DUNE_PYTHON_VENVSETUP FALSE CACHE BOOL "The internal venv setup failed: some required packages could not be installed")
    return()
  endif()

So it fails with a warning (not in the summary) that the required packages could not be installed. Here we already assume pip was found. Before that we have in the same file

if(NOT DUNE_PYTHON_pip_FOUND)
    message(WARNING "dune_python_install_package: Requested installations, but pip was not found!")
    set(DUNE_PYTHON_VENVSETUP FALSE CACHE BOOL "The internal venv setup failed due to missing pip")
    return()
  endif()

So this already allows the configuration to continue.

The failure happens in DunePythonVirtualEnv.cmake as @simon.praetorius wrote. We could easily make that error into a warning as well. I tried that (removing my pip locally) and that then completes the configuration with two warnings but of course at this point no information in the summary. We could remove the warning in DunePythonInstallPackages since that is redundant.

PS: The cache variable DUNE_PYTHON_VENVSETUP is not correctly named since the internal venv setup has worked it's only the Python bindings that will not be correctly added due to missing requirements. It's used to disable running the dune_python_add_test command (DunePythonTestCommand) and should probably be renamed.

mentioned in issue #275

changed the description

In DunePythonVirtualEnv.cmake there are some places that could be improved in this regard, e.g.,

# DunePythonVirtualEnv.cmake:132
if(NOT(DUNE_PYTHON_virtualenv_FOUND OR DUNE_PYTHON_venv_FOUND))
  message(FATAL_ERROR "One of the python packages virtualenv/venv is needed on the host system!")
endif()

could be changed to

# DunePythonVirtualEnv.cmake:132
if(NOT(DUNE_PYTHON_virtualenv_FOUND OR DUNE_PYTHON_venv_FOUND))
  message(WARNING "One of the python packages virtualenv/venv is needed on the host system!")
  set(DUNE_ENABLE_PYTHONBINDINGS FALSE)
  return()
endif()

For completeness, another solution would be to invest time in solving #279 (closed). If solved, this would obliviate the "if requ x not found disable Python" logic.

This issue is not about the python requirements for the bindings.

I'm just saying it would solve this issue, too.

So you also want to setup the vierual env at a different stage of the build process?

yes during install, it's described in #279 (closed).

So one would always need to run install before testing anything in a module. This would also influence the way dune-testtools can be used - I wasn't aware that you wanted to change that as well.

I consider dune-testtools a completely different case because there a pure Python package has to be installed to be used during configure time by CMake. That's quite a special case. I would be fine with documenting in dune-testtools that you need to manually do so, before configuring Dune. But maybe there is other used cases with Python modules where none of the approaches would work... Getting off-track for this issue here...

@simon.praetorius @andreas.dedner FYI, I can reproduce the initial error in a fresh ubuntu (20.04 in my case) that doesn't have python3-venv installed via aptitude. Then I get this warning + error (the error doesn't match the actual problem but the warning suggests the solution)

-- WARNING: Failed to build a virtual env with pip installed, trying again without pip
#18 4.679 -- If you are using Debian or Ubuntu, consider installing python3-venv and / or python-virtualenv
#18 4.768 CMake Error at cmake/modules/DunePythonVirtualenv.cmake:241 (message):
#18 4.768   dune-common set up a virtualenv, but needs pip to be installed into it.

In any case, this should not be a fatal error as suggested in this issue.

docker build -t minimal --build-arg DUNE_BRANCH=master . to reproduce the error Dockerfile gcc.minimal.opts

Yes, there are also some hidden hard errors by the dune_execute_process command. Not sure whether all of these can be catched/handled before

Where did you find those?

The dune_execute_process can raise a hard error if something failed and give a corresponding error message, e.g. in DunePYthonVirtualEnv.cmake:177.

I missed that - that would be before the place where the current fatal error is coming up in line 241 which is where I changed the error to warning (a return wasn't required there). I'm not sure I understand the logic here - first one tries to setup a venv without pip (line 176) but then fails because pip is not available? In line 222 there is the check for DUNE_PYTHON_ALLOW_GET_PIP but that was also done previously in line 156.

Is there a reason to not at least replace that fatal error with a warning during the setup of the virtual env now?

As I said above the problem is gone then - one gets two warning and no nice message in the summary but that is then a different issue. We seem to all agree that there should be no hard error when setting up the internal venv. That is just still there from Dominic's original version where the internal venv had to be activated separately and then a hard error made sense.

Does this work? A warning does not stop the execution and in the next step you call functions relying on the found venv or found pip. Or do you mean, warning + return? If it is tried out and works, I'm perfectly fine with a simple solution :-)

I mean warning + return which I tested and it least it worked for the one setting I tested. I don't know how to check this for example in the nightly build. Perhaps if we had a image without python-dev then we could have one of the pipelines use that to test it?

You already get an error if you have python-dev but not python-venv, see #283 (comment 87022). This is a different case than not having python at all no?

This can be solved with the warning + return as well, I think.

I think we cannot test everything in the nightly/automatic test-suite. We have

1. no python
2. python, but no pip
3. python but not python-dev
4. python but no python-venv
5. ...

Probably there are a lot more corner cases. This can only be tested locally, I believe.

Probably there are a lot more corner cases.

e.g with mpi without mpi (cf. !1054 (merged)) (I think there is no setup without MPI so far?)

I'm not sure whether we can write a test the starts with nothing and then step by step installs more host system packages (for python or via apt). There is, I guess, a problem with the permissions.

Of course, we could write a sequence of docker images that contain more and more packages. If proper layered images are used, this would not be too expensive.

So your idea would be something like

declare -a pacakages=(
  "python3-dev", "mpi", ...

for p in packages ; do
  apt install $p
  dunecontrol all
  runTests
done

I could try adding that to my github actions setup - there are less restrictions with the permissions there.

That said - I'm not quite sure github provides a runner with a suitable minimal image https://docs.github.com/en/actions/learn-github-actions/workflow-syntax-for-github-actions#jobsjob_idruns-on

So, maybe we should create a docker image for dune without python (and without mpi) that could be used as starting point

We can possibly use a 'docker' container on the ubuntu github runner with some existing minimal image? As long as the image is setup to give sudo privileges and internet access so that apt install works within the docker image that would work?

I just tested using the docker test @tkoch provided above in the github action tests and that produced the expected error. So this seems an option for testing setup in reduced images. Would require someone to come up with a suitable bash script that starts a docker image and then successively updates the packages in such a way that in each iteration building for example dune-common to dune-grid work in each stage.

mentioned in issue #285 (closed)

I started to build a minimal docker image for these tests: docker/ci!108 (merged) It contains only a very small set of packages, except the ubuntu default packages: ca-certificates, cmake, gcc, g++, git, make, wget. I will prepare a job in dune-common (or in the nightly tests) to run the build of dune with this setup and then step by step install more packages.

My local test revealed the following:

0. missing pkg-config errors in lib/dunemodules.lib (ignored)
1. no python3: everything compiles fine
2. install python3:

CMake Error at cmake/modules/DunePythonCommonMacros.cmake:97 (message):
  Found a Python interpreter but the Python bindings also requires the Python
  libraries (a package named like python-dev package or python3-devel)

3. install python3-dev:

-- Failed to find the python package pip with interpreter /usr/bin/python3. (missing: DUNE_PYTHON_pip_FOUND) 
-- Failed to find the python package virtualenv with interpreter /usr/bin/python3. (missing: DUNE_PYTHON_virtualenv_FOUND) 
Traceback (most recent call last):
  File "/duneci/modules/dune-common/cmake/scripts/pyversion.py", line 24, in <module>
    import pkg_resources
ModuleNotFoundError: No module named 'pkg_resources'
-- Found venv_/usr/bin/python3: TRUE  
-- Building a virtualenv in /duneci/modules/dune-common/build-cmake/dune-env
-- WARNING: Failed to build a virtual env with pip installed, trying again without pip
-- If you are using Debian or Ubuntu, consider installing python3-venv and / or python-virtualenv
-- Failed to find the python package pip with interpreter /duneci/modules/dune-common/build-cmake/dune-env/bin/python. (missing: pippresent) 
CMake Error at cmake/modules/DunePythonVirtualenv.cmake:241 (message):
  dune-common set up a virtualenv, but needs pip to be installed into it.

                           You can either install it yourself manually activating the virtualenv with
                           the activate script in your build directory /duneci/modules/dune-common/build-cmake or you set
                           the CMake variable DUNE_PYTHON_ALLOW_GET_PIP to allow Dune to use get-pip.py
                           from https://bootstrap.pypa.io/get-pip.py

4. install python3-venv:

-- Failed to find the python package pip with interpreter /usr/bin/python3. (missing: DUNE_PYTHON_pip_FOUND) 
-- Failed to find the python package virtualenv with interpreter /usr/bin/python3. (missing: DUNE_PYTHON_virtualenv_FOUND) 
Traceback (most recent call last):
  File "/duneci/modules/dune-common/cmake/scripts/pyversion.py", line 24, in <module>
    import pkg_resources
ModuleNotFoundError: No module named 'pkg_resources'

and then later in the python build:

Obtaining file:///duneci/modules/dune-common/build-cmake/python
  Checking if build backend supports build_editable ... error
  ERROR: Command errored out with exit status 1:
   command: /duneci/modules/dune-common/build-cmake/python/../dune-env/bin/python /duneci/modules/dune-common/build-cmake/dune-env/lib/python3.6/site-packages/pip/_vendor/pep517/in_process/_in_process.py _supported_features /tmp/tmpl5veu5x_
       cwd: /duneci/modules/dune-common/build-cmake/python
  Complete output (10 lines):
  Traceback (most recent call last):
    File "/duneci/modules/dune-common/build-cmake/dune-env/lib/python3.6/site-packages/pip/_vendor/pep517/in_process/_in_process.py", line 363, in <module>
      main()
    File "/duneci/modules/dune-common/build-cmake/dune-env/lib/python3.6/site-packages/pip/_vendor/pep517/in_process/_in_process.py", line 345, in main
      json_out['return_val'] = hook(**hook_input['kwargs'])
    File "/duneci/modules/dune-common/build-cmake/dune-env/lib/python3.6/site-packages/pip/_vendor/pep517/in_process/_in_process.py", line 112, in _supported_features
      backend = _build_backend()
    File "/duneci/modules/dune-common/build-cmake/dune-env/lib/python3.6/site-packages/pip/_vendor/pep517/in_process/_in_process.py", line 102, in _build_backend
      obj = getattr(obj, path_part)
  AttributeError: module 'setuptools.build_meta' has no attribute '__legacy__'

(This error seems to be related to https://github.com/pypa/setuptools/issues/1694)

5. install python3-pip: At least the CMake error is gone, the setuptools error is still there.

6. Following the error message in 3., I have set the CMake variable DUNE_PYTHON_ALLOW_GET_PIP:BOOL=1. Then I get the message:

-- Installing pip using https://bootstrap.pypa.io/get-pip.py...
[...]
CMake Warning at cmake/modules/DunePythonInstallPackage.cmake:104 (message):
  dune_python_install_package: Requested installations, but pip was not
  found!
Call Stack (most recent call first):
  python/CMakeLists.txt:12 (dune_python_install_package)

7. Fixing 6. by properly setting that pip is found results in the next warning/error:

CMake Warning at cmake/modules/DuneExecuteProcess.cmake:62 (message):
  python package requirements could not be installed - possibly connection to
  the python package index failed
[...]
  Failed to build mpi4py

  ERROR: Could not build wheels for mpi4py, which is required to install
  pyproject.toml-based projects

Note, even if required packages could not be installed by pip, the targets _common and _typereqistry are still compiled. Probably this should be deactivated in case they cannot be installed and used.

The problem with mpi4py were mentioned in another issue before. A simple fix for the python-pip-error message would be to have MPI as requirement for the python bindings.

(is resolved already in current master)

The current concept is that if HAVE_MPI is set then mpi4py is added to the requirements. The issue that the wheel can not be build is normally a warning and the package is compiled. From your description it seems that your image doesn't contain mpi? Then mpi4py should not be added (see cmake/modules/DunePythonInstallPackage.cmake:100 - do you get the message saying that mpi4py is being added?).

Following the link you gave an option seems to be to upgrade setuptools? Perhaps changing python/pyproject.toml.in to something like

requires = ["setuptools>=41", "wheel", "pip>=21"]

would solve that?

Not sure whether I have done this correctly. I have copied your line in the python/pyproject.toml.in file, deleted the build directory and run everything again. This results in the same error. How can I check the installed setuptools and pip versions in the virtual env?

OK, I have done source build-cmake/dune-env/bin/activate && python -m pip list. This results in

Package       Version
------------- -------
Jinja2        3.0.3
MarkupSafe    2.0.1
mpi4py        3.1.3
numpy         1.19.5
pip           21.3.1
pkg_resources 0.0.0
portalocker   2.3.2
setuptools    39.0.1
wheel         0.37.0

Thus, the setuptools is not installed in the correct version

Sorry, got that wrong place - pyproject.toml is for an isolated build which is not used for the internal venv. Could you please try adding the setuptools>=41​ to dune.module instead.

This works.

In dune.module the mpi4py package is hardcoded. Removing this, the other error above also vanishes. But then mpi4py is not available even if HAVE_MPI is true.

which branch of dune-common are you using. mpi4py in dune.module was removed in 108eda60 and shouldn't be there anymore in master. That commit also adds the code at cmake/modules/DunePythonInstallPackage.cmake:100 I mentioned above

+  if(HAVE_MPI AND NOT SKBUILD)
+    message(STATUS "Adding mpi4py to the Python requirements")
+    string(APPEND ProjectPythonRequires " mpi4py")
+  endif()

The development is too fast for me :-) I have updated to the recent master now...

Thanks @simon.praetorius!

For the FATAL_ERROR problem, I have started a branch: !1069 (merged)

mentioned in merge request !1071 (merged)

mentioned in merge request !1069 (merged)

I have created the docker image registry.dune-project.org/docker/ci/ubuntu:18.04-minimal that has a minimal set of packages only, i.e., cmake-3.13, gcc-7, g++-7, git, and no python. With this we can test that everything works fine in this setting. I have not yet found a way to incrementally install debian packages in a gitlab-ci job, although the duneci user has system rights. In a local docker run it works fine to use apt update && apt install .... Maybe someone has an idea how to transform this into a gitlab-ci job

isn't the main issue missing Internet access in the CI. Could you wget the deb packages in the Dockerfile and install those in a script during the CI run?

mentioned in commit 64ffaac6

closed with merge request !1069 (merged)