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

mkdoc: Work around a problematic deprecation_example header dep #15069

Merged
merged 1 commit into from
May 24, 2021

Conversation

jwnimmer-tri
Copy link
Collaborator

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

When collecting the list of package headers, in the case of the deprecation example (which has no package library) we were ending up with two different copies of the header being passed to mkdoc:

  bazel-out/k8-opt/bin/bindings/pydrake/common/_virtual_includes/deprecation_example_class/drake/bindings/pydrake/common/test/deprecation_example/example_class.h
  bindings/pydrake/common/test/deprecation_example/example_class.h

The first one is the path we want -- the one used while compiling, and thus the one with a matching -I flag. The second one is a correct dependency, but not used for compiling, so does not have any matching -I flag, so breaks mkdoc.

Here, we customize our skylark header collection to ignore the second file.

(Note that #15066 and #15068 are also required for Bazel 4.1 builds to succeed.)


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 jwnimmer-tri marked this pull request as ready for review May 21, 2021 17:05
@jwnimmer-tri
Copy link
Collaborator Author

+@EricCousineau-TRI for both reviews, please.

I've confirmed that with both Bazel 4.0.0 and Bazel 4.1.0, the contents of the generated documentation_pybind.h are identical between master vs this patch.

Copy link
Contributor

@EricCousineau-TRI EricCousineau-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_strong:

'tis quite a wiggy behavior!

Reviewed 1 of 1 files at r1.
Reviewable status: 1 unresolved discussion (waiting on @jwnimmer-tri)


tools/skylark/pybind.bzl, line 340 at r1 (raw file):

            # Remove headers that are duplicated as both a virtual include path
            # and a source path.  We'll to use the virtual include path, since

nit grammar missing verb btw "We'll ? to use"

Perhaps the to is redundant?

@EricCousineau-TRI
Copy link
Contributor


tools/skylark/pybind.bzl, line 340 at r1 (raw file):

Previously, EricCousineau-TRI (Eric Cousineau) wrote…

nit grammar missing verb btw "We'll ? to use"

Perhaps the to is redundant?

Demoting this to BTW.

When collecting the list of package headers, in the case of the deprecation
example (which has no package library) we were ending up with two different
copies of the header being passed to mkdoc:

  bazel-out/k8-opt/bin/bindings/pydrake/common/_virtual_includes/deprecation_example_class/drake/bindings/pydrake/common/test/deprecation_example/example_class.h
  bindings/pydrake/common/test/deprecation_example/example_class.h

The first one is the path we want -- the one used while compiling, and thus
the one with a matching -I flag.  The second one is a correct dependency,
but not used for compiling, so does not have any -I flag, so breaks mkdoc.

Here, we customize our skylark header collection to ignore the second file.
@jwnimmer-tri jwnimmer-tri merged commit 55c5d6d into RobotLocomotion:master May 24, 2021
@jwnimmer-tri jwnimmer-tri deleted the bazel-410-fixes branch May 24, 2021 17:23
@jwnimmer-tri
Copy link
Collaborator Author

Best guess, bazelbuild/bazel@e3b7e17 is what bit us.

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