-
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
Remove unused libyaml and libdl2 dependencies #18734
Remove unused libyaml and libdl2 dependencies #18734
Conversation
@drake-jenkins-bot linux-focal-unprovisioned-gcc-bazel-experimental-everything-release please +@svenevs for feature review, please. (Does this seem like sufficient coverage for pre-merge?) |
There is no direct CI coverage of the That's all a bit of a hassle. To me, testing locally manually starting from an empty Docker image would be easiest:
|
Done. No import errors. |
@drake-jenkins-bot linux-focal-unprovisioned-gcc-bazel-experimental-snopt-mosek-packaging please. These jobs are additionally needed to evaluate the changes here, the contents of |
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.
+@jwnimmer-tri for platform review, per schedule, please.
To me, testing locally manually starting from an empty Docker image would be easiest:
- Boot an empty Docker (once on Focal, and once on Jammy).
- In that container, run the version of binary_distribution/install_prereqs.sh from this PR. Notably the binary script, not the source one.
- Unpack last night's nightly tgz release (https://drake.mit.edu/from_binary.html) into the container.
- Try import 1pydrake.all`. If there are missing shared libraries, it will complain.
Feature review repeated these steps and they were successful. I also confirmed from a fresh macOS brew
state that libyaml
can be removed (this was never tracked anywhere, it is OK for this PR to remove it).
@mwoehlke-kitware I added more information on how to do this package analysis as you finish off the ticket in #17161 (comment)
@jwnimmer-tri there are some new potential issues from this analysis, but none relate to this PR. shlibs:Depends
vs our packages here, removing libdl2
and libyaml-cpp-dev
is A-OK.
Reviewed 3 of 3 files at r1, all commit messages.
Reviewable status: 1 unresolved discussion, LGTM missing from assignee jwnimmer-tri(platform), missing label for release notes (waiting on @jwnimmer-tri and @mwoehlke-kitware)
-- commits
line 2 at r1:
nit: on squash perhaps state them explicitly (libyaml
and libdl
)? Remove unused libyaml and libdl dependencies
?
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 3 of 3 files at r1, all commit messages.
Reviewable status: 1 unresolved discussion, missing label for release notes (waiting on @mwoehlke-kitware)
16db4e3
to
ea1d422
Compare
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 all commit messages.
Reviewable status: missing label for release notes (waiting on @mwoehlke-kitware)
Previously, svenevs (Stephen McDowell) wrote…
nit: on squash perhaps state them explicitly (
libyaml
andlibdl
)?Remove unused libyaml and libdl dependencies
?
Done.
Toward #17161.
This change is