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 +2 -2
1 changed files with 2 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit 17e609c3a5 - Show all commits

4
CMakeLists.txt
View File

@ -124,8 +124,8 @@ if(NOT ${FSFW_ETL_LIB_NAME}_FOUND)
GIT_TAG ${FSFW_ETL_LIB_VERSION}
)
FetchContent_MakeAvailable(etl)
add_library(${FSFW_ETL_LINK_TARGET} ALIAS etl)
FetchContent_MakeAvailable(${FSFW_ETL_LIB_NAME})
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
add_library(${FSFW_ETL_LINK_TARGET} ALIAS ${FSFW_ETL_LIB_NAME})
endif()
set(FSFW_CORE_INC_PATH "inc")