Feature: Reintroduce FetchContent by sandro-elsweijer · Pull Request #2303 · DLR-AMR/t8code

sandro-elsweijer · 2026-04-30T10:49:14Z

Describe your changes here:
Reintroduces FetchContent after its removal in #2289

Improvements in this PR:

MakeAvailable was called multiple times for each target
Renamed dependencies to thirdparty, because not all libs in fetchcontent are mandatory
Combined fixes for SC and GTest
Fixed Gtest fix for Windows systems
Fixed CI
Marked some variables as internal instead of advanced
removed git_progress, because the output was too long and cmake truncated it

All these boxes must be checked by the AUTHOR before requesting review:

The PR is small enough to be reviewed easily. If not, consider splitting up the changes in multiple PRs.
The title starts with one of the following prefixes: Documentation:, Bugfix:, Feature:, Improvement: or Other:.
If the PR is related to an issue, make sure to link it.
The author made sure that, as a reviewer, he/she would check all boxes below.

All these boxes must be checked by the REVIEWERS before merging the pull request:

As a reviewer please read through all the code lines and make sure that the code is fully understood, bug free, well-documented and well-structured.

General

The reviewer executed the new code features at least once and checked the results manually.
The code follows the t8code coding guidelines.
New source/header files are properly added to the CMake files.
The code is well documented. In particular, all function declarations, structs/classes and their members have a proper doxygen documentation. Make sure to add a file documentation for each file!
All new algorithms and data structures are sufficiently optimal in terms of memory and runtime (If this should be merged, but there is still potential for optimization, create a new issue).

Tests

The code is covered in an existing or new test case using Google Test.
The code coverage of the project (reported in the CI) should not decrease. If coverage is decreased, make sure that this is reasonable and acceptable.
Valgrind doesn't find any bugs in the new code. This script can be used to check for errors; see also this wiki article.

If the Pull request introduces code that is not covered by the github action (for example coupling with a new library):

Should this use case be added to the github action?
If not, does the specific use case compile and all tests pass (check manually).

Scripts and Wiki

If a new directory with source files is added, it must be covered by the scripts/internal/find_all_source_files.sh to check the indentation of these files.
If this PR introduces a new feature, it must be covered in an example or tutorial and a Wiki article.

License

The author added a BSD statement to doc/ (or already has one).

This reverts commit 9c31d89.

codecov · 2026-04-30T13:42:49Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 79.42%. Comparing base (59f17ae) to head (9961d39).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #2303   +/-   ##
=======================================
  Coverage   79.42%   79.42%           
=======================================
  Files         115      115           
  Lines       19285    19285           
=======================================
  Hits        15318    15318           
  Misses       3967     3967

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

benegee

Awesome!

From what I can tell, this looks good so far!

benegee · 2026-05-06T07:13:47Z

@@ -1,3 +1,12 @@
+# Updated dependencies
+
+We have switched from a submodules approach to a FetchContent approach for our dependencies towards p4est and sc. If you don't have already installed p4est and sc, calling cmake will install the right branch from their github. If you have p4est and sc installed already and want to use that version no further code will be downloaded. If your current configuration is to the internal p4est and sc installation you don't need to update your workflow. The submodules of p4est and sc should be removed with this update. Instead they will be downloaded and installed in the _deps folder in your build folder.


[...] dependencies on p4est [...]?

If you have already installed p4est [...]?

If you current configuration is to use the internal [...] ?

benegee · 2026-05-06T07:18:15Z

+    - name: Get sc commit hash  and url from thirdparty.json
+      run: |
+        hash=$(jq -r '.thirdparty[] | select(.name=="SC") .source.ref' cmake/thirdparty.json)
+        echo sc_commit=$hash >> $GITHUB_ENV


lower case GITHUB_ENV vars vs. upper case above.
Is this ok?

benegee · 2026-05-06T07:19:50Z

      MAKEFLAGS: ${{ matrix.MAKEFLAGS }}
      IGNORE_CACHE: false # Use this to force a new installation of sc and p4est for this specific workflow run
-      CACHE_COUNTER: 0 # Increase this number to force a new installation of sc and p4est and to update the cache once
+      CACHE_COUNTER: 8 # Increase this number to force a new installation of sc and p4est and to update the cache once


benegee · 2026-05-06T07:24:16Z

+        # If the named option variable does not exist in the CMake cache, warn and proceed.
+        if(NOT DEFINED ${DEP_CMAKE_OPTION})
+            message(FATAL_ERROR "Loading thirdparty library ${DEP_NAME} at index ${INDEX} references unknown CMake option '${DEP_CMAKE_OPTION}'. Aborting.")


FATAL_ERROR will abort, I think.

benegee · 2026-05-06T07:24:39Z

+
+    message(STATUS "Configuring thirdparty library: ${DEP_NAME} (Type: ${DEP_TYPE}, SHALLOW: ${DEP_SHALLOW})")
+
+    #If Dep_SHALLOW is not a valid boolean, set it to FALSE and print a warning.


Suggested change

#If Dep_SHALLOW is not a valid boolean, set it to FALSE and print a warning.

# If Dep_SHALLOW is not a valid boolean, set it to FALSE and print a warning.

benegee · 2026-05-06T07:28:35Z

+
+    #If Dep_SHALLOW is not a valid boolean, set it to FALSE and print a warning.
+    if(NOT DEP_SHALLOW STREQUAL "TRUE" AND NOT DEP_SHALLOW STREQUAL "FALSE")
+        message(STATUS "Invalid value for 'shallow' in thirdparty library ${DEP_NAME}: '${DEP_SHALLOW}'. Expected 'true' or 'false'. Defaulting to 'false'.")


Suggested change

