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

Installed headers in geometry are broken #14792

Closed
SeanCurtis-TRI opened this issue Mar 17, 2021 · 0 comments · Fixed by #15070
Closed

Installed headers in geometry are broken #14792

SeanCurtis-TRI opened this issue Mar 17, 2021 · 0 comments · Fixed by #15070
Assignees
Labels
component: geometry general Geometry infrastructure or topics that defy categorization into other geometry components priority: low type: cleanup unused team: dynamics

Comments

@SeanCurtis-TRI
Copy link
Contributor

SeanCurtis-TRI commented Mar 17, 2021

Problem

By design, we want to exclude headers that include <fcl/fcl.h> from being installed. For example, the distance to point callback explicitly calls this out as well as the other callbacks:

However, some of these non-installed headers are included in installed headers:

That means we're installing headers that depend on uninstalled header. A bad thing.

Solution

For make_box_field.h and make_cylinder_field.h, it is sufficient to move the implementation to the .cc file and make sure the T = {double, AutoDiffXd} implementations get instantiated.

For collision_filter_legacy.h, its declared dependency on that header is an anachronism, it can simply be removed. However, there are downstream headers/tests/etc. that are benefiting from what the proximity_utilities.h pulls in for them. So, the down stream code will need to make up for the fact that they aren't including what they use.

@SeanCurtis-TRI SeanCurtis-TRI added type: cleanup priority: low unused team: dynamics component: geometry general Geometry infrastructure or topics that defy categorization into other geometry components labels Mar 17, 2021
@SeanCurtis-TRI SeanCurtis-TRI self-assigned this Mar 17, 2021
SeanCurtis-TRI added a commit to SeanCurtis-TRI/drake that referenced this issue May 21, 2021
There are a number of header files (that directly lead to including fcl)
that are marked to be installed. However, there were three header files
that *are* installed that included them directly.

This removes those transitive references and shores up downstream code
that lazily inherited headers via this path.

Resolves RobotLocomotion#14792
SeanCurtis-TRI added a commit to SeanCurtis-TRI/drake that referenced this issue May 21, 2021
There are a number of header files (that directly lead to including fcl)
that are marked to be installed. However, there were three header files
that *are* installed that included them directly.

This removes those transitive references and shores up downstream code
that lazily inherited headers via this path.

Resolves RobotLocomotion#14792
SeanCurtis-TRI added a commit that referenced this issue May 21, 2021
…es (#15070)

There are a number of header files (that directly lead to including fcl)
that are marked to be installed. However, there were three header files
that *are* installed that included them directly.

This removes those transitive references and shores up downstream code
that lazily inherited headers via this path.

Resolves #14792
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: geometry general Geometry infrastructure or topics that defy categorization into other geometry components priority: low type: cleanup unused team: dynamics
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant