-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
[setup] Restructure Ubuntu dependencies on clang-9 #16622
[setup] Restructure Ubuntu dependencies on clang-9 #16622
Conversation
f76f2b5
to
56f3ae3
Compare
This comment was marked as outdated.
This comment was marked as outdated.
Interesting, our Unprovisioned Clang CMake builds pass Edit: I've now split this into |
On Ubuntu, move clang-9 into a separate "--with-clang" designation, which is enabled by default to maintain our current behavior. In the future, we should probably switch it off by default, to save download time and disk space during builds. Note that we still unconditionally require Clang's parsing library and clang-format binary to be installed during a source build, because we use those during the pydrake build. Add libomp-9-dev alongside clang-9. That allows us to use -fopenmp; otherwise, we would have linker errors. Remove vestigial and broken --without-doc-only option.
56f3ae3
to
11194b7
Compare
@drake-jenkins-bot linux-bionic-unprovisioned-clang-cmake-experimental-release please |
+@rpoyner-tri for feature review, please. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 7 of 7 files at r1, all commit messages.
Reviewable status: needs at least two assigned reviewers, labeled "do not merge" (waiting on @jwnimmer-tri)
+@EricCousineau-TRI for platform review per schedule, please. After reviews are complete, I'll help coordinate the CI updating dance for landing this. We might just be able to merge it straight-away. I'll figure that out later on. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 6 of 7 files at r1, all commit messages.
Reviewable status: labeled "do not merge" (waiting on @jwnimmer-tri)
Since this is a monotonic addition of packages, with no need (yet) for anything else that depends on them, we can safely merge this now without an extra CI interlock. I'll do that now and then open an issue for the CI image updates. => #16642 |
Towards #16606.
This change is