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

Jump to bottom

Replace internal dependencies by FetchContent #1583

Merged
merged 34 commits into from
Sep 5, 2024
Merged

Replace internal dependencies by FetchContent #1583

Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
b05ad8f
Remove specific version from openPMD json find_package() command
DerNils-git May 24, 2023
8f7e3e8
Merge branch 'openPMD:dev' into dev
DerNils-git Jul 3, 2023
620d861
Replace internal dependency on nlohmann_json by FetchContent
DerNils-git Jul 17, 2023
6136a97
Update required cmake version for fetchContent and add openPMD interf…
DerNils-git Jul 19, 2023
41a4658
Add nlohmann_json inculde directories
DerNils-git Jul 19, 2023
dc57e24
Revert "Add nlohmann_json inculde directories"
DerNils-git Jul 19, 2023
74fe22c
Revert "Update required cmake version for fetchContent and add openPM…
DerNils-git Jul 19, 2023
78448ea
Revert "Replace internal dependency on nlohmann_json by FetchContent"
DerNils-git Jul 19, 2023
803c359
Merge branch 'openPMD:dev' into add_cmake_fetchContent
DerNils-git Dec 31, 2023
0ba14dc
Replace internal JSON by CMake FetchContent command.
DerNils-git Dec 31, 2023
d8cda47
Replace the old json directory by FetchContent data.
DerNils-git Dec 31, 2023
7d90228
Replace internal TOML11 library by a FetchContent call
DerNils-git Dec 31, 2023
1402795
Replace internal Catch2 library by a FetchContent call
DerNils-git Dec 31, 2023
2557842
Update versions within FetchContent to stay with the latest version
DerNils-git Jan 3, 2024
8d7dbd6
Set C++ Standard to 17 and do not move Catch2 to v3 yet.
DerNils-git Jan 3, 2024
0e624d4
Remove version updates. Update README.md
DerNils-git Jan 4, 2024
73189c5
Replace PyBind11 dependency by FetchContent
DerNils-git Jan 4, 2024
1c71d9a
Fix pybind11 not found
DerNils-git Jan 4, 2024
0084e9b
Remove pybind11 from repo. Add pybind11 path to gitignore. Simplify p…
DerNils-git Jan 6, 2024
a4ab2b5
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Jan 6, 2024
acc6e07
Correct Download message for Catch2
DerNils-git Jan 16, 2024
5c8f968
More detailed description of FetchContent in README.md
DerNils-git Jan 16, 2024
5976cc9
Request cmake>=3.24 in appveyor CI build
franzpoeschel Jan 18, 2024
cbdcc1c
Remove OVERRIDE_FIND_PACKAGE to reduce cmake to version 3.22.
DerNils-git Jan 21, 2024
ea5b8db
Revert "Request cmake>=3.24 in appveyor CI build"
franzpoeschel Jan 22, 2024
16a9928
Change to trigger CI.
DerNils-git Jan 24, 2024
0f280e2
Use SOURCE_DIR only in CMake >= 3.23
franzpoeschel Jan 25, 2024
63f239a
Merge remote-tracking branch 'mainline/dev' into add_cmake_fetchConte…
ax3l Sep 5, 2024
7b8c6ed
Keep Thirdparty in this PR
ax3l Sep 5, 2024
a87abab
Doc: CMake 3.22.0+
ax3l Sep 5, 2024
fc87c50
Keep Explicit Superbuild Control
ax3l Sep 5, 2024
c04fe38
FetchContent: Repo & Branch/Tag Control
ax3l Sep 5, 2024
4d77402
Nested Superbuild Support
ax3l Sep 5, 2024
cce18a8
Update `.gitignore`
ax3l Sep 5, 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
11 changes: 11 additions & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,17 @@ Temporary Items
.spack-env/
spack.lock

#########
# CMake #
#########
CMakeUserPresets.json

# Directories which are created and filled by FetchContent
share/openPMD/thirdParty/json/
share/openPMD/thirdParty/toml11/
share/openPMD/thirdParty/catch2/
share/openPMD/thirdParty/pybind11/
Copy link
Contributor

Choose a reason for hiding this comment

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

We should be aware that with this, the build folder will modify the source tree.
What does the workflow look like switching between different openPMD versions that specify different dependency versions. Will FetchContent notice and update the dependencies' versions upon configuring?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That is true and might be a problem. I don't have experience with this.

Moving from lower to higher versions of OpenPMD should not be a problem. These directories have been deleted using git rm and should therefore be deleted when moving forward. FetchContent can then download into these directories w/o problem.

A problem might arise if one moves from a higher to a lower version. If the directories have been populated by FetchContent and git tries to add the old libraries again in the same directories I can not predict the behavior.

It would be possible to use a different directory with FetchContent until we are sure that it works as expected.

Copy link
Contributor

Choose a reason for hiding this comment

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

Since @ax3l uses FetchContent in WarpX, let's wait until he is back to see what their approach looks like (relevant bits in their CMake config: https://github.com/ECP-WarpX/WarpX/blob/11e2a1722aac8929db0ed677f634673d235c1396/cmake/dependencies/openPMD.cmake#L29)

Copy link
Member

Choose a reason for hiding this comment

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

I'll move this to the build directory, as we do in other project.


#########
# Tools #
#########
Expand Down
138 changes: 59 additions & 79 deletions CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# Preamble ####################################################################
#
cmake_minimum_required(VERSION 3.15.0)
cmake_minimum_required(VERSION 3.24.0)

project(openPMD VERSION 0.16.0) # LANGUAGES CXX

Expand All @@ -9,23 +9,6 @@ set(openPMD_STANDARD_VERSION 1.1.0)

include(${openPMD_SOURCE_DIR}/cmake/openPMDFunctions.cmake)


# CMake policies ##############################################################
#
# Search in <PackageName>_ROOT:
# https://cmake.org/cmake/help/v3.12/policy/CMP0074.html
if(POLICY CMP0074)
cmake_policy(SET CMP0074 NEW)
endif()

# We use simple syntax in cmake_dependent_option, so we are compatible with the
# extended syntax in CMake 3.22+
# https://cmake.org/cmake/help/v3.22/policy/CMP0127.html
if(POLICY CMP0127)
cmake_policy(SET CMP0127 NEW)
endif()


# No in-Source builds #########################################################
#
# In-source builds clutter up the source directory and lead to mistakes with
Expand Down Expand Up @@ -148,10 +131,6 @@ openpmd_option(PYTHON "Enable Python bindings" AUTO)
option(openPMD_INSTALL "Add installation targets" ON)
option(openPMD_INSTALL_RPATH "Add RPATHs to installed binaries" ON)
option(openPMD_HAVE_PKGCONFIG "Generate a .pc file for pkg-config" ON)
option(openPMD_USE_INTERNAL_CATCH "Use internally shipped Catch2" ON)
option(openPMD_USE_INTERNAL_PYBIND11 "Use internally shipped pybind11" ON)
option(openPMD_USE_INTERNAL_JSON "Use internally shipped nlohmann-json" ON)
option(openPMD_USE_INTERNAL_TOML11 "Use internally shipped toml11" ON)

option(openPMD_USE_INVASIVE_TESTS "Enable unit tests that modify source code" OFF)
option(openPMD_USE_VERIFY "Enable internal VERIFY (assert) macro independent of build type" ON)
Expand Down Expand Up @@ -233,6 +212,7 @@ endfunction()
# Clang+MPI: Potentially needed MPI::MPI_C targets in the past
# (exact MPI flavor & Clang version lost)
# BullMPI: PUBLIC dependency to MPI::MPI_CXX is missing in MPI::MPI_C target
include(FetchContent)
set(openPMD_MPI_LINK_C_DEFAULT OFF)
option(openPMD_MPI_LINK_C "Also link the MPI C targets" ${openPMD_MPI_LINK_C_DEFAULT})
mark_as_advanced(openPMD_MPI_LINK_C)
Expand All @@ -259,27 +239,39 @@ endif()


# external library: nlohmann-json (required)
if(openPMD_USE_INTERNAL_JSON)
set(JSON_BuildTests OFF CACHE INTERNAL "")
set(JSON_Install OFF CACHE INTERNAL "") # only used PRIVATE
add_subdirectory("${openPMD_SOURCE_DIR}/share/openPMD/thirdParty/json")
message(STATUS "nlohmann-json: Using INTERNAL version '3.9.1'")
else()
find_package(nlohmann_json 3.9.1 CONFIG REQUIRED)
message(STATUS "nlohmann-json: Found version '${nlohmann_json_VERSION}'")
find_package(nlohmann_json CONFIG)
if(NOT nlohmann_json_FOUND)
message(STATUS "Fetching nlohmann-json from https://github.com/nlohmann/json")
FetchContent_Declare(
nlohmann_json
GIT_REPOSITORY https://github.com/nlohmann/json.git
GIT_TAG v3.11.3
SOURCE_DIR "${openPMD_SOURCE_DIR}/share/openPMD/thirdParty/json/"
OVERRIDE_FIND_PACKAGE
)
set(JSON_BuildTests OFF CACHE INTERNAL "NLohmann JSON option defiend internally by OpenPMD")
set(JSON_Install OFF CACHE INTERNAL "NLohmann JSON option defiend internally by OpenPMD") # only used PRIVATE
FetchContent_MakeAvailable(nlohmann_json)
endif()
add_library(openPMD::thirdparty::nlohmann_json INTERFACE IMPORTED)
target_link_libraries(openPMD::thirdparty::nlohmann_json
INTERFACE nlohmann_json::nlohmann_json)

# external library: toml11
if(openPMD_USE_INTERNAL_TOML11)
set(toml11_INSTALL OFF CACHE INTERNAL "")
add_subdirectory("${openPMD_SOURCE_DIR}/share/openPMD/thirdParty/toml11")
message(STATUS "toml11: Using INTERNAL version '3.7.1'")
else()
find_package(toml11 3.7.1 CONFIG REQUIRED)
message(STATUS "toml11: Found version '${toml11_VERSION}'")
find_package(toml11 CONFIG)
if(NOT toml11_FOUND)
message(STATUS "Fetching toml11 from https://github.com/ToruNiina/toml11")
FetchContent_Declare(
toml11
GIT_REPOSITORY https://github.com/ToruNiina/toml11
# Migrate to the latest commit to remove CMake Warning which is not yet
# available in any official release.
Copy link
Contributor

Choose a reason for hiding this comment

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

You mean the warning below? Do I understand correctly that this has been fixed on toml11 dev and we are waiting for the next release?

CMake Deprecation Warning at share/openPMD/thirdParty/toml11/CMakeLists.txt:1 (cmake_minimum_required):                                                                                                                    
  Compatibility with CMake < 3.5 will be removed from a future version of                                                                                                                                                  
  CMake.                                                                                                                                                                                                                   
                                                                                                                                                                                                                           
  Update the VERSION argument <min> value or use a ...<max> suffix to tell                                                                                                                                                 
  CMake that the project does not need compatibility with older versions.

Copy link
Contributor Author

@DerNils-git DerNils-git Jan 16, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Yes.
But also moving to the last commit is not trivial with toml11 since that requires CMAKE_CXX_STANDARD to be set which I could not add to openPMD-api w/o breaking the CI.

Copy link
Contributor

Choose a reason for hiding this comment

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

since that requires CMAKE_CXX_STANDARD to be set which I could not add to openPMD-api w/o breaking the CI.

That's probably an issue unrelated to this PR which we should better solve separately?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, after finishing this PR I would try to Update catch2 and toml11 with separate issues as described above

GIT_TAG v3.7.1
SOURCE_DIR "${openPMD_SOURCE_DIR}/share/openPMD/thirdParty/toml11/"
OVERRIDE_FIND_PACKAGE
)
set(toml11_INSTALL OFF CACHE INTERNAL "toml11 option defiend internally by OpenPMD")
FetchContent_MakeAvailable(toml11)
endif()
add_library(openPMD::thirdparty::toml11 INTERFACE IMPORTED)
target_link_libraries(openPMD::thirdparty::toml11
Expand Down Expand Up @@ -396,47 +388,31 @@ endif()

# external library: pybind11 (optional)
set(_PY_DEV_MODULE Development.Module)
if(CMAKE_VERSION VERSION_LESS 3.18.0)
# over-specification needed for CMake<3.18
# https://pybind11.readthedocs.io/en/latest/compiling.html#findpython-mode
# https://cmake.org/cmake/help/v3.18/module/FindPython.html
set(_PY_DEV_MODULE Development)
endif()
if(openPMD_USE_PYTHON STREQUAL AUTO)
find_package(Python 3.8.0 COMPONENTS Interpreter ${_PY_DEV_MODULE})
if(Python_FOUND)
if(openPMD_USE_INTERNAL_PYBIND11)
add_subdirectory("${openPMD_SOURCE_DIR}/share/openPMD/thirdParty/pybind11")
set(openPMD_HAVE_PYTHON TRUE)
message(STATUS "pybind11: Using INTERNAL version 2.11.1")
else()
find_package(pybind11 2.11.1 CONFIG)
if(pybind11_FOUND)
set(openPMD_HAVE_PYTHON TRUE)
message(STATUS "pybind11: Found version '${pybind11_VERSION}'")
else()
set(openPMD_HAVE_PYTHON FALSE)
endif()
endif()
else()
set(openPMD_HAVE_PYTHON FALSE)
endif()
elseif(openPMD_USE_PYTHON)
find_package(Python COMPONENTS Interpreter ${_PY_DEV_MODULE} REQUIRED)
if(openPMD_USE_INTERNAL_PYBIND11)
add_subdirectory("${openPMD_SOURCE_DIR}/share/openPMD/thirdParty/pybind11")
set(openPMD_HAVE_PYTHON TRUE)
message(STATUS "pybind11: Using INTERNAL version 2.11.1")
else()
find_package(pybind11 2.11.1 REQUIRED CONFIG)
set(openPMD_HAVE_PYTHON TRUE)
message(STATUS "pybind11: Found version '${pybind11_VERSION}'")
else()
set(openPMD_HAVE_PYTHON FALSE)
endif()
if(Python_FOUND)
find_package(pybind11 CONFIG)
if(NOT pybind11_FOUND)
message(STATUS "Fetching PyBind11 from https://github.com/pybind/pybind11")
FetchContent_Declare(
pybind11
GIT_REPOSITORY https://github.com/pybind/pybind11.git
GIT_TAG v2.11.1
SOURCE_DIR "${openPMD_SOURCE_DIR}/share/openPMD/thirdParty/pybind11/"
OVERRIDE_FIND_PACKAGE
)
FetchContent_MakeAvailable(pybind11)
endif()
set(openPMD_HAVE_PYTHON TRUE)
else()
set(openPMD_HAVE_PYTHON FALSE)
endif()


# Targets #####################################################################
#
set(CORE_SOURCE
Expand Down Expand Up @@ -550,18 +526,22 @@ target_include_directories(openPMD PUBLIC

# Catch2 for unit tests
if(openPMD_BUILD_TESTING)
add_library(openPMD::thirdparty::Catch2 INTERFACE IMPORTED)
if(openPMD_USE_INTERNAL_CATCH)
target_include_directories(openPMD::thirdparty::Catch2 SYSTEM INTERFACE
$<BUILD_INTERFACE:${openPMD_SOURCE_DIR}/share/openPMD/thirdParty/catch2/include>
find_package(Catch2 CONFIG)
if(NOT Catch2_FOUND)
message(STATUS "Fetching toml11 from https://github.com/catchorg/Catch2")
DerNils-git marked this conversation as resolved.
Show resolved Hide resolved
FetchContent_Declare(
Catch2
GIT_REPOSITORY https://github.com/catchorg/Catch2.git
# ToDo Migrate to v3 and latest release
GIT_TAG v2.13.10
SOURCE_DIR "${openPMD_SOURCE_DIR}/share/openPMD/thirdParty/catch2/"
OVERRIDE_FIND_PACKAGE
)
message(STATUS "Catch2: Using INTERNAL version '2.13.10'")
else()
find_package(Catch2 2.13.10 REQUIRED CONFIG)
target_link_libraries(openPMD::thirdparty::Catch2
INTERFACE Catch2::Catch2)
message(STATUS "Catch2: Found version '${Catch2_VERSION}'")
FetchContent_MakeAvailable(Catch2)
endif()
add_library(openPMD::thirdparty::Catch2 INTERFACE IMPORTED)
target_link_libraries(openPMD::thirdparty::Catch2
INTERFACE Catch2::Catch2)
endif()

if(openPMD_HAVE_MPI)
Expand Down
15 changes: 6 additions & 9 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -261,15 +261,12 @@ CMake controls options with prefixed `-D`, e.g. `-DopenPMD_USE_MPI=OFF`:
<sup>1</sup> *e.g. changes C++ visibility keywords, breaks MSVC*
<sup>2</sup> *this includes most pre-/post-condition checks, disabling without specific cause is highly discouraged*

Additionally, the following libraries are shipped internally.
The following options allow to switch to external installs:

| CMake Option | Values | Library | Version |
|---------------------------------|------------|---------------|----------|
| `openPMD_USE_INTERNAL_CATCH` | **ON**/OFF | Catch2 | 2.13.10+ |
| `openPMD_USE_INTERNAL_PYBIND11` | **ON**/OFF | pybind11 | 2.11.1+ |
| `openPMD_USE_INTERNAL_JSON` | **ON**/OFF | NLohmann-JSON | 3.9.1+ |
| `openPMD_USE_INTERNAL_TOML11` | **ON**/OFF | toml11 | 3.7.1+ |
Additionally, the following libraries are shipped internally or, if the
DerNils-git marked this conversation as resolved.
Show resolved Hide resolved
corresponding `<PACKAGENAME>_ROOT` variable is provided, can be provided externally:
* [Catch2](https://github.com/catchorg/Catch2) (2.13.10+)
* [PyBind11](https://github.com/pybind/pybind11) (2.11.1+)
* [NLohmann-JSON](https://github.com/nlohmann/json) (3.9.1+)
* [toml11](https://github.com/ToruNiina/toml11) (3.7.1+)

By default, this will build as a shared library (`libopenPMD.[so|dylib|dll]`) and installs also its headers.
In order to build a static library, append `-DBUILD_SHARED_LIBS=OFF` to the `cmake` command.
Expand Down
Loading