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.

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

Fetch content #593

Merged
merged 5 commits into from
Mar 4, 2025
Merged

Fetch content #593

merged 5 commits into from
Mar 4, 2025

Conversation

flagarde
Copy link
Contributor

Hi,

This is a rework for #446. This PR is taking into account 3 scenari :

  1. User use the get_cpm.cmake (no change for them)

  2. Fetching with URL https://github.com/cpm-cmake/CPM.cmake/archive/refs/tags/v0.40.2.zip (use the .git_archival.txt)

  3. Fetching with GIT_REPOSITORY (use git program to parse version)

As this PR doesn't modify the standard way to obtain CPM it allows to test it before switching to this method as the prefered one

TheLartians
TheLartians previously approved these changes Mar 2, 2025
Copy link
Member

@TheLartians TheLartians left a comment

Choose a reason for hiding this comment

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

Thanks for creating this PR! I really like this approach for "trying out" the this new way of obtaining CPM. Once tested with a new release we can add this approach as an alternative to the readme. I think it still needs a run cmake-format run before merging though.

@flagarde
Copy link
Contributor Author

flagarde commented Mar 4, 2025

@TheLartians I run cmake-format. Seems it is passing now

Copy link
Member

@TheLartians TheLartians left a comment

Choose a reason for hiding this comment

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

Great, thanks!

@TheLartians TheLartians merged commit 0f23108 into cpm-cmake:master Mar 4, 2025
11 checks passed
@TheLartians
Copy link
Member

Made a release v0.40.7 for testing!

@flagarde
Copy link
Contributor Author

flagarde commented Mar 5, 2025

It seems to work but :

  • Sha5 or sha256 should appear in the release page to be able to use it for FetchContent

  • If the user use GIT_TAG even a release one v0.4.7 for example, it is still considered as development because .git present. One solution (if this is not the expected behaviour) is to parse the version extraced from git and if it corespond to a semver then not add the -development string

@TheLartians
Copy link
Member

Sha5 or sha256 should appear in the release page to be able to use it for FetchContent

I think we could update the publish script to also create a get_cpm_fetchcontent.cmake to help with that at least.

If the user use GIT_TAG even a release one v0.4.7 for example, it is still considered as development because .git present. One solution (if this is not the expected behaviour) is to parse the version extraced from git and if it corespond to a semver then not add the -development string

Yeah as the output of git describe --tags should also give the number of commits since the latest tag (if there are any), IMO it should be fine to omit the -development string as there should be no confusion with release versions.

@flagarde
Copy link
Contributor Author

flagarde commented Mar 5, 2025

For the sha256 I would do the contrary. Use some github action to write the SHA256 etc (all the one CMake can understand) inside the readme of the release. Modify the get_cpm to internally use this new method (it is more robust than file(download) cmake command).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants