[INFRA] Use CPM #3288

eseiler · 2024-10-09T11:14:12Z

This should make things easier eventually. See https://github.com/cpm-cmake/CPM.cmake

Ideally, we would use CMake to create seqan3-config.cmake and seqan3-config-version.cmake, i.e. use proper CMake packaging, which got a lot better since SeqAn3 was created.

For now, I'm keeping the old way of packaging.

Overview

Dependency	Clone	Source Package	Installed Package
CPM.cmake	CPM	Vendored	-
sdsl	CPM	Vendored	Vendored
cereal	CPM	Vendored	Vendored
google benchmark	CPM	CPM	-
google test	CPM	CPM	-
doxygen-awesome	CPM	CPM	-
SeqAn2	CPM	CPM	-
use_ccache	CPM	Vendored	-

CPM.cmake

When running from a git clone, build_system/CPM.cmake downloads the actual CPM.cmake and includes it.
For source packages, the downloaded CPM.cmake replaces the build_system/CPM.cmake file in the project.

CPM

Dependencies are gathered by CPM.

Local packages (i.e., such found via find_package) can be used by setting the environment variable CPM_USE_LOCAL_PACKAGES to ON, or passing -DCPM_USE_LOCAL_PACKAGES=ON to CMake.

Packages can also be overridden by passing -D<DEPENDENCY>_SOURCE_DIR=<path> to CMake.
For example, with -Ddoxygen_awesome_SOURCE_DIR=/path/to/doxygen-awesome, the doxygen-awesome package will be used from /path/to/doxygen-awesome.
<DEPENDENCY> refers to the value of NAME in build_system/package-lock.cmake.

Vendored

Vendored dependencies are included in the project and have a fixed path.
For example, sdsl is vendored in include/seqan3/vendor/sdsl-lite.

SeqAn2

SeqAn2 will be included when building tests if

SEQAN3_WITH_SEQAN2 is set to ON, or
the environment variable CI is set

SEQAN3_WITH_SEQAN2 is OFF by default.
If CI is set, you can pass -DSEQAN3_WITH_SEQAN2_CI=OFF to CMake to disable SeqAn2.

Werror

Warnings are treated as errors if

SEQAN3_WITH_WERROR is set to ON (Default)

SEQAN3_WITH_WERROR is ON by default.
You can pass -DSEQAN3_WITH_WERROR=OFF to CMake to disable treating warnings as errors.

To do

seqan-actions · 2024-10-09T11:19:41Z

Documentation preview available at https://docs.seqan.de/preview/seqan/seqan3/3288

codecov · 2024-10-09T11:32:33Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.13%. Comparing base (30bdf8d) to head (d4a9857).
Report is 14 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #3288   +/-   ##
=======================================
  Coverage   98.13%   98.13%           
=======================================
  Files         271      271           
  Lines       11955    11955           
  Branches      104      104           
=======================================
  Hits        11732    11732           
  Misses        223      223

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

SGSSGene

Currently I am a big fan of CPM and this PR is a big step in the right direction.

Where do the changes of gtest_build-> gtest_main and benchmark_build -> benchmark_main come from? that is from having newer dependencies?
The workflows/CI has many -j2 removed, was that on purpose?

I put some random annotation with my thoughts to some lines. I think that this PR already tackles so much, that it is good to go.

SGSSGene · 2024-10-21T20:10:09Z

.github/workflows/cron_latest_libraries.yml

+          sed -i -E 's@(set \(SEQAN3_\S+_VERSION )[^\)]+\)@\1main)@g' $FILE
+          sed -i -E 's@VERSION( \$\{SEQAN3_\S+_VERSION\})@GIT_TAG\1@g' $FILE
+          sed -i -E 's@SEQAN3_(SDSL|CEREAL)_VERSION main@SEQAN3_\1_VERSION master@g' $FILE
+          cat $FILE


What do these lines do?

Basically, making every package in build_system/package-lock.cmake use GIT_TAG {main,master}.
Not sure if we really need this cron, but I adapted it for now

SGSSGene · 2024-10-21T20:12:29Z

.github/workflows/cron_latest_libraries.yml


      - name: Configure tests
        run: |
-          mkdir seqan3-build
-          cd seqan3-build
+          mkdir build && cd build


Random thoughts (no action required):
"pro-tip": mkdir build && cd $_

