rapids-cmake can generate pinned versions file #530
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
robertmaynard
changed the title
… they are testing
rapids-cmake can now generate a `versions.json` that contains the exact git SHA1 used by depdendencies which can be re-used to construct reproducible builds.
robertmaynard
force-pushed
the
fea/generate_pinned_manifest_file
branch
from
ec84328
to
cbba2a0
Compare
robertmaynard
added
feature request
robertmaynard
force-pushed
the
fea/generate_pinned_manifest_file
branch
from
bfd2ffc
to
0d0111e
Compare
bdice
reviewed
Feb 22, 2024
testing/cpm/cpm_generate_pins-format-patches/patches/hide_kernels.patch
Outdated
Show resolved
Hide resolved
testing/export/write_language-multiple-nested-enables/CMakeLists.txt
Outdated
Show resolved
Hide resolved
Co-authored-by: Bradley Dice <bdice@bradleydice.com>
…ts.txt Co-authored-by: Bradley Dice <bdice@bradleydice.com>
bdice
reviewed
Feb 23, 2024
2 tasks
bdice
reviewed
Feb 26, 2024
Co-authored-by: Bradley Dice <bdice@bradleydice.com>
bdice
approved these changes
Feb 27, 2024
vyasr
requested changes
Feb 28, 2024
There was a problem hiding this comment.
Do we want to make rapids-cpm-init respect an environment variable/CMake option so that users can do something like cmake ... -DRAPIDS_CMAKE_GENERATE_PINNED_VERSIONS (name TBD)? Or do we expect that consumers of this feature will modify their calls to rapids_cpm_init?
I haven't reviewed tests yet, will do that in a second pass after we get through this first round of questions.
vyasr
requested changes
Mar 1, 2024
There was a problem hiding this comment.
My question on the overall review (not tied to any line) from the last review remains outstanding, but I don't think it's super important. The other remaining items are pretty small and we should be good to finish this up in one more round.
Comment on lines
45
to
49
| # Everything should have shallow marked as false | ||
| # so that clones by SHA1 work | ||
| if(${proj}_shallow) | ||
| message(FATAL_ERROR "${proj}_shallow} is expected to be false, but got ${${proj}_shallow}") | ||
| endif() |
There was a problem hiding this comment.
Out of scope for this PR, but given the amount of duplication required to write CMake tests it might be good for us to centralize helpers that get used in multiple tests.
vyasr
approved these changes
Mar 4, 2024
|
/merge |
This was referenced Apr 25, 2024
rapids-bot bot
pushed a commit
that referenced
this pull request
May 1, 2024
Modifies pinning tests from #530: * to only test projects that were downloaded by CPM (e.g. ignoring the `fmt` that might already exist in the build environment) * to echo out `pinned_versions.json` and `versions.json` in logs from failed tests, to make debugging faster ## Notes for Reviewers #592 proposes some testing changes that aren't specific to the goals of that PR. Since that PR might be stuck for a bit (rapidsai/build-planning#56 (comment)), this proposes pulling those out into a separate PR: * so that other changes in this project benefit from them * to shrink the diff of #592 and therefore the risk of merge conflicts ### How I tested this Pushed a commit with the new test error message content changes but keeping `fmt` in the failing tests, to confirm that the expected tests failed. <details><summary>got the expected outputs (click me)</summary> ```text The following tests FAILED: 698 - cpm_generate_pins-nested-makefile (Failed) 700 - cpm_generate_pins-nested-ninja (Failed) 702 - cpm_generate_pins-nested-ninja_multi-config (Failed) 722 - cpm_generate_pins-simple-makefile (Failed) 724 - cpm_generate_pins-simple-ninja (Failed) 726 - cpm_generate_pins-simple-ninja_multi-config (Failed) ``` And they failed in the expected way more informative logs! ```text CMake Error at CMakeLists.txt:51 (message): pinned fmt tag (10.2.1) should differ compared to baseline 10.2.1 pinned_versions.json: { "always_download" : true, "git_shallow" : false, "git_tag" : "${version}", "git_url" : "https://github.com/fmtlib/fmt.git", "version" : "10.2.1" } versions.json: { "git_tag" : "${version}", "git_url" : "https://github.com/fmtlib/fmt.git", "version" : "10.2.1" } ``` ([build link](https://github.com/rapidsai/rapids-cmake/actions/runs/8837613213/job/24267079292?pr=592)) </details> # Authors: - James Lamb (https://github.com/jameslamb) Approvers: - Robert Maynard (https://github.com/robertmaynard) - Kyle Edwards (https://github.com/KyleFromNVIDIA) URL: #599
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Add rapids_cpm_generate_pinned_versions to support reproducible builds
rapids-cmake can now generate a
versions.jsonthat contains the exact git SHA1 used by depdendencies which can be re-usedto construct reproducible builds.
Checklist
cmake-format.jsonis up to date with these changes.include_guard(GLOBAL))