Skip to content

[CMake Examples] Update required CMake versions #384

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

Merged
merged 3 commits into from
Apr 22, 2025

Conversation

tyler-yankee
Copy link
Collaborator

@tyler-yankee tyler-yankee commented Apr 14, 2025

Closes #22889.

  • Updates drake_cmake_external to use CMake 3.22...4.0 in accordance with https://drake.mit.edu/from_source.html (as of writing) while allowing for modern policies.
  • Updates drake_cmake_installed and drake_cmake_installed_apt to use CMake 3.9...4.0, as consuming binary Drake does not require modern CMake, but we'll allow modern policies (and get rid of deprecation warnings about older versions when using newer ones).

This change is Reviewable

@tyler-yankee tyler-yankee marked this pull request as ready for review April 14, 2025 20:09
Copy link
Collaborator Author

@tyler-yankee tyler-yankee left a comment

Choose a reason for hiding this comment

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

Successful CI corresponding with Drake #22898: https://drake-jenkins.csail.mit.edu/job/linux-jammy-unprovisioned-external-examples/3/.

+@BetsyMcPhail for feature review, please

Reviewable status: all discussions resolved, LGTM missing from assignee betsymcphail, platform LGTM missing (waiting on @BetsyMcPhail)

Copy link
Contributor

@BetsyMcPhail BetsyMcPhail left a comment

Choose a reason for hiding this comment

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

Reviewed 3 of 3 files at r1, all commit messages.
Reviewable status: all discussions resolved, LGTM missing from assignee betsymcphail, platform LGTM missing (waiting on @tyler-yankee)

Copy link
Contributor

@BetsyMcPhail BetsyMcPhail left a comment

Choose a reason for hiding this comment

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

Reviewable status: all discussions resolved, LGTM missing from assignee betsymcphail, platform LGTM missing (waiting on @tyler-yankee)

Copy link
Contributor

@BetsyMcPhail BetsyMcPhail left a comment

Choose a reason for hiding this comment

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

+@ggould-tri for platform review Tuesday, per schedule

Reviewable status: all discussions resolved, LGTM missing from assignee betsymcphail, LGTM missing from assignee ggould-tri, platform LGTM missing (waiting on @ggould-tri)

Copy link
Contributor

@ggould-tri ggould-tri left a comment

Choose a reason for hiding this comment

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

Reviewed 3 of 3 files at r1, all commit messages.
Reviewable status: all discussions resolved, LGTM missing from assignee ggould-tri, LGTM missing from assignee betsymcphail, platform LGTM missing (waiting on @tyler-yankee)

Copy link
Contributor

@ggould-tri ggould-tri left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewable status: all discussions resolved, LGTM missing from assignee betsymcphail, platform LGTM from [ggould-tri] (waiting on @tyler-yankee)

Copy link
Collaborator Author

@tyler-yankee tyler-yankee left a comment

Choose a reason for hiding this comment

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

Blocked from merging until we resolve some discussions on the Drake PR -- TL;DR building Drake and consuming Drake require different minimum versions of CMake, so drake_cmake_installed and drake_cmake_installed_apt may be subject to change.

Reviewable status: all discussions resolved, LGTM missing from assignee betsymcphail, platform LGTM from [ggould-tri] (waiting on @tyler-yankee)

Copy link
Contributor

@ggould-tri ggould-tri left a comment

Choose a reason for hiding this comment

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

Reviewable status: all discussions resolved, LGTM missing from assignee betsymcphail, platform LGTM from [ggould-tri] (waiting on @tyler-yankee)

@tyler-yankee tyler-yankee changed the title [CMake Examples] Update minimum required to 3.22 [CMake Examples] Update required CMake versions Apr 18, 2025
@tyler-yankee
Copy link
Collaborator Author

Okay, the examples are updated after discussion on #22898; essentially, we want to require 3.22 only for building Drake from source, while a lower version range is supported when consuming a binary Drake.

We can also get rid of the manual policy adjustment that I wrote before by giving a maximum version to cmake_minimum_required to allow for modern policies with modern CMake; if you're using CMake < 3.24, then you will get the warning about download extract timestamps, but that means you should update your CMake, not that DEE should update its policies. (And if you're using CMake >= 3.24, nothing to worry about.)

Copy link
Contributor

@ggould-tri ggould-tri left a comment

Choose a reason for hiding this comment

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

Reviewed 3 of 3 files at r2, all commit messages.
Reviewable status: all discussions resolved, LGTM missing from assignee betsymcphail, platform LGTM from [ggould-tri] (waiting on @tyler-yankee)

Copy link
Contributor

@BetsyMcPhail BetsyMcPhail left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewed 3 of 3 files at r2, all commit messages.
Reviewable status: :shipit: complete! all discussions resolved, platform LGTM from [ggould-tri, betsymcphail] (waiting on @tyler-yankee)

@BetsyMcPhail BetsyMcPhail merged commit 6be5668 into RobotLocomotion:main Apr 22, 2025
11 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.

Update Drake's CMake versions across the board
3 participants