Skip to content

Conversation

ax3l
Copy link
Member

@ax3l ax3l commented Sep 6, 2024

Cloning whole repos can be quite slow and CMake does not support shallow clones yet. We can use release tarballs instead.

Follow-up to #1583

Configure from scratch time:

  • Before: 50.2s
  • Now: 9.7s

Todo list:

@ax3l ax3l added the install label Sep 6, 2024
@ax3l ax3l added this to the 0.16.0 milestone Sep 6, 2024
@ax3l ax3l changed the title Cmake superbuild tar CMake Superbuild: Tarball Sep 6, 2024
@ax3l ax3l changed the title CMake Superbuild: Tarball [WIP] CMake Superbuild: Tarball Sep 6, 2024
@ax3l ax3l added third party third party libraries that are shipped and/or linked dependencies Pull requests that update a dependency file labels Sep 6, 2024
@ax3l ax3l force-pushed the cmake-superbuild-tar branch 2 times, most recently from 130e38a to 6e91a44 Compare September 6, 2024 03:53
Copy link
Contributor

@nischild nischild left a comment

Choose a reason for hiding this comment

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

Reducing the configure time by a factor of almost five is significant so an obvious improvement.
Just a small disadvantage. One has to remember to update two lines if the version of a dependency should be updated instead of one.

We are working on a wrapper for FetchContent_Declare to add a few features which should improve usability if the SOURCE_DIR directory is defined. We want to reduce the amount of downloads. If you are interested we could discuss this separately.

@ax3l
Copy link
Member Author

ax3l commented Sep 6, 2024

We are working on a wrapper for FetchContent_Declare to add a few features which should improve usability if the SOURCE_DIR directory is defined. We want to reduce the amount of downloads. If you are interested we could discuss this separately.

Thanks! That sounds very interesting as a follow-up, happy to take a look on what you are working on 👍

Just a small disadvantage. One has to remember to update two lines if the version of a dependency should be updated instead of one.

I would not wait for this extra update for the 0.16.0 release, since it is not critical and we do change the superbuild dependencies very, very seldomly. The superbuild is really just a developer convenience feature and should be deactivated for non-development deployments in favor of a proper external package/module.

"Local path to Catch2 source directory (preferred if set)")

# tarball fetcher
set(openPMD_catch_tar "https://github.com/catchorg/Catch2/archive/refs/tags/v2.13.10.tar.gz"
Copy link
Contributor

Choose a reason for hiding this comment

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

Will openPMD_catch_branch, openPMD_catch_repo and openPMD_catch_src be removed?

Copy link
Member Author

@ax3l ax3l Sep 17, 2024

Choose a reason for hiding this comment

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

We will support both. The order now of precedence is:

Copy link
Member Author

Choose a reason for hiding this comment

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

Docs added that explain the precedence.

@ax3l ax3l changed the title [WIP] CMake Superbuild: Tarball CMake Superbuild: Tarball Sep 17, 2024
Cloning whole repos can be quite slow and CMake does not support
shallow clones yet. We can use release tarballs instead.
@ax3l ax3l force-pushed the cmake-superbuild-tar branch from 6e91a44 to 761f136 Compare September 17, 2024 22:39
@ax3l ax3l merged commit 88089bc into openPMD:dev Sep 18, 2024
31 checks passed
@ax3l ax3l deleted the cmake-superbuild-tar branch September 18, 2024 16:57
This was referenced Oct 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dependencies Pull requests that update a dependency file install third party third party libraries that are shipped and/or linked

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants