Skip to content

[cmake] fixes for cmake 4.0.0#18326

Merged
couet merged 2 commits intoroot-project:masterfrom
couet:cmake-fix
Apr 10, 2025
Merged

[cmake] fixes for cmake 4.0.0#18326
couet merged 2 commits intoroot-project:masterfrom
couet:cmake-fix

Conversation

@couet
Copy link
Copy Markdown
Member

@couet couet commented Apr 9, 2025

cmake 4.0.0 (the official current cmake version) needs some fixes for DAVIX VDT etc ..(see #18312)

@couet couet requested a review from bellenot as a code owner April 9, 2025 07:48
@couet couet mentioned this pull request Apr 9, 2025
1 task
@ferdymercury ferdymercury linked an issue Apr 9, 2025 that may be closed by this pull request
1 task
@guitargeek
Copy link
Copy Markdown
Contributor

The Fedora 42 build config has disable some builtins and features because of CMake 4:
https://github.com/root-project/root/blob/master/.github/workflows/root-ci-config/buildconfig/fedora42.txt#L9
Namely:

builtin_cppzmq=OFF
builtin_zeromq=OFF
roofit_multiprocess=OFF

Can we remove these lines in the same PR maybe? That would also ensure your changes get more test coverage. But can also be done later

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Apr 9, 2025

Test Results

    18 files      18 suites   4d 6h 19m 30s ⏱️
 2 722 tests  2 721 ✅ 0 💤 1 ❌
47 315 runs  47 314 ✅ 0 💤 1 ❌

For more details on these failures, see this check.

Results for commit 36f1fdc.

♻️ This comment has been updated with latest results.

@couet couet requested a review from dpiparo as a code owner April 9, 2025 11:07
@couet couet requested review from guitargeek and removed request for dpiparo April 9, 2025 11:07
@dpiparo
Copy link
Copy Markdown
Member

dpiparo commented Apr 9, 2025

@couet when it's time to merge can you please reformulate the commit messages?

@dpiparo dpiparo self-requested a review April 9, 2025 19:29
Copy link
Copy Markdown
Member

@dpiparo dpiparo left a comment

Choose a reason for hiding this comment

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

The commit messages need a bit of attention: see https://github.com/root-project/root/blob/master/CONTRIBUTING.md .
For example, the commit messages start with a tag. For example, the commit in the fedora42 configuration should start with "[ci]" while the one for the CMakeLists with "[cmake]". Moreover, the commit message should describe the why of the changes, rather than the what and how - we can gather this from the commit summary and the change diff, respectively. The commit message should be wrapped at 72 characters.

@couet couet changed the title fixes for cmake 4.0.0 [cmake] fixes for cmake 4.0.0 Apr 10, 2025
@couet couet requested a review from dpiparo April 10, 2025 07:27
@couet
Copy link
Copy Markdown
Member Author

couet commented Apr 10, 2025

I do not see how to modify the message for fedora42 I guess at merging time it will be possible ?

@ferdymercury
Copy link
Copy Markdown
Collaborator

ferdymercury commented Apr 10, 2025

I do not see how to modify the message for fedora42 I guess at merging time it will be possible ?

Locally it's possible:

git checkout cmake-fix
git rebase --interactive master
# replace by hand "pick" with "reword". Save and Exit.
# edit and save message commit by commit.
git push -f origin cmake-fix

(Sometimes, it takes less time to just open a new PR from scratch with completely new commits.)

@couet couet merged commit f574737 into root-project:master Apr 10, 2025
19 of 23 checks passed
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.

Problem with cmake 4.0.0

5 participants