message(STATUS "Invalid value for 'shallow' in thirdparty library ${DEP_NAME}: '${DEP_SHALLOW}'. Expected 'true' or 'false'. Defaulting to 'false'.")

message(STATUS "Invalid value for 'shallow' in thirdparty library ${DEP_NAME}: '${DEP_SHALLOW}'. Expected 'TRUE' or 'FALSE'. Defaulting to 'FALSE'.")

Or do you want to allow lower case values as well? (In this case TOLOWER has be used before the STREQUAL, I think)

benegee · 2026-05-06T07:30:32Z

+############################################ ThirdParty ############################################
+
+# Set some variables for the thirdparty libraries
+set(gtest_force_shared_crt ON CACHE INTERNAL "" FORCE)


Suggested change

set(gtest_force_shared_crt ON CACHE INTERNAL "" FORCE)

set( gtest_force_shared_crt ON CACHE INTERNAL "" FORCE )

benegee · 2026-05-06T07:33:32Z

 endif()

-mark_as_advanced( FORCE gtest_disable_mpi)
+# Capture the list of variables before adding the thirdparty libraries to mark the added ones as advanced.


Will this include the FETCHCONTENT variables, which are already marked in thirdparty.cmake?

sandro-elsweijer added 15 commits April 30, 2026 10:41

fix ci

0097a30

add newline at eof

f3cce7a

simplify depends on cmake option by making it mandatory

da7660f

move dependencies into separate cmake script

eb061dd

rename dependencies to thirdparty

000aa1a

remove MakeAvailable() duplication and align fixes

43d8e04

added jq to docker image instead of installing it in the ci

84b85ee

Reapply "Improvement: Implement FetchContent"

4118df9

This reverts commit 9c31d89.

Merge branch 'submodule_adjustment' into reintroduce_fetchcontent

f60b819

remove comment

48e5ac5

increase counter [run ci]

449bf65

rename dependency file in ci [run ci]

9250bb9

reintroduce deleted comment

4c8a84b

remove reintroduced netcdf linkage

d750d68

adapt path of thirdparty file in ci [run ci]

762093d

sandro-elsweijer force-pushed the reintroduce_fetchcontent branch from 9b6430f to 762093d Compare April 30, 2026 11:06

sandro-elsweijer added 7 commits April 30, 2026 13:20

fix ci runner bug [run ci]

ad4c88e

use checkout action instead of manual checkout [run ci]

0b08e59

fix checkout [run ci]

50a1de9

move folder creation [run ci]

37f54fa

remove redundant include

da96b61

remove wrong comment

7a7843b

deactivate ci caching to debug the ci

254c5d5

sandro-elsweijer force-pushed the reintroduce_fetchcontent branch from 970b2cf to 254c5d5 Compare April 30, 2026 13:00

[run ci]

9961d39

sandro-elsweijer added 4 commits May 4, 2026 09:58

fix advanced options from fetchcontent

01bbd66

make options internal instead of advanced

9e1db72

remove git_progress due to buffer overflow

05860a7

enable cache and increase counter

3b5e8d1

sandro-elsweijer marked this pull request as ready for review May 4, 2026 13:16

benegee reviewed May 6, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature: Reintroduce FetchContent#2303

Feature: Reintroduce FetchContent#2303
sandro-elsweijer wants to merge 27 commits intomainfrom
reintroduce_fetchcontent

sandro-elsweijer commented Apr 30, 2026 •

edited

Loading

Uh oh!

codecov Bot commented Apr 30, 2026

Uh oh!

benegee left a comment

Uh oh!

benegee May 6, 2026

Uh oh!

benegee May 6, 2026

Uh oh!

benegee May 6, 2026

Uh oh!

benegee May 6, 2026

Uh oh!

benegee May 6, 2026

Uh oh!

benegee May 6, 2026

Uh oh!

benegee May 6, 2026

Uh oh!

benegee May 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		@@ -1,3 +1,12 @@
		# Updated dependencies

		We have switched from a submodules approach to a FetchContent approach for our dependencies towards p4est and sc. If you don't have already installed p4est and sc, calling cmake will install the right branch from their github. If you have p4est and sc installed already and want to use that version no further code will be downloaded. If your current configuration is to the internal p4est and sc installation you don't need to update your workflow. The submodules of p4est and sc should be removed with this update. Instead they will be downloaded and installed in the _deps folder in your build folder.


		message(STATUS "Configuring thirdparty library: ${DEP_NAME} (Type: ${DEP_TYPE}, SHALLOW: ${DEP_SHALLOW})")

		#If Dep_SHALLOW is not a valid boolean, set it to FALSE and print a warning.

	#If Dep_SHALLOW is not a valid boolean, set it to FALSE and print a warning.
	# If Dep_SHALLOW is not a valid boolean, set it to FALSE and print a warning.

	message(STATUS "Invalid value for 'shallow' in thirdparty library ${DEP_NAME}: '${DEP_SHALLOW}'. Expected 'true' or 'false'. Defaulting to 'false'.")
	message(STATUS "Invalid value for 'shallow' in thirdparty library ${DEP_NAME}: '${DEP_SHALLOW}'. Expected 'TRUE' or 'FALSE'. Defaulting to 'FALSE'.")

	set(gtest_force_shared_crt ON CACHE INTERNAL "" FORCE)
	set( gtest_force_shared_crt ON CACHE INTERNAL "" FORCE )

Conversation

sandro-elsweijer commented Apr 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

General

Tests

Scripts and Wiki

License

Uh oh!

codecov Bot commented Apr 30, 2026

Codecov Report

Uh oh!

benegee left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

sandro-elsweijer commented Apr 30, 2026 •

edited

Loading