Skip to content
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

pydrake: Remove spurious documentation header dependencies #15068

Conversation

jwnimmer-tri
Copy link
Collaborator

@jwnimmer-tri jwnimmer-tri commented May 21, 2021

These were added in #9603 at a time when our pydrake linking was broken. We were linking the examples' libraries outside of libdrake.so, so their headers were not part of libdrake's headers, so for mkdoc to find them we needed to list them here individually.

Now that we link the examples correctly (i.e., directly into libdrake.so; see #9826), we no longer need to feed the redundant dependencies to mkdoc.

As of Bazel 4.1.0, the redundant headers actually break mkdoc, because we try to add these headers twice, with the second path spelling not matching any valid include path.


This change is Reviewable

@jwnimmer-tri jwnimmer-tri added priority: high status: single reviewer ok https://drake.mit.edu/reviewable.html labels May 21, 2021
@jwnimmer-tri
Copy link
Collaborator Author

jwnimmer-tri commented May 21, 2021

(CI will fail until #15066 or #15070 is merged first.)

These were added in 6f37fd6 at a time when our pydrake linking was broken.
We were linking the examples' libraries outside of libdrake.so, so their
headers were not part of libdrake's headers, so for mkdoc to find them we
needed to list them here individually.

Now that we link the examples correctly (i.e., directly into libdrake.so),
we no longer need to feed the redundant dependencies to mkdoc.

As of bazel 4.1, the redundant headers actually break mkdoc, because we try
to add these headers twice, with the second path spelling not matching any
valid include path.
@jwnimmer-tri jwnimmer-tri force-pushed the pydrake-examples-headers-docs branch from ad9292a to 957b39b Compare May 21, 2021 18:40
@jwnimmer-tri jwnimmer-tri marked this pull request as ready for review May 21, 2021 18:41
@SeanCurtis-TRI SeanCurtis-TRI self-assigned this May 21, 2021
Copy link
Contributor

@SeanCurtis-TRI SeanCurtis-TRI left a comment

Choose a reason for hiding this comment

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

+@SeanCurtis-TRI

Reviewable status: LGTM missing from assignee SeanCurtis-TRI(platform) (waiting on @SeanCurtis-TRI)

Copy link
Contributor

@SeanCurtis-TRI SeanCurtis-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:

Reviewed 1 of 1 files at r1.
Reviewable status: :shipit: complete! all discussions resolved, LGTM from assignee SeanCurtis-TRI(platform) (waiting on @jwnimmer-tri)

@SeanCurtis-TRI SeanCurtis-TRI merged commit 71cee83 into RobotLocomotion:master May 21, 2021
@jwnimmer-tri jwnimmer-tri deleted the pydrake-examples-headers-docs branch May 21, 2021 19:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: high status: single reviewer ok https://drake.mit.edu/reviewable.html
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants