-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
[workspace] Don't build libsdformat's URDF parser #16772
[workspace] Don't build libsdformat's URDF parser #16772
Conversation
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 9 of 9 files at r1, all commit messages.
Reviewable status: 1 unresolved discussion, LGTM missing from assignee EricCousineau-TRI(platform) (waiting on @EricCousineau-TRI and @jwnimmer-tri)
tools/workspace/sdformat/patches/no_urdf.patch, line 1 at r1 (raw file):
Disable transmorification of URDF files into SDFormat files.
typo
Code quote:
transmorification
Drake has its own URDF parser that we must always use, so we should disable transmogrification of URDF files into SDFormat files. Don't generate sdf/sdf.hh; it's only effect is IWYU violations.
87d686a
to
ed814a8
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.
-@EricCousineau-TRI +@sherm1 for platform review per schedule (tomorrow), please.
\CC @sammy-tri FYI for awareness.
Reviewable status: LGTM missing from assignee sherm1(platform) (waiting on @rpoyner-tri and @sherm1)
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 1 of 1 files at r2, all commit messages.
Reviewable status: LGTM missing from assignee sherm1(platform) (waiting on @sherm1)
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.
Platform . I BTW'd three lines with trailing blanks in case you want to fix them.
Reviewed 8 of 9 files at r1, 1 of 1 files at r2, all commit messages.
Reviewable status:complete! all discussions resolved, LGTM from assignees rpoyner-tri(platform),sherm1(platform) (waiting on @jwnimmer-tri)
tools/workspace/sdformat/patches/no_urdf.patch, line 14 at r2 (raw file):
-#include "parser_urdf.hh" +// #include "parser_urdf.hh"
BTW trailing blanks
tools/workspace/sdformat/patches/no_urdf.patch, line 30 at r2 (raw file):
} +#endif // Disable URDF2SDF for Drake.
BTW trailing blanks
tools/workspace/sdformat/patches/no_urdf.patch, line 46 at r2 (raw file):
} +#endif // Disable URDF2SDF for Drake.
BTW trailing blanks
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.
Those blanks are how diff
and patch
work by default, so I'm going to leave them alone.
Reviewable status:
complete! all discussions resolved, LGTM from assignees rpoyner-tri(platform),sherm1(platform) (waiting on @jwnimmer-tri)
Thanks! \cc @azeey |
Drake has its own URDF parser that we must always use, so we should disable libsdformat's automatic transmogrification of URDF files into SDFormat files.
Don't generate
sdf/sdf.hh
; it's only effect is IWYU violations.Towards #12610.
+@rpoyner-tri for feature review, please.
+@EricCousineau-TRI for platform review (as SME), please.
Should we have anyone from OpenRobotics take a look here as well?
This change is![Reviewable](https://camo.githubusercontent.com/1541c4039185914e83657d3683ec25920c672c6c5c7ab4240ee7bff601adec0b/68747470733a2f2f72657669657761626c652e696f2f7265766965775f627574746f6e2e737667)