(I can't decide what I like more....)

SGSSGene · 2024-10-21T20:14:14Z

build_system/cpack_install.cmake.in

+        configure_file ("@use_ccache_SOURCE_DIR@/ccache/CMakeLists.txt"
+                        "${CMAKE_CURRENT_BINARY_DIR}/test/cmake/seqan3_require_ccache.cmake" COPYONLY)
+    endif ()
+endif ()


Random thoughts (no action required):
I wonder if sometime in the future, something like this could be replaced by some CPM package.

SGSSGene · 2024-10-21T20:20:11Z

build_system/seqan3-config.cmake

+        seqan3_config_error ("The SDSL library is required, but wasn't found.")
+    endif ()
+else ()
+    seqan3_config_error ("The SDSL library is required, but wasn't found.")


This whole block seems twisted.

Yes, because it needs to work for clones, source packages, and installed packages.
Ideally, there would be different scripts for those three, but it would require some rewriting of all the CMake stuff.

SGSSGene · 2024-10-21T20:27:31Z

test/seqan3-test.cmake

+# Cannot be toggled on if initially set to off.
+cmake_dependent_option (SEQAN3_WITH_SEQAN2_CI "Build tests with SeqAn2." ON "DEFINED ENV{CI}" OFF)
+
+if (SEQAN3_WITH_SEQAN2 OR SEQAN3_WITH_SEQAN2_CI)


This seems a little over complicated. would vote for just the SEQAN3_WITH_SEQAN2 flag and the .github/workflow/xyz.yml file just have to set the flag on the command line when cmake is being called.

But then you would have to make sure that all ~10 workflows do set the flag.
It would also be possible to change the default for the option depending on whether CI is set or not; but it wouldn't be updated if the env changes.

eseiler · 2024-10-22T12:32:17Z

Currently I am a big fan of CPM and this PR is a big step in the right direction.

Where do the changes of gtest_build-> gtest_main and benchmark_build -> benchmark_main come from? that is from having newer dependencies?

{gtest,benchmark}_build where target defined by SeqAn3. {gtest,benchmark}_main are targets defined by gtest and benchmark.

The workflows/CI has many -j2 removed, was that on purpose?

It's set in the environment, such that we don't need to update each workflow if the runners get more cores.

https://github.com/seqan/actions/blob/305a1ba7fe9e52ab47371785c3387d0dbcd23a02/docker/gcc/Dockerfile#L35-L36

https://github.com/seqan/actions/blob/305a1ba7fe9e52ab47371785c3387d0dbcd23a02/setup-compiler/action.yml#L106-L107

I guess there is also a way to make it dynamic in the docker image, but it isn't for now.

I put some random annotation with my thoughts to some lines. I think that this PR already tackles so much, that it is good to go.

SGSSGene

CPM here we come!!!!

seqan-actions added lint [INTERNAL] signal for linting and removed lint [INTERNAL] signal for linting labels Oct 9, 2024

eseiler force-pushed the infra/cpm branch from bae58bf to 46ff9f5 Compare October 14, 2024 12:54

seqan-actions added lint [INTERNAL] signal for linting and removed lint [INTERNAL] signal for linting labels Oct 14, 2024

eseiler force-pushed the infra/cpm branch from 46ff9f5 to ed2459b Compare October 14, 2024 13:52

seqan-actions added lint [INTERNAL] signal for linting and removed lint [INTERNAL] signal for linting labels Oct 14, 2024

eseiler added 2 commits October 15, 2024 14:32

[INFRA] Add CPM commands to .cmake-format

4d1855f

[INFRA] Add CPM

55ea61d

eseiler force-pushed the infra/cpm branch from ed2459b to 287b37c Compare October 15, 2024 12:52

seqan-actions added the lint [INTERNAL] signal for linting label Oct 15, 2024

seqan-actions added lint [INTERNAL] signal for linting and removed lint [INTERNAL] signal for linting labels Oct 15, 2024

eseiler force-pushed the infra/cpm branch from c9a7103 to f962298 Compare October 15, 2024 13:51

seqan-actions added lint [INTERNAL] signal for linting and removed lint [INTERNAL] signal for linting labels Oct 15, 2024

[INFRA] Use CPM for cereal and sdsl

e7c77ba

eseiler force-pushed the infra/cpm branch from b18c622 to 31e4f03 Compare October 15, 2024 14:51

seqan-actions added lint [INTERNAL] signal for linting and removed lint [INTERNAL] signal for linting labels Oct 15, 2024

eseiler added 10 commits October 16, 2024 17:34

[INFRA] Use CPM for doxygen-awesome

b65f7b2

[INFRA] Use CPM for tests

b901768

[INFRA] Use CPM for seqan2

c488856

[INFRA] Add option for building with Werror

ad6ca74

[INFRA] Remove unused cmake scripts

7852e86

[INFRA] Use CPM packages in tests

3f36874

[INFRA] Update install target

d787120

[INFRA] Update cpack target

f079f75

[INFRA] Update CI

ddd89a7

[INFRA] Remove submodules

d4a9857

eseiler force-pushed the infra/cpm branch from 31e4f03 to d4a9857 Compare October 16, 2024 15:35

seqan-actions added lint [INTERNAL] signal for linting and removed lint [INTERNAL] signal for linting labels Oct 16, 2024

eseiler marked this pull request as ready for review October 21, 2024 11:20

SGSSGene requested changes Oct 21, 2024

View reviewed changes

eseiler requested a review from SGSSGene October 22, 2024 12:32

eseiler changed the title ~~[WIP] Use CPM~~ [INFRA] Use CPM Oct 22, 2024

SGSSGene approved these changes Oct 22, 2024

View reviewed changes

eseiler merged commit 31a3b14 into seqan:main Oct 23, 2024
40 checks passed

eseiler deleted the infra/cpm branch October 23, 2024 08:53

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[INFRA] Use CPM #3288

[INFRA] Use CPM #3288

eseiler commented Oct 9, 2024 •

edited

Loading

seqan-actions commented Oct 9, 2024

codecov bot commented Oct 9, 2024 •

edited

Loading

SGSSGene left a comment

SGSSGene Oct 21, 2024

eseiler Oct 22, 2024

SGSSGene Oct 21, 2024

SGSSGene Oct 21, 2024

SGSSGene Oct 21, 2024

eseiler Oct 22, 2024

SGSSGene Oct 21, 2024

eseiler Oct 22, 2024

eseiler commented Oct 22, 2024

SGSSGene left a comment

[INFRA] Use CPM #3288

[INFRA] Use CPM #3288

Conversation

eseiler commented Oct 9, 2024 • edited Loading

Overview

CPM.cmake

CPM

Vendored

SeqAn2

Werror

To do

seqan-actions commented Oct 9, 2024

codecov bot commented Oct 9, 2024 • edited Loading

Codecov Report

SGSSGene left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eseiler commented Oct 22, 2024

SGSSGene left a comment

Choose a reason for hiding this comment

eseiler commented Oct 9, 2024 •

edited

Loading

codecov bot commented Oct 9, 2024 •

edited

Loading