[workspace] Better hide ccd and fcl externals #18678
Conversation
jwnimmer-tri
added
priority: low
status: do not merge
status: do not review
release notes: none
jwnimmer-tri
force-pushed
the
workspace-finish-fcl-hidden
branch
from
2e8cd56 to
b19295d
Compare
jwnimmer-tri
force-pushed
the
workspace-finish-fcl-hidden
branch
from
b19295d to
9ecde88
Compare
Fix ccd BUILD file to disallow things like `#include "polytope.h"` of private headers, which previously resolved to its `src` directory. Remove unnecessary maintenance of fcl version numbers; zeros are fine. Opt-in fcl to use use strict genrule checking per our current habits, so it will fail-fast in case anything is surprising when upgrading. Switch fcl to use vendor_cxx infrastructure to wrap a hidden namespace around all of its C++ code.
jwnimmer-tri
force-pushed
the
workspace-finish-fcl-hidden
branch
from
9ecde88 to
8c0b031
Compare
|
+@SeanCurtis-TRI for feature review, please. I'm hoping the build system changes are clear enough for a general developer audience. The comments in the |
There was a problem hiding this comment.
As a representative of the "general developer audience", I think I can say that the build machinations could be followed but not vetted. I was able to tell myself a story about everything I saw that placed it all in a coherent narrative, but I couldn't generate it on my own and doubt that I could make assertions about correctness. There are enough voodoo bazel rules being used with semantics that aren't clear from the invocation.
So, in conclusion, I think the general developer audience could follow this. But they couldn't validate nor generate it.
Reviewed 19 of 23 files at r1, 4 of 4 files at r2, all commit messages.
Reviewable status: needs at least two assigned reviewers (waiting on @jwnimmer-tri)
There was a problem hiding this comment.
I suppose the most important thing is if a developer gets a build error, how able would they be to fix it on their own? Like if they forgot a DRAKE_NO_EXPORT and saw an attribute error. Ah well, I guess we'll find out.
+@sammy-tri for platform review per schedule, please.
Reviewable status: LGTM missing from assignee sammy-tri(platform) (waiting on @sammy-tri)
There was a problem hiding this comment.
Yep. I subscribe to the "we'll find out" as we go. It'll be educational. And, quite simply, it's not like fcl is widely referenced.
Reviewable status: LGTM missing from assignee sammy-tri(platform) (waiting on @sammy-tri)
There was a problem hiding this comment.
Reviewed 19 of 23 files at r1, 4 of 4 files at r2, all commit messages.
Reviewable status:complete! all discussions resolved, LGTM from assignees sammy-tri(platform),SeanCurtis-TRI(platform) (waiting on @jwnimmer-tri)
Fix
ccdBUILD file to disallow things like#include "polytope.h"which previously resolved to itssrcdirectory.Remove unnecessary fcl version rules; use strict genrule checking.
Towards #17231.
This change is