fsfw/fsfw
fsfw/fsfw
10
3
Fork 6
Code Issues 31 Pull Requests 4 Releases 10 Wiki Activity

Hotfix CMake & ETL #604

Merged
gaisser merged 6 commits from mueller/hotfix-etl into development 2022-05-02 14:36:33 +02:00
Conversation 0 Commits 6 Files Changed 1 +14 -14
1 changed files with 14 additions and 14 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit c80f06fbcb - Show all commits

28
CMakeLists.txt
View File

@ -108,23 +108,23 @@ endif()
message(STATUS "Finding and/or providing ETL library")
# Check whether the user has already installed ETL first
find_package(${FSFW_ETL_LIB_NAME} ${FSFW_ETL_LIB_MAJOR_VERSION} QUIET)
# find_package(${FSFW_ETL_LIB_NAME} ${FSFW_ETL_LIB_MAJOR_VERSION} QUIET)
# Not installed, so use FetchContent to download and provide etl
if(NOT ${FSFW_ETL_LIB_NAME}_FOUND)
message(STATUS
"No ETL installation was found with find_package. Installing and providing "
"etl with FindPackage"
)
include(FetchContent)
# if(NOT ${FSFW_ETL_LIB_NAME}_FOUND)
message(STATUS
"No ETL installation was found with find_package. Installing and providing "
"etl with FindPackage"
)
include(FetchContent)
FetchContent_Declare(
${FSFW_ETL_LIB_NAME}
GIT_REPOSITORY https://github.com/ETLCPP/etl
GIT_TAG ${FSFW_ETL_LIB_VERSION}
)
FetchContent_Declare(
${FSFW_ETL_LIB_NAME}
GIT_REPOSITORY https://github.com/ETLCPP/etl
GIT_TAG ${FSFW_ETL_LIB_VERSION}
)
FetchContent_MakeAvailable(etl)
endif()
FetchContent_MakeAvailable(etl)
# endif()
gaisser marked this conversation as resolved Outdated
gaisser commented 2022-04-27 10:00:01 +02:00
Outdated
Review
Copy Link

If I read this correct, we should only call FetchContent_MakeAvailable once:

# WRONG: Should declare all details first
FetchContent_Declare(uses_other ...)
FetchContent_MakeAvailable(uses_other)

FetchContent_Declare(other ...)    # Will be ignored, uses_other beat us to it
FetchContent_MakeAvailable(other)  # Would use details declared by uses_other

# CORRECT: All details declared first, so they will take priority
FetchContent_Declare(uses_other ...)
FetchContent_Declare(other ...)
FetchContent_MakeAvailable(uses_other other)

This ensures dependency collection works correct for dependencies of the dependency (which we might not have ATM)

If I read [this](https://cmake.org/cmake/help/latest/module/FetchContent.html) correct, we should only call FetchContent_MakeAvailable once: ``` # WRONG: Should declare all details first FetchContent_Declare(uses_other ...) FetchContent_MakeAvailable(uses_other) FetchContent_Declare(other ...) # Will be ignored, uses_other beat us to it FetchContent_MakeAvailable(other) # Would use details declared by uses_other ``` ``` # CORRECT: All details declared first, so they will take priority FetchContent_Declare(uses_other ...) FetchContent_Declare(other ...) FetchContent_MakeAvailable(uses_other other) ``` This ensures dependency collection works correct for dependencies of the dependency (which we might not have ATM)
muellerr commented 2022-04-27 13:24:00 +02:00
Outdated
Review
Copy Link

The Catch2 dependency only needs to be loaded for unit tests I think

The Catch2 dependency only needs to be loaded for unit tests I think
muellerr commented 2022-04-27 13:26:03 +02:00
Outdated
Review
Copy Link

This could be solved with an if clause which either makes etl available or makes both available depending on whether Catch2 was loaded.

Or maybe MakeAvailable can deal with undeclared dependencies.. maybe I will test this

This could be solved with an if clause which either makes etl available or makes both available depending on whether Catch2 was loaded. Or maybe MakeAvailable can deal with undeclared dependencies.. maybe I will test this
set(FSFW_CORE_INC_PATH "inc")