From e83f0b6febfb45f70aa3732118b4fe9e278c843f Mon Sep 17 00:00:00 2001
From: Christophe Guillon <christophe.guillon@inria.fr>
Date: Mon, 26 Aug 2024 18:26:55 +0200
Subject: [PATCH] [Build] Update aidge_export_aidge build

Remove unecessary aidge_export_aidge build dependencies.
Add inclusion of cpu TensorImpl to default main.cpp.
Change test_export.py unit test to build only the
python binding such that the test works under cibuildwheel.
Fix test_export.py to not install tests in the
libAidge installation directory.
---
 .../aidge_export_aidge/static/CMakeLists.txt  | 27 +++++++++----------
 .../static/cmake/PybindModuleCreation.cmake   | 13 ++++-----
 .../static/export-config.cmake.in             |  5 ++++
 aidge_core/aidge_export_aidge/static/main.cpp |  4 +++
 aidge_core/unit_tests/static/main.cpp         |  4 +++
 aidge_core/unit_tests/test_export.py          | 12 ++++++---
 pyproject.toml                                | 18 ++++++-------
 7 files changed, 48 insertions(+), 35 deletions(-)

diff --git a/aidge_core/aidge_export_aidge/static/CMakeLists.txt b/aidge_core/aidge_export_aidge/static/CMakeLists.txt
index 4220bb9d5..d7fe26d9c 100644
--- a/aidge_core/aidge_export_aidge/static/CMakeLists.txt
+++ b/aidge_core/aidge_export_aidge/static/CMakeLists.txt
@@ -1,4 +1,4 @@
-cmake_minimum_required(VERSION 3.15)
+cmake_minimum_required(VERSION 3.18)
 set(CXX_STANDARD 14)
 
 file(STRINGS "${CMAKE_SOURCE_DIR}/project_name.txt" project_name)
@@ -18,6 +18,7 @@ set(module_name _${CMAKE_PROJECT_NAME}) # target name
 ##############################################
 # Define options
 option(PYBIND "python binding" ON)
+option(STANDALONE "Build standalone executable" ON)
 option(WERROR "Warning as error" OFF)
 option(TEST "Enable tests" OFF)
 option(COVERAGE "Enable coverage" OFF)
@@ -61,16 +62,8 @@ set_property(TARGET ${module_name} PROPERTY POSITION_INDEPENDENT_CODE ON)
 
 # PYTHON BINDING
 if (PYBIND)
-    # Handles Python + pybind11 headers dependencies
     include(PybindModuleCreation)
     generate_python_binding(${CMAKE_PROJECT_NAME} ${module_name})
-
-    target_link_libraries(${module_name}
-        PUBLIC
-            pybind11::pybind11
-        PRIVATE
-            Python::Python
-        )
 endif()
 
 if( ${ENABLE_ASAN} )
@@ -94,7 +87,6 @@ target_include_directories(${module_name}
         ${CMAKE_CURRENT_SOURCE_DIR}/src
 )
 
