Speed up the copy of artifacts #17692
Merged
Conversation
This file contains hidden or 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
Test Results 18 files 18 suites 4d 3h 19m 49s ⏱️ Results for commit ff7cc75. ♻️ This comment has been updated with latest results. |
pcanal
reviewed
Feb 11, 2025
pcanal
reviewed
Feb 11, 2025
pcanal
approved these changes
Feb 11, 2025
Member
pcanal
left a comment
pcanal
left a comment
There was a problem hiding this comment.
This looks good to me. Unlike headers, the consequence of over-updating is negligible for the artifacts.
hageboeck
force-pushed
the
artifact_copy
branch
from
3703874 to
4f21628
Compare
Use cmake -E copy_directory to execute the copy instead of doing it file by file. Fix root-project#14953.
When running ctest, all tutorials are invoked from the source directory, so here we do the same for hsimple.C.
hageboeck
force-pushed
the
artifact_copy
branch
from
4f21628 to
ff7cc75
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
This was factored out of #17607.
In contrast to the initial proposal in #17607, I managed to keep the artifact copy at build time. This should alleviate all problems where an out-of-date file might surprise developers.
I used
cmake -E copy_directoryinstead of going file-by-file, which again significantly reduces build targets and makes the initial copy much faster. We don't gain on the second invocation of make, and when tutorials are touched, one pays a 5s penalty, but I think the 15s saved on first invocation and the reduced number of targets make this worthwhile.make -j6 move_artifactsFor the people who wonder why we do this at all
gROOT->GetTutorialDir(), which points to the path where theroot.exeis.root.exe, and we didn't yet have a discussion on whether or not and how this should be changed.Fix #14953.