[drake_cmake_external] Upgrade external dependencies

[tyler-yankee]

Upgrade package dependencies to match Drake's MODULE.bazel.

Towards #432 and RobotLocomotion/drake#23667.

---

This change is 

[tyler-yankee]

+(status: not ready to merge) until RobotLocomotion/drake#23694 lands

Reviewable status: all discussions resolved, platform LGTM missing

[tyler-yankee]

+a:@Aiden2244 feature per f2f, please

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

[Aiden2244]

drake_cmake_external/CMakeLists.txt line 45 at r1 (raw file):

    -DCMAKE_PREFIX_PATH:PATH=${CMAKE_PREFIX_PATH}
    -DCMAKE_VERBOSE_MAKEFILE:BOOL=${CMAKE_VERBOSE_MAKEFILE}
    # Suppress warnings from Eigen's deprecated upstream CMake.

Would you mind providing me with some more context as to why this was removed from the CMakeLists.txt file? Is this related to the Eigen version bump?

[tyler-yankee]

Reviewable status: 1 unresolved discussion, LGTM missing from assignee aiden2244, platform LGTM missing (waiting on @Aiden2244)

---

drake_cmake_external/CMakeLists.txt line 45 at r1 (raw file):

Previously, Aiden2244 (Aiden McCormack) wrote…

Would you mind providing me with some more context as to why this was removed from the CMakeLists.txt file? Is this related to the Eigen version bump?

Yes; in the however-many years since Eigen's released, they've at least somewhat modernized their CMake.

1. https://gitlab.com/libeigen/eigen/-/commit/98620b58c307cc60fc50ee08364e3fe0daa1e701 puts a band-aid on the first warning upstream, so we can remove our version. (Commit from 2024, so it wouldn't have been in the old Eigen.)
2. The second one is a bit harder to trace, but I think it's https://gitlab.com/libeigen/eigen/-/commit/1cdec386530c6b844389b96c199e723a1e4e71c7 for the upstream fix.

