Skip to content

[Build] Enable build of C++ unit tests with python bindings

Context

Up to now it was not possible to build the aidge_core archive with python bindings (-DPYBIND=ON) and link with unit tests or other dependent executable.

This was due mainly to a wrong declaration of dependency for _aidge_core (the archive) on Python::Module.

Actually, due to dependency in the archive itself on pybind/python for the support of python bindings, the archive requires to be dependent on the embedded python interpreter (Python::Python).

Though, while for standard executable it is fine to link with the python interpreter, we must not link with it when building python modules (indeed, python module should not explicitly refer to the interpreter in order to be portable across python versions).

Last, as for executable links, the python interpreter and the pybind headers are required, we must add these to the installed package dependency such that dependent targets get all dependency resolution correct.

To summarize, there was three issues with respect to the _aidge_core build:

  1. dependency on Python::Module instead of Python::Python
  2. when building a python module target, it must not depend on python runtime, only on pybind/python headers
  3. dependencies, here python headers/runtime and pybind headers and PYBIND define, must be declared in the installed package for consumers

Also the build system did not support well the case where the pyton embedded interpreter is not present such as in cibuildwheel containers. For this case, one must take car to not add dependencies on Python::Python when PYBIND is ON. Of course in this case, standalone executables and tests cannot be built.

There was also wrongly defined build dependencies in the aidge_export_aidge build system:

  1. the generated dnn module does not depend itself on the python runtime or pybind (I guess this was inherited by the _aidge_core build system, but this particularity is only for _aidge_core)
  2. the generated test main file should include the cpu implementation of Tensor in order to execute correctly

Refer also to request for comment eclipse/aidge/aidge_core#147 for more discussion on the dependency on pybind and python runtime in the core archive. Ultimately this dependency and the conditional -DPYDIND CPP flags should be removed if possible. If it is the case at some point, the python/pybind dependencies for the core archive could be completly removed, simplifying greatly its interfaces declaration.

Modified files

This merge request contains three commits: A. the first commit fixes the issues and dependencies related to the _aidge_core archive B. the second one fixes the build and test of aidge_export_aidge C. minor commit to add verbosity to the pytests cibuildwheel tests in order to see build system outputs and per test details

Note that the motivating change is the first commit A, this commit do not break dependent builds in other modules or tests.

Note that as for aidge_export_aidge, the other dependent modules builds (aidge_backend_cpu, ...) could be simplified once the commit A is merge to aidge_core, as with this change all dependencies are correctly defined.

Detailed major modifications

Now the installed aidge_core package include in its interface dependency the pybind11 headers which are installed along the package and the dependency on Python headers. Also a conditional dependency is added on the Python runtime (only for non module targets and in the case where AIDGE_REQUIRES_PYTHON and PYTHON_HAS_EMBED are both true).

This does not break the existing dependent build, though, once merged these build could remove weird dependency declaration, in particular:

  • Remove -DPYBIND preprocessor define in a module if it is not necessary (this should be discouraged actually)
  • Remove dependency on pybind/python for the module's archive (i.e. the infamous: target_link_libraries(${module_name} PUBLIC pybind11::pybind11 PRIVATE Python::Module))

TODO

Nope

Edited by Christophe Guillon

Merge request reports