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

chore: some CMake cleanup #3564

Merged
merged 2 commits into from
Oct 28, 2024
Merged

chore: some CMake cleanup #3564

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c3a6180
chore: some CMake cleanup
henryiii Oct 23, 2024
08461e7
Update libmamba/CMakeLists.txt
henryiii Oct 25, 2024
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
2 changes: 0 additions & 2 deletions CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,6 @@
# The full license is in the file LICENSE, distributed with this software.

cmake_minimum_required(VERSION 3.16)
cmake_policy(SET CMP0025 NEW) # Introduced in cmake 3.0
cmake_policy(SET CMP0077 NEW) # Introduced in cmake 3.13
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Setting the minimum version of CMake sets all policies to NEW that were introduced on or before that value. (Also true if you set a version range, like 3.16...3.30)


project(mamba)

Expand Down
2 changes: 1 addition & 1 deletion docs/source/developer_zone/dev_environment.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ Install development dependencies
.. code:: bash

micromamba create -n mamba -c conda-forge -f dev/environment-dev.yml
micromamba activate -n mamba
micromamba activate mamba
Copy link
Contributor Author

Choose a reason for hiding this comment

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

There's no -n here.


Compile ``libmamba`` and other artifacts
****************************************
Expand Down
2 changes: 1 addition & 1 deletion docs/source/installation/micromamba-installation.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ To build from source, install the development dependencies, using a Conda compat
.. code-block:: bash

micromamba create -n mamba --file dev/environment-micromamba-static.yml
micromamba activate -n mamba
micromamba activate mamba

Use CMake from this environment to drive the build:

Expand Down
20 changes: 10 additions & 10 deletions libmamba/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -405,15 +405,15 @@ set(
# Targets and link
# ================

find_package(fmt REQUIRED)
find_package(spdlog REQUIRED)
find_package(tl-expected REQUIRED)
find_package(nlohmann_json REQUIRED)
find_package(simdjson REQUIRED)
find_package(yaml-cpp REQUIRED)
find_package(reproc REQUIRED)
find_package(reproc++ REQUIRED)
find_package(Libsolv REQUIRED)
find_package(fmt CONFIG REQUIRED)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Using CONFIG explicitly makes the error message better. I didn't go through every one to figure out whether they were CONFIG or MODULE, but these are some of the first ones hit, so error message is important for these.

find_package(spdlog CONFIG REQUIRED)
find_package(tl-expected CONFIG REQUIRED)
find_package(nlohmann_json CONFIG REQUIRED)
find_package(simdjson CONFIG REQUIRED)
find_package(yaml-cpp CONFIG REQUIRED)
find_package(reproc CONFIG REQUIRED)
find_package(reproc++ CONFIG REQUIRED)
find_package(Libsolv MODULE REQUIRED)
add_subdirectory(ext/solv-cpp)

macro(libmamba_create_target target_name linkage output_name)
Expand Down Expand Up @@ -539,7 +539,7 @@ macro(libmamba_create_target target_name linkage output_name)
set(CMAKE_PREFIX_PATH "$ENV{VCPKG_ROOT}/installed/x64-windows-static-md/")

# For Windows we have a vcpkg based build system right now.
find_package(LibArchive REQUIRED)
find_package(LibArchive CONFIG REQUIRED)
henryiii marked this conversation as resolved.
Show resolved Hide resolved
find_package(CURL CONFIG REQUIRED)
find_library(LIBLZMA_LIBRARIES lzma REQUIRED)
find_library(LZ4_LIBRARY NAMES lz4)
Expand Down
10 changes: 3 additions & 7 deletions libmambapy/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,20 +7,16 @@ cmake_minimum_required(VERSION 3.18.2)

include("../cmake/CompilerWarnings.cmake")

cmake_policy(SET CMP0025 NEW)
cmake_policy(SET CMP0077 NEW)
cmake_policy(SET CMP0057 NEW)

project(libmambapy)

if(NOT TARGET mamba::libmamba)
find_package(libmamba REQUIRED)
find_package(libmamba CONFIG REQUIRED)
set(libmamba_target mamba::libmamba-dyn)
else()
set(libmamba_target mamba::libmamba)
endif()

find_package(Python COMPONENTS Interpreter Development)
find_package(Python COMPONENTS Interpreter Development.Module)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The default Development looks for the Embed target too, which isn't required and sometimes (like on manylinux) not available.

find_package(pybind11 REQUIRED)

pybind11_add_module(
Expand Down Expand Up @@ -49,7 +45,7 @@ target_compile_features(bindings PRIVATE cxx_std_17)
if(SKBUILD)
install(TARGETS bindings DESTINATION ${MAMBA_INSTALL_PYTHON_EXT_LIBDIR})
else()
# WARNING: this default should probably not be used as it is but set extranlly by a proper
# WARNING: this default should probably not be used as it is but set externally by a proper
# Python packager tool
set(
MAMBA_INSTALL_PYTHON_EXT_LIBDIR
Expand Down
Loading