Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Explicit and smart CMake target #2935

Merged
merged 2 commits into from
Oct 31, 2023
Merged

Explicit and smart CMake target #2935

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2c1efec
Explicit and smart CMake target
AntoinePrv Oct 25, 2023
0a9c023
Only expose explicit CMake targets
AntoinePrv Oct 31, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 15 additions & 5 deletions libmamba/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -600,14 +600,15 @@ macro(libmamba_create_target target_name linkage output_name)
endif ()

list(APPEND libmamba_targets ${target_name})
add_library(mamba::${target_name} ALIAS ${target_name})
endmacro()


set(libmamba_targets "")

if (BUILD_SHARED)
message(STATUS "Adding shared libmamba target")
libmamba_create_target(libmamba SHARED libmamba)
libmamba_create_target(libmamba-dyn SHARED libmamba)
endif ()

if (BUILD_STATIC)
Expand All @@ -620,9 +621,15 @@ if (BUILD_STATIC)
else ()
libmamba_create_target(libmamba-static STATIC libmamba)
endif ()
endif ()
endif()

if (NOT (BUILD_SHARED OR BUILD_STATIC))
if(BUILD_SHARED_LIBS AND BUILD_SHARED)
add_library(mamba::libmamba ALIAS libmamba-dyn)
elseif(BUILD_STATIC)
add_library(mamba::libmamba ALIAS libmamba-static)
elseif(BUILD_SHARED)
add_library(mamba::libmamba ALIAS libmamba-dyn)
else()
Comment on lines +626 to +632
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Continuing the discussion from https://github.com/mamba-org/mamba/pull/2935/files#r1375979846: what if BUILD_SHARED_LIBS AND BUILD_SHARED AND BUILD_STATIC?

Also, could it be simplified to:

Suggested change
if(BUILD_SHARED_LIBS AND BUILD_SHARED)
add_library(mamba::libmamba ALIAS libmamba-dyn)
elseif(BUILD_STATIC)
add_library(mamba::libmamba ALIAS libmamba-static)
elseif(BUILD_SHARED)
add_library(mamba::libmamba ALIAS libmamba-dyn)
else()
if(BUILD_SHARED_LIBS OR BUILD_SHARED)
add_library(mamba::libmamba ALIAS libmamba-dyn)
elseif(BUILD_STATIC)
add_library(mamba::libmamba ALIAS libmamba-static)
else()

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if BUILD_SHARED_LIBS is set but not BUILD_SHARED (which IMHO we should really scope with MAMBA_BUILD_SHARED), then the libmamba-dyn target will not exists. The Point of the mamba::libmamba alias is more "whaterver, as long as it works".

message(FATAL_ERROR "Select at least a build variant for libmamba")
endif ()

Expand Down Expand Up @@ -652,8 +659,11 @@ install(DIRECTORY "${LIBMAMBA_INCLUDE_DIR}/"
PATTERN "*.h")

# Makes the project importable from the build directory
export(EXPORT ${PROJECT_NAME}-targets
FILE "${CMAKE_CURRENT_BINARY_DIR}/${PROJECT_NAME}Targets.cmake")
export(
EXPORT ${PROJECT_NAME}-targets
FILE "${CMAKE_CURRENT_BINARY_DIR}/${PROJECT_NAME}Targets.cmake"
NAMESPACE mamba::
)

# Configure 'mambaConfig.cmake' for a build tree
set(MAMBA_CONFIG_CODE "####### Expanded from \@MAMBA_CONFIG_CODE\@ #######\n")
Expand Down
14 changes: 7 additions & 7 deletions libmamba/libmambaConfig.cmake.in
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,13 +34,13 @@ find_dependency(yaml-cpp)
if(NOT (TARGET libmamba OR TARGET libmamba-static))
include("${CMAKE_CURRENT_LIST_DIR}/@[email protected]")

if (TARGET libmamba-static)
get_target_property(@PROJECT_NAME@_INCLUDE_DIR libmamba-static INTERFACE_INCLUDE_DIRECTORIES)
get_target_property(@PROJECT_NAME@_STATIC_LIBRARY libmamba-static LOCATION)
endif ()

if (TARGET libmamba)
if (TARGET mamba::libmamba-dyn)
get_target_property(@PROJECT_NAME@_INCLUDE_DIR libmamba INTERFACE_INCLUDE_DIRECTORIES)
get_target_property(@PROJECT_NAME@_LIBRARY libmamba LOCATION)
endif ()
endif()

if (TARGET mamba::libmamba-static)
get_target_property(@PROJECT_NAME@_INCLUDE_DIR libmamba-static INTERFACE_INCLUDE_DIRECTORIES)
get_target_property(@PROJECT_NAME@_STATIC_LIBRARY libmamba-static LOCATION)
endif()
endif()
10 changes: 2 additions & 8 deletions libmamba/tests/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,8 @@
cmake_minimum_required(VERSION 3.16)

