CI: test minimal builds #20725
CI: test minimal builds #20725
Conversation
Test Results 22 files 22 suites 3d 22h 26m 41s ⏱️ Results for commit 06a9676. ♻️ This comment has been updated with latest results. |
|
Thank you for this! Looks like we need a check here, around the Python tests: if(BUILD_TESTING AND ROOT_pyroot_FOUND)Can you try to add that? |
| - name: Apply option minimal request from matrix for this job | ||
| if: ${{ matrix.minimal == true }} | ||
| env: | ||
| GLOBAL_CONFIG_DIR: '.github/workflows/root-ci-config/buildconfig' | ||
| CONFIGFILE_STEM: '.github/workflows/root-ci-config/buildconfig/${{ matrix.image }}' | ||
| run: | | ||
| echo "Applying minimal request options" | ||
| # Add commands to apply minimal request options here | ||
| set -x | ||
| cp "$GLOBAL_CONFIG_DIR/global-minimal.txt" "$GLOBAL_CONFIG_DIR/global.txt" | ||
| if [ -f "${CONFIGFILE_STEM}-minimal.txt" ]; then | ||
| cp "${CONFIGFILE_STEM}-minimal.txt" "${CONFIGFILE_STEM}.txt" | ||
| fi |
There was a problem hiding this comment.
IMHO this mechanism only makes sense if we plan to have minimal builds on multiple platforms (which I'd argue is not worth it). Otherwise we gain nothing by splitting global-minimal.txt and alma10-minimal.txt.
Why not just merge global-minimal.txt into alma10-minimal.txt, making sure that all flags set by global.txt are overridden if necessary?
There was a problem hiding this comment.
Thanks. Letting @pcanal comment on that (I just did a rebase of his work to see where we were standing.)
There was a problem hiding this comment.
making sure that all flags set by global.txt are overridden if necessary?
I think that would be defeating the purpose, see the new version of global.txt which actually match the semantic of minimal: nothing set except minimal and test ... with only 2 debatable addition: ccache to reduce turn around and fail-on-missing which may or may not be meaningful here.
The options set in alma10 are to only account for platform dependent behavior (and I guess vdt probably should not be there - we need to try to remove it before merging).
guitargeek
left a comment
guitargeek
left a comment
There was a problem hiding this comment.
We need to fixup the post-install tests, and I'd suggests to simplify this txt configuration scheme so we only need one alma10-minimal.txt file and no extra YAML configuration code.
as suggested by guitargeek
Co-authored-by: Jonas Rembser <jonas.rembser@cern.ch>
|
Thanks for fixing the post-install tests! Next, we need @pcanal to comment on #20725 (comment) to proceed. |
|
I pushed on this PR the missing commit (reducing the size of |
pcanal
left a comment
pcanal
left a comment
There was a problem hiding this comment.
This is in my opinion good to go.
3 participants
Just rebased version of pcanal's #20311