Note that CI (running under CMake 3.28, for Noble's apt) doesn't show the CMake policy warnings.

[Aiden2244]

drake_cmake_external/CMakeLists.txt line 45 at r1 (raw file):

Previously, tyler-yankee (Tyler Yankee) wrote…

Yes; in the however-many years since Eigen's released, they've at least somewhat modernized their CMake.

1. https://gitlab.com/libeigen/eigen/-/commit/98620b58c307cc60fc50ee08364e3fe0daa1e701 puts a band-aid on the first warning upstream, so we can remove our version. (Commit from 2024, so it wouldn't have been in the old Eigen.)
2. The second one is a bit harder to trace, but I think it's https://gitlab.com/libeigen/eigen/-/commit/1cdec386530c6b844389b96c199e723a1e4e71c7 for the upstream fix.

Note that CI (running under CMake 3.28, for Noble's apt) doesn't show the CMake policy warnings.

Great, thanks for explaining Tyler!

[Aiden2244]

@Aiden2244 reviewed 1 of 1 files at r1, all commit messages.
Reviewable status: all discussions resolved, platform LGTM missing (waiting on @tyler-yankee)

[tyler-yankee]

+a:@jwnimmer-tri for platform review (per schedule), please.

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

[jwnimmer-tri]

@jwnimmer-tri reviewed 1 of 1 files at r1, all commit messages.
Reviewable status: 1 unresolved discussion, LGTM missing from assignee jwnimmer-tri, platform LGTM missing (waiting on @tyler-yankee)

---

drake_cmake_external/CMakeLists.txt line 35 at r1 (raw file):

# be removed (as well as removing -DWITH_USER_EIGEN:BOOL=ON, below).
ExternalProject_Add(eigen
  URL https://gitlab.com/libeigen/eigen/-/archive/5.0.0/eigen-5.0.0.tar.gz

Working

Drake has a patch against 5.0.0:

https://github.com/RobotLocomotion/drake/blob/acf6950da41bb73f1b6c5293b9d852beee98aecc/MODULE.bazel#L57-L63

I thought Drake failed to even build without it. At the very least, possibly certain calling code not exercised by this example might fail to build without it.

I don't think we should upgrade until 5.0.1 (which is due out this week). Or else we need to dig in and figure out why CI is happy even without this patch missing.

[tyler-yankee]

Reviewable status: 1 unresolved discussion, LGTM missing from assignee jwnimmer-tri, platform LGTM missing (waiting on @jwnimmer-tri)

---

drake_cmake_external/CMakeLists.txt line 35 at r1 (raw file):

Previously, jwnimmer-tri (Jeremy Nimmer) wrote…

Working

Drake has a patch against 5.0.0:

https://github.com/RobotLocomotion/drake/blob/acf6950da41bb73f1b6c5293b9d852beee98aecc/MODULE.bazel#L57-L63

I thought Drake failed to even build without it. At the very least, possibly certain calling code not exercised by this example might fail to build without it.

I don't think we should upgrade until 5.0.1 (which is due out this week). Or else we need to dig in and figure out why CI is happy even without this patch missing.

I'm now also confused; it builds Drake just like a regular CMake build upstream would. In any case, if 5.0.1 is on the very-near horizon, I'm fine to just wait.

[jwnimmer-tri]

Reviewable status: 1 unresolved discussion, LGTM missing from assignee jwnimmer-tri, platform LGTM missing (waiting on @tyler-yankee)

---

drake_cmake_external/CMakeLists.txt line 35 at r1 (raw file):

Previously, tyler-yankee (Tyler Yankee) wrote…

I'm now also confused; it builds Drake just like a regular CMake build upstream would. In any case, if 5.0.1 is on the very-near horizon, I'm fine to just wait.

5.0.1 is "official" now. I would be OK having 5.0.1 here even though drake/MODULE.bazel is still 5.0.0 + patch. I'll be working to bump Drake to 5.0.1 soon enough.

[jwnimmer-tri]

Reviewable status: 2 unresolved discussions, LGTM missing from assignee jwnimmer-tri, platform LGTM missing (waiting on @tyler-yankee)

---

drake_cmake_external/CMakeLists.txt line 35 at r1 (raw file):

# be removed (as well as removing -DWITH_USER_EIGEN:BOOL=ON, below).
ExternalProject_Add(eigen
  URL https://gitlab.com/libeigen/eigen/-/archive/5.0.0/eigen-5.0.0.tar.gz

Working

Running this locally, Eigen 5.0.0 seems to compile static BLAS and LAPACK libraries by default? That doesn't seem like a good thing -- it should be using the same BLAS as the rest of the build. I suppose they've added some options to tweak that, which we'll need to set.

[jwnimmer-tri]

Reviewable status: 2 unresolved discussions, LGTM missing from assignee jwnimmer-tri, platform LGTM missing (waiting on @tyler-yankee)

---

drake_cmake_external/CMakeLists.txt line 35 at r1 (raw file):

Previously, jwnimmer-tri (Jeremy Nimmer) wrote…

Working

Running this locally, Eigen 5.0.0 seems to compile static BLAS and LAPACK libraries by default? That doesn't seem like a good thing -- it should be using the same BLAS as the rest of the build. I suppose they've added some options to tweak that, which we'll need to set.

option(EIGEN_BUILD_BLAS "Toggles the building of the Eigen Blas library" ${PROJECT_IS_TOP_LEVEL})
option(EIGEN_BUILD_LAPACK "Toggles the building of the included Eigen LAPACK library" ${PROJECT_IS_TOP_LEVEL})

I'm not sure why they are ON, given that Eigen shouldn't be "top level" in our superbuild? Over to you...

[jwnimmer-tri]

Reviewable status: 2 unresolved discussions, LGTM missing from assignee jwnimmer-tri, platform LGTM missing (waiting on @tyler-yankee)

---

drake_cmake_external/CMakeLists.txt line 35 at r1 (raw file):

I'm now also confused; it builds Drake just like a regular CMake build upstream would...

Right, it worried me too, so I did some digging.

I wanted to see if we were doing something wrong, so I checked and confirmed that the drake_cmake_external build of Drake was indeed using the downloaded Eigen 5.0.0 instead of accidentally the system Eigen.

So then I went back to find what part of Drake broke with 5.0.0 by reverting our local patch and running a full first-party Drake build and test.

The failure only happens with expressions Eigen::VectorX<symbolic::Monomial> op Eigen::VectorX<symbolic::Monomial> that need to promote to Eigen::VectorX<symbolic::Polynomial> but don't with Eigen 5.0.0. We don't have any of those calls in our library code, rather only in test code. So the Drake build won't fail, but if a user had such an operation in their own application code it would fail to build with Eigen 5.0.0.

So yes, we can't use 5.0.0 here and need to bump to 5.0.1, but nothing else is wrong with the infrastructure.

[jwnimmer-tri]

Reviewable status: 2 unresolved discussions, LGTM missing from assignee jwnimmer-tri, platform LGTM missing (waiting on @tyler-yankee)

---

drake_cmake_external/CMakeLists.txt line 35 at r1 (raw file):

Previously, jwnimmer-tri (Jeremy Nimmer) wrote…

I'm now also confused; it builds Drake just like a regular CMake build upstream would...

Right, it worried me too, so I did some digging.

I wanted to see if we were doing something wrong, so I checked and confirmed that the drake_cmake_external build of Drake was indeed using the downloaded Eigen 5.0.0 instead of accidentally the system Eigen.

So then I went back to find what part of Drake broke with 5.0.0 by reverting our local patch and running a full first-party Drake build and test.

The failure only happens with expressions Eigen::VectorX<symbolic::Monomial> op Eigen::VectorX<symbolic::Monomial> that need to promote to Eigen::VectorX<symbolic::Polynomial> but don't with Eigen 5.0.0. We don't have any of those calls in our library code, rather only in test code. So the Drake build won't fail, but if a user had such an operation in their own application code it would fail to build with Eigen 5.0.0.

So yes, we can't use 5.0.0 here and need to bump to 5.0.1, but nothing else is wrong with the infrastructure.

Eigen 5.0.1 is officially out now (and on BCR).

[jwnimmer-tri]

Reviewable status: 2 unresolved discussions, LGTM missing from assignee jwnimmer-tri, platform LGTM missing (waiting on @tyler-yankee)

---

drake_cmake_external/CMakeLists.txt line 35 at r1 (raw file):

Previously, jwnimmer-tri (Jeremy Nimmer) wrote…

option(EIGEN_BUILD_BLAS "Toggles the building of the Eigen Blas library" ${PROJECT_IS_TOP_LEVEL})
option(EIGEN_BUILD_LAPACK "Toggles the building of the included Eigen LAPACK library" ${PROJECT_IS_TOP_LEVEL})

I'm not sure why they are ON, given that Eigen shouldn't be "top level" in our superbuild? Over to you...

Oh, maybe it's not a BLAS used by Eigen, it's a BLAS implemented atop Eigen? That would be less of a problem, but in any case I don't think we want to DEE to build it, since it's not used.

[tyler-yankee]

Reviewable status: 2 unresolved discussions, LGTM missing from assignee jwnimmer-tri, platform LGTM missing (waiting on @Aiden2244 and @jwnimmer-tri)

---

drake_cmake_external/CMakeLists.txt line 35 at r1 (raw file):

Previously, jwnimmer-tri (Jeremy Nimmer) wrote…

Eigen 5.0.1 is officially out now (and on BCR).

Ah makes sense, thanks for digging on this! Given some other things going on this week anyways I wanted to wait for the dust to settle on Drake. I've upgraded here to 5.0.1 now.

---

drake_cmake_external/CMakeLists.txt line 35 at r1 (raw file):
I'll have to push another fix for this.

given that Eigen shouldn't be "top level" in our superbuild?

See https://cmake.org/cmake/help/latest/variable/PROJECT_IS_TOP_LEVEL.html, especially:

The value will be true in:

• the top-level directory of an external project added by ExternalProject

We build Eigen from source in this way, so it behaves as though it was first-party Eigen. We still want the options OFF (just to clarify)?

[tyler-yankee]

Reviewable status: 2 unresolved discussions, LGTM missing from assignee jwnimmer-tri, platform LGTM missing (waiting on @Aiden2244 and @jwnimmer-tri)

---

drake_cmake_external/CMakeLists.txt line 35 at r1 (raw file):

Previously, tyler-yankee (Tyler Yankee) wrote…

I'll have to push another fix for this.

given that Eigen shouldn't be "top level" in our superbuild?

See https://cmake.org/cmake/help/latest/variable/PROJECT_IS_TOP_LEVEL.html, especially:

The value will be true in:

• the top-level directory of an external project added by ExternalProject

We build Eigen from source in this way, so it behaves as though it was first-party Eigen. We still want the options OFF (just to clarify)?

(For context - I'll note that the point/effect of defaulting these options to PROJECT_IS_TOP_LEVEL is that if, for example, we vendored and built Eigen's sources via add_subdirectory instead of ExternalProject, those flags wouldn't fire.)

[tyler-yankee]

Reviewable status: 2 unresolved discussions, LGTM missing from assignee jwnimmer-tri, platform LGTM missing (waiting on @Aiden2244 and @jwnimmer-tri)

---

drake_cmake_external/CMakeLists.txt line 35 at r1 (raw file):

Previously, tyler-yankee (Tyler Yankee) wrote…

(For context - I'll note that the point/effect of defaulting these options to PROJECT_IS_TOP_LEVEL is that if, for example, we vendored and built Eigen's sources via add_subdirectory instead of ExternalProject, those flags wouldn't fire.)

This actually turns into a fun little rabbit hole. They also use this to control several other flags, including building tests, documentation, demos, pkgconfig...

[jwnimmer-tri]

@jwnimmer-tri reviewed 1 of 1 files at r2, all commit messages.
Reviewable status: 1 unresolved discussion, LGTM missing from assignee jwnimmer-tri, platform LGTM missing (waiting on @tyler-yankee)

---

drake_cmake_external/CMakeLists.txt line 35 at r1 (raw file):

We still want the options OFF (just to clarify)?

Yes. We don't use their BLAS library in this example, and it takes forever to build.

[tyler-yankee]

Reviewable status: 1 unresolved discussion, LGTM missing from assignee jwnimmer-tri, platform LGTM missing (waiting on @jwnimmer-tri)

---

drake_cmake_external/CMakeLists.txt line 35 at r1 (raw file):

Previously, jwnimmer-tri (Jeremy Nimmer) wrote…

We still want the options OFF (just to clarify)?

Yes. We don't use their BLAS library in this example, and it takes forever to build.

Fair; the rest of them too?

[jwnimmer-tri]

Reviewable status: 1 unresolved discussion, LGTM missing from assignee jwnimmer-tri, platform LGTM missing (waiting on @tyler-yankee)

---

drake_cmake_external/CMakeLists.txt line 35 at r1 (raw file):

Previously, tyler-yankee (Tyler Yankee) wrote…

Fair; the rest of them too?

The only thing we want it for it to install (copy) the <Eigen/...> headers. It's header-only library, so its "build and install" should be nearly instantaneous.

[tyler-yankee]

Reviewable status: 1 unresolved discussion, LGTM missing from assignee jwnimmer-tri, platform LGTM missing (waiting on @jwnimmer-tri)

---

drake_cmake_external/CMakeLists.txt line 35 at r1 (raw file):

Previously, jwnimmer-tri (Jeremy Nimmer) wrote…

The only thing we want it for it to install (copy) the <Eigen/...> headers. It's header-only library, so its "build and install" should be nearly instantaneous.

done (and verified locally that the Eigen build is super fast now)

[jwnimmer-tri]

@jwnimmer-tri reviewed 1 of 1 files at r3, all commit messages.
Reviewable status: all discussions resolved, LGTM missing from assignee jwnimmer-tri, platform LGTM missing (waiting on @tyler-yankee)

[jwnimmer-tri]

FYI also if ExternalProject_Add is the typical way that projects would consume Eigen from source, then we should probably tell upstream that their defaults are terrible.

[tyler-yankee]

Reviewable status: 1 unresolved discussion, LGTM missing from assignee jwnimmer-tri, platform LGTM missing (waiting on @jwnimmer-tri)

---

drake_cmake_external/CMakeLists.txt line 54 at r3 (raw file):

    -DEIGEN_BUILD_DOC:BOOL=OFF
    -DEIGEN_BUILD_DEMOS:BOOL=OFF
    -DEIGEN_BUILD_PKGCONFIG:BOOL=OFF

I was a little too trigger-happy. Drake's build needs the packaging bits in order to find the Eigen we're supplying (derp).

[jwnimmer-tri]

@jwnimmer-tri reviewed 1 of 1 files at r4, all commit messages.
Reviewable status: all discussions resolved, LGTM missing from assignee jwnimmer-tri, platform LGTM missing (waiting on @tyler-yankee)

[jwnimmer-tri]

Oops, forgot I hadn't .

Reviewable status: complete! all discussions resolved, platform LGTM from [jwnimmer-tri] (waiting on @tyler-yankee)

[tyler-yankee]

FYI also if ExternalProject_Add is the typical way that projects would consume Eigen from source, then we should probably tell upstream that their defaults are terrible.

I pinged some folks internally on this (since I'm not the most experienced with consuming CMake projects in this way). Eigen is following a very common pattern for packages which are commonly used as dependencies, but it's not a great one, either, because of precisely the situation like ours. If we used CMake's FetchContent (which is a complete structural difference, separate conversation...), or something like had Eigen as a git submodule and used add_subdirectory, then we wouldn't run into this. But ExternalProject_Add is a perfectly fine and standard way to consume dependencies from source.

Their defaults are getting into the territory of "X works when I build it this way, but when I build it this other way it doesn't work anymore," which I'm not a fan of, nor am I of having to spell out all their options and turn them off explicitly. But it is what it is. :|

Reviewable status: complete! all discussions resolved, platform LGTM from [jwnimmer-tri] (waiting on @tyler-yankee)

[tyler-yankee]

I had separated the commits on this branch intentionally. Does this repo not have an option to merge without squashing?

Reviewable status: complete! all discussions resolved, platform LGTM from [jwnimmer-tri] (waiting on @tyler-yankee)

[jwnimmer-tri]

Does this repo not have an option to merge without squashing?

At the moment, it only allows squashing.