if (BUILD_SHARED)
set(libmamba_target "libmamba")
else ()
set(libmamba_target "libmamba-static")
endif()

add_executable(testing_libmamba_lock libmamba_lock/lock.cpp)

target_link_libraries(testing_libmamba_lock PUBLIC ${libmamba_target})
target_link_libraries(testing_libmamba_lock PUBLIC mamba::libmamba)

target_compile_features(testing_libmamba_lock PUBLIC cxx_std_17)

Expand Down Expand Up @@ -97,7 +91,7 @@ find_package(Threads REQUIRED)
target_link_libraries(
test_libmamba
PUBLIC
${libmamba_target}
mamba::libmamba
reproc
reproc++
PRIVATE
Expand Down
6 changes: 3 additions & 3 deletions libmambapy/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,9 @@ cmake_policy(SET CMP0057 NEW)

project(libmambapy)

if(NOT TARGET libmamba)
if(NOT TARGET mamba::libmamba)
find_package(libmamba REQUIRED)
endif ()
endif()

find_package(Python COMPONENTS Interpreter Development)
find_package(pybind11 REQUIRED)
Expand All @@ -25,7 +25,7 @@ pybind11_add_module(bindings

mamba_target_add_compile_warnings(bindings WARNING_AS_ERROR ${MAMBA_WARNING_AS_ERROR})

target_link_libraries(bindings PUBLIC pybind11::pybind11 libmamba)
target_link_libraries(bindings PRIVATE pybind11::pybind11 mamba::libmamba)
set_property(TARGET bindings PROPERTY CXX_STANDARD 17)

install(TARGETS bindings
Expand Down
22 changes: 3 additions & 19 deletions mamba_package/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,18 +10,6 @@ cmake_policy(SET CMP0077 NEW) # Introduced in cmake 3.13

project(mamba-package)

# Build options
# =============

if (BUILD_STATIC AND NOT BUILD_SHARED)
set(MAMBA_PACKAGE_LINKAGE "STATIC" CACHE STRING "mamba-package linkage against libraries")
else ()
set(MAMBA_PACKAGE_LINKAGE "DYNAMIC" CACHE STRING "mamba-package linkage against libraries")
endif ()

string(TOUPPER "${MAMBA_PACKAGE_LINKAGE}" linkage_upper)
message(STATUS "'mamba-package' linkage: ${MAMBA_PACKAGE_LINKAGE}")

# Source files
# ============

Expand All @@ -37,7 +25,7 @@ set(MAMBA_PACKAGE_HEADERS
# Dependencies
# ============

if(NOT (TARGET libmamba OR TARGET libmamba-static))
if(NOT TARGET mamba::libmamba)
find_package(libmamba REQUIRED)
endif ()

Expand All @@ -47,12 +35,8 @@ endif ()
add_executable(mamba-package ${MAMBA_PACKAGE_SRCS} ${MAMBA_PACKAGE_HEADERS})
mamba_target_add_compile_warnings(mamba-package WARNING_AS_ERROR ${MAMBA_WARNING_AS_ERROR})

if (${MAMBA_PACKAGE_LINKAGE} STREQUAL "STATIC")
target_link_libraries(mamba-package PRIVATE libmamba-static)
else ()
target_link_libraries(mamba-package PRIVATE libmamba)
endif ()
target_link_libraries(mamba-package PRIVATE mamba::libmamba)

set_property(TARGET mamba-package PROPERTY CXX_STANDARD 17)
set_target_properties(mamba-package PROPERTIES CXX_STANDARD 17)

install(TARGETS mamba-package)
6 changes: 3 additions & 3 deletions micromamba/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,17 +65,17 @@ add_executable(micromamba ${MICROMAMBA_SRCS} ${MICROMAMBA_HEADERS})
mamba_target_add_compile_warnings(micromamba WARNING_AS_ERROR ${MAMBA_WARNING_AS_ERROR})
mamba_target_set_lto(micromamba MODE ${MAMBA_LTO})

if(NOT (TARGET libmamba OR TARGET libmamba-static))
if(NOT (TARGET mamba::libmamba-dyn OR TARGET mamba::libmamba-static))
find_package(libmamba REQUIRED)
endif ()

string(TOUPPER "${MICROMAMBA_LINKAGE}" linkage_upper)
message(STATUS "Micromamba linkage: ${MICROMAMBA_LINKAGE}")

if (${MICROMAMBA_LINKAGE} STREQUAL "STATIC")
set(libmamba_target "libmamba-static")
set(libmamba_target mamba::libmamba-static)
else ()
set(libmamba_target "libmamba")
set(libmamba_target mamba::libmamba-dyn)
endif ()

find_package(Threads REQUIRED)
Expand Down
Loading