-target_link_libraries(${module_name} PUBLIC fmt::fmt)
 target_compile_features(${module_name} PRIVATE cxx_std_14)
 
 target_compile_options(${module_name} PRIVATE
@@ -151,8 +143,13 @@ install(FILES
 ## Exporting from the build tree
 message(STATUS "Exporting created targets to use them in another build")
 export(EXPORT ${CMAKE_PROJECT_NAME}-targets
-    FILE "${CMAKE_CURRENT_BINARY_DIR}/${project}-targets.cmake")
-
-# Compile executable
-add_executable(main main.cpp)
-target_link_libraries(main PUBLIC _aidge_core ${module_name})
+    FILE "${CMAKE_CURRENT_BINARY_DIR}/${CMAKE_PROJECT_NAME}-targets.cmake")
+
+if(STANDALONE)
+    if(AIDGE_REQUIRES_PYTHON AND NOT AIDGE_PYTHON_HAS_EMBED)
+        message(WARNING "Skipping compilation of standalone executable: missing Python embedded interpreter")
+    else()
+        add_executable(main main.cpp)
+        target_link_libraries(main PRIVATE ${module_name})
+    endif()
+endif()
diff --git a/aidge_core/aidge_export_aidge/static/cmake/PybindModuleCreation.cmake b/aidge_core/aidge_export_aidge/static/cmake/PybindModuleCreation.cmake
index 193f33322..217a48351 100644
--- a/aidge_core/aidge_export_aidge/static/cmake/PybindModuleCreation.cmake
+++ b/aidge_core/aidge_export_aidge/static/cmake/PybindModuleCreation.cmake
@@ -1,8 +1,7 @@
 function(generate_python_binding name target_to_bind)
 
-    find_package(Python COMPONENTS Interpreter Development)
+    find_package(Python COMPONENTS Interpreter Development.Module)
 
-    add_definitions(-DPYBIND)
     Include(FetchContent)
     FetchContent_Declare(
     PyBind11
@@ -15,11 +14,9 @@ function(generate_python_binding name target_to_bind)
     file(GLOB_RECURSE pybind_src_files "python_binding/*.cpp")
 
     pybind11_add_module(${name} MODULE ${pybind_src_files} "NO_EXTRAS") # NO EXTRA recquired for pip install
-    target_include_directories(${name} PUBLIC "python_binding")
+    target_include_directories(${name} PRIVATE "python_binding")
+
+    # Link target library to bind
+    target_link_libraries(${name} PRIVATE ${target_to_bind})
 
-    # Handles Python + pybind11 headers dependencies
-    target_link_libraries(${name}
-        PUBLIC
-            ${target_to_bind}
-    )
 endfunction()
diff --git a/aidge_core/aidge_export_aidge/static/export-config.cmake.in b/aidge_core/aidge_export_aidge/static/export-config.cmake.in
index f3604be11..f0be5e076 100644
--- a/aidge_core/aidge_export_aidge/static/export-config.cmake.in
+++ b/aidge_core/aidge_export_aidge/static/export-config.cmake.in
@@ -1,3 +1,8 @@
+@PACKAGE_INIT@
+
+include(CMakeFindDependencyMacro)
+find_dependency(aidge_core)
+
 include(${CMAKE_CURRENT_LIST_DIR}/aidge_backend_cpu-config-version.cmake)
 
 include(${CMAKE_CURRENT_LIST_DIR}/aidge_backend_cpu-targets.cmake)
diff --git a/aidge_core/aidge_export_aidge/static/main.cpp b/aidge_core/aidge_export_aidge/static/main.cpp
index ab8bac185..61bc3ebeb 100644
--- a/aidge_core/aidge_export_aidge/static/main.cpp
+++ b/aidge_core/aidge_export_aidge/static/main.cpp
@@ -1,6 +1,10 @@
 #include <iostream>
 #include <aidge/backend/cpu.hpp>
 
+/* Register default cpu Tensor implementation */
+#include <aidge/backend/cpu/data/TensorImpl.hpp>
+
+/* Include model generator */
 #include "include/dnn.hpp"
 
 int main()
diff --git a/aidge_core/unit_tests/static/main.cpp b/aidge_core/unit_tests/static/main.cpp
index 06171e2a0..640fc1fe6 100644
--- a/aidge_core/unit_tests/static/main.cpp
+++ b/aidge_core/unit_tests/static/main.cpp
@@ -4,6 +4,10 @@ This file is copied in the test export.
 */
 #include <iostream>
 
+/* Register default cpu Tensor implementation */
+#include <aidge/backend/cpu/data/TensorImpl.hpp>
+
+/* Include model generator */
 #include "include/dnn.hpp"
 
 int main()
diff --git a/aidge_core/unit_tests/test_export.py b/aidge_core/unit_tests/test_export.py
index 9fb16128e..5d2e700a8 100644
--- a/aidge_core/unit_tests/test_export.py
+++ b/aidge_core/unit_tests/test_export.py
@@ -65,6 +65,7 @@ class test_export(unittest.TestCase):
     def setUp(self):
         self.EXPORT_PATH: pathlib.Path = pathlib.Path("dummy_export")
         self.BUILD_DIR: pathlib.Path = self.EXPORT_PATH / "build"
+        self.INSTALL_DIR: pathlib.Path = (self.EXPORT_PATH / "install").absolute()
 
     def tearDown(self):
         pass
@@ -96,9 +97,10 @@ class test_export(unittest.TestCase):
         )
         os.makedirs(self.BUILD_DIR, exist_ok=True)
         clean_dir(self.BUILD_DIR)  # if build dir existed already ensure its emptyness
+        clean_dir(self.INSTALL_DIR)
 
         # Test compilation of export
-        install_path = (
+        search_path = (
             os.path.join(sys.prefix, "lib", "libAidge")
             if "AIDGE_INSTALL" not in os.environ
             else os.environ["AIDGE_INSTALL"]
@@ -116,14 +118,16 @@ class test_export(unittest.TestCase):
                 [
                     "cmake",
                     str(self.EXPORT_PATH.absolute()),
-                    "-DPYBIND=1",
-                    f"-DCMAKE_INSTALL_PREFIX:PATH={install_path}",
+                    "-DPYBIND=ON",
+                    f"-DCMAKE_PREFIX_PATH={search_path}", # search dependencies
+                    f"-DCMAKE_INSTALL_PREFIX:PATH={self.INSTALL_DIR}", # local install
                 ],
                 cwd=str(self.BUILD_DIR),
             ):
                 print(std_line, end="")
         except subprocess.CalledProcessError as e:
             print(f"An error occurred: {e}\nFailed to configure export.")
+            raise SystemExit(1)
 
         ##########################
         # BUILD EXPORT
@@ -135,6 +139,7 @@ class test_export(unittest.TestCase):
                 print(std_line, end="")
         except subprocess.CalledProcessError as e:
             print(f"An error occurred: {e}\nFailed to build export.")
+            raise SystemExit(1)
 
         ##########################
         # INSTALL EXPORT
@@ -146,6 +151,7 @@ class test_export(unittest.TestCase):
                 print(std_line, end="")
         except subprocess.CalledProcessError as e:
             print(f"An error occurred: {e}\nFailed to install export.")
+            raise SystemExit(1)
 
 
 if __name__ == "__main__":
diff --git a/pyproject.toml b/pyproject.toml
index 031972c71..3c32fbcf4 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -51,15 +51,15 @@ write_to = "aidge_core/_version.py"
 [tool.cibuildwheel]
 build-frontend = "build"
 test-requires = "pytest"
-# FIXME: The ignored export test requires a to build the generated export via cmake.
-# However due to a strange bug I haven't been able to properly link Python::Module to the export target
-# Resulting in the need to link Python::Python which is the python interpreter.
-# This suppresses the issue but sadly this target is not available on the cibuilwheel image.
-# Hence the test is ignored. If you want to try and solve this bug go on. 
-# Just take care to increment the counter just below.
-# 
-# Work time spent on this bug : 24h
-test-command = "pytest --ignore={package}/aidge_core/unit_tests/test_export.py {package}/aidge_core/unit_tests"
+# WARNING: in the test suite the `test_export.py` used to be skipped
+# because it did not build when the python embedded interpreter is not available
+# as it is the case for cibuildwheel containers.
+# Now the build system takes care of this and skips the generation of a standalone
+# executable when it is not possible.
+# The root causes for this conditional build is that 1. the python embedded interpreter
+# is not alweays available, and 2. the aidge_core library depends on it as of now.
+# Hopefully this latter dependency may be removed in the future, simplifying the build.
+test-command = "pytest {package}/aidge_core/unit_tests"
 # uncomment to run cibuildwheel locally on selected distros
 # build=[
 # "cp38-manylinux_x86_64",
-- 
GitLab