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

[parsing] Add UsdParser smoke test stub #21028

Merged
merged 1 commit into from
Feb 27, 2024

Conversation

jwnimmer-tri
Copy link
Collaborator

@jwnimmer-tri jwnimmer-tri commented Feb 23, 2024

This turns up a slew of previously-unseen errors in our OpenUSD build system, which we also fix here.

Towards #20898. Amends #20923.

Requires:

Sample command for local testing:

bazel test --define=WITH_USD=ON //multibody/parsing:detail_usd_parser_test 

This change is Reviewable

@jwnimmer-tri jwnimmer-tri added priority: low release notes: none This pull request should not be mentioned in the release notes labels Feb 23, 2024
@jwnimmer-tri
Copy link
Collaborator Author

jwnimmer-tri commented Feb 23, 2024

+@rpoyner-tri for review, please.

I think it's useful to feature-review this, but I'm not sure if we should land it yet. Seeking your advice:

None of this new code is run/covered in CI, because I haven't finished vendoring Boost yet (and OpenUSD needs Boost) suitably for CI. On local Jammy, if we have already have Ubuntu boost installed then this all works OK and we can manually test the code.

For the prior little smoke test of a few lines, I was OK without CI. Now that this is hundreds of lines, plausibly we should wait to merge until I first get Boost out of the way. WDYT?

Copy link
Contributor

@rpoyner-tri rpoyner-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: with a question about test output

Reviewed 10 of 10 files at r1, all commit messages.
Reviewable status: 1 unresolved discussion, needs at least two assigned reviewers

a discussion (no related file):
minor The log output for the stub test so far is perhaps less than inspiring:

rico@torg:~/checkout/drake$ bazel run  --define=WITH_USD=ON  //multibody/parsing:detail_usd_parser_test
INFO: Invocation ID: 88d2ab81-f06f-4898-838a-9446a8ef8a38
INFO: Analyzed target //multibody/parsing:detail_usd_parser_test (0 packages loaded, 2 targets configured).
INFO: Found 1 target...
Target //multibody/parsing:detail_usd_parser_test up-to-date:
  bazel-bin/multibody/parsing/detail_usd_parser_test
INFO: Elapsed time: 15.639s, Critical Path: 15.20s
INFO: 7 processes: 5 internal, 2 linux-sandbox.
INFO: Build completed successfully, 7 total actions
INFO: Running command line: external/bazel_tools/tools/test/test-setup.sh multibody/parsing/detail_usd_parser_test
exec ${PAGER:-/usr/bin/less} "$0" || exit 1
Executing tests from //multibody/parsing:detail_usd_parser_test
-----------------------------------------------------------------------------
Using drake_cc_googletest_main.cc

[==========] Running 1 test from 1 test suite.
[----------] Global test environment set-up.
[----------] 1 test from UsdParserTest
[ RUN      ] UsdParserTest.Stub
Coding Error: in _Load at line 260 of external/openusd_internal/pxr/base/plug/plugin.cpp -- Failed to load plugin 'usd': /home/rico/.cache/bazel/_bazel_rico/7f8997f28c9253517a55d673c67a6c74/execroot/drake/bazel-out/k8-opt/bin/multibody/parsing/detail_usd_parser_test.runfiles/openusd_internal/pxr/usd/usd/@PLUG_INFO_ROOT@/@PLUG_INFO_LIBRARY_PATH@: cannot open shared object file: No such file or directory in '/home/rico/.cache/bazel/_bazel_rico/7f8997f28c9253517a55d673c67a6c74/execroot/drake/bazel-out/k8-opt/bin/multibody/parsing/detail_usd_parser_test.runfiles/openusd_internal/pxr/usd/usd/@PLUG_INFO_ROOT@/@PLUG_INFO_LIBRARY_PATH@'
Coding Error: in _Load at line 260 of external/openusd_internal/pxr/base/plug/plugin.cpp -- Failed to load plugin 'usd': /home/rico/.cache/bazel/_bazel_rico/7f8997f28c9253517a55d673c67a6c74/execroot/drake/bazel-out/k8-opt/bin/multibody/parsing/detail_usd_parser_test.runfiles/openusd_internal/pxr/usd/usd/@PLUG_INFO_ROOT@/@PLUG_INFO_LIBRARY_PATH@: cannot open shared object file: No such file or directory in '/home/rico/.cache/bazel/_bazel_rico/7f8997f28c9253517a55d673c67a6c74/execroot/drake/bazel-out/k8-opt/bin/multibody/parsing/detail_usd_parser_test.runfiles/openusd_internal/pxr/usd/usd/@PLUG_INFO_ROOT@/@PLUG_INFO_LIBRARY_PATH@'
Coding Error: in Create at line 381 of external/openusd_internal/pxr/usd/ar/resolver.cpp -- Failed to load plugin usd for Usd_UsdzResolver
[       OK ] UsdParserTest.Stub (19 ms)
[----------] 1 test from UsdParserTest (19 ms total)

[----------] Global test environment tear-down
[==========] 1 test from 1 test suite ran. (19 ms total)
[  PASSED  ] 1 test.

But maybe that is just where we are. Is there anything that should worry me here?


Copy link
Collaborator Author

@jwnimmer-tri jwnimmer-tri left a comment

Choose a reason for hiding this comment

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

Reviewable status: 1 unresolved discussion, needs at least two assigned reviewers

a discussion (no related file):

But maybe that is just where we are.

Yup, that's how I view it.

OpenUSD uses it's own implementation of "runfiles" quite a bit (it calls them "resource files"). This code does the bare minimum to make the "stage" constructor not segfault, but I imagine that soon enough we'llll also need to fix the deeper problems. We'll need to scrape the list of RESOURCE_FILES from the assorted CMakeLists.txt into our lockfile summary, and then run cmake_configure_file on all of them and add them as data = [] dependencies. (The lack of cmake_configure_file reprocessing is why we see @FOO@ complaints from the plugin tool.)

More fundamentally, we might need to need the plugin system to be inert -- we really don't want Drake's MbP parser scanning through random files on the user's disk drive trying to dlopen unwanted plugins.

Is there anything that should worry me here?

I think it'll be journey to get this particular parser up and running. Certainly spewing errors will need to be fixed before we enable it by default. My thought process is to capture small bits of ground and hold them.

There's an argument to be made for me trying to write down all of the hairballs as TODOs, but on the other hand this feature is so early in its life that they would probably not be very durable, and churn overly much. For now, I'm OK with having the test programs reflect what's working (or not) and keep going from there.


Copy link
Contributor

@rpoyner-tri rpoyner-tri left a comment

Choose a reason for hiding this comment

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

Wait for boost or not: let's wait, since we have one cycle of evidence that each step of integration is churning up plenty of fresh skeletons.

Reviewable status: needs at least two assigned reviewers

Copy link
Contributor

@rpoyner-tri rpoyner-tri left a comment

Choose a reason for hiding this comment

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

Reviewed 10 of 10 files at r2, all commit messages.
Reviewable status: needs at least two assigned reviewers, labeled "do not merge", commits need curation (https://drake.mit.edu/reviewable.html#curated-commits) (waiting on @jwnimmer-tri)

@jwnimmer-tri
Copy link
Collaborator Author

+@sherm1 do you have the energy for one more platform review today? If not that's fine, we can pass it over to Sean.

Copy link
Member

@sherm1 sherm1 left a comment

Choose a reason for hiding this comment

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

Ah, I think I'm out of gas and time for this lovely Monday. Sorry, Sean! -@sherm1 +@SeanCurtis-TRI for platform review, please (off rotation)

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

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: With the question indicated below.

Reviewed 8 of 10 files at r1, 1 of 10 files at r2, 9 of 9 files at r3, all commit messages.
Reviewable status: 1 unresolved discussion

a discussion (no related file):
When I run the test, I get the following written to the console:

Coding Error: in _Load at line 260 of external/openusd_internal/pxr/base/plug/plugin.cpp -- Failed to load plugin 'usd': /home/seancurtis/.cache/bazel/_bazel_seancurtis/c482a09839de82c7b8c8a508d26ab2c3/execroot/drake/bazel-out/k8-opt/bin/multibody/parsing/detail_usd_parser_test.runfiles/openusd_internal/pxr/usd/usd/@PLUG_INFO_ROOT@/@PLUG_INFO_LIBRARY_PATH@: cannot open shared object file: No such file or directory in '/home/seancurtis/.cache/bazel/_bazel_seancurtis/c482a09839de82c7b8c8a508d26ab2c3/execroot/drake/bazel-out/k8-opt/bin/multibody/parsing/detail_usd_parser_test.runfiles/openusd_internal/pxr/usd/usd/@PLUG_INFO_ROOT@/@PLUG_INFO_LIBRARY_PATH@'
Coding Error: in _Load at line 260 of external/openusd_internal/pxr/base/plug/plugin.cpp -- Failed to load plugin 'usd': /home/seancurtis/.cache/bazel/_bazel_seancurtis/c482a09839de82c7b8c8a508d26ab2c3/execroot/drake/bazel-out/k8-opt/bin/multibody/parsing/detail_usd_parser_test.runfiles/openusd_internal/pxr/usd/usd/@PLUG_INFO_ROOT@/@PLUG_INFO_LIBRARY_PATH@: cannot open shared object file: No such file or directory in '/home/seancurtis/.cache/bazel/_bazel_seancurtis/c482a09839de82c7b8c8a508d26ab2c3/execroot/drake/bazel-out/k8-opt/bin/multibody/parsing/detail_usd_parser_test.runfiles/openusd_internal/pxr/usd/usd/@PLUG_INFO_ROOT@/@PLUG_INFO_LIBRARY_PATH@'
Coding Error: in Create at line 381 of external/openusd_internal/pxr/usd/ar/resolver.cpp -- Failed to load plugin usd for Usd_UsdzResolver

The test otherwise passes. Should this be a source of concern? Seems to me the plugin for "usd" would be critical.


Copy link
Collaborator Author

@jwnimmer-tri jwnimmer-tri left a comment

Choose a reason for hiding this comment

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

Reviewable status: 1 unresolved discussion

a discussion (no related file):

Previously, SeanCurtis-TRI (Sean Curtis) wrote…

When I run the test, I get the following written to the console:

Coding Error: in _Load at line 260 of external/openusd_internal/pxr/base/plug/plugin.cpp -- Failed to load plugin 'usd': /home/seancurtis/.cache/bazel/_bazel_seancurtis/c482a09839de82c7b8c8a508d26ab2c3/execroot/drake/bazel-out/k8-opt/bin/multibody/parsing/detail_usd_parser_test.runfiles/openusd_internal/pxr/usd/usd/@PLUG_INFO_ROOT@/@PLUG_INFO_LIBRARY_PATH@: cannot open shared object file: No such file or directory in '/home/seancurtis/.cache/bazel/_bazel_seancurtis/c482a09839de82c7b8c8a508d26ab2c3/execroot/drake/bazel-out/k8-opt/bin/multibody/parsing/detail_usd_parser_test.runfiles/openusd_internal/pxr/usd/usd/@PLUG_INFO_ROOT@/@PLUG_INFO_LIBRARY_PATH@'
Coding Error: in _Load at line 260 of external/openusd_internal/pxr/base/plug/plugin.cpp -- Failed to load plugin 'usd': /home/seancurtis/.cache/bazel/_bazel_seancurtis/c482a09839de82c7b8c8a508d26ab2c3/execroot/drake/bazel-out/k8-opt/bin/multibody/parsing/detail_usd_parser_test.runfiles/openusd_internal/pxr/usd/usd/@PLUG_INFO_ROOT@/@PLUG_INFO_LIBRARY_PATH@: cannot open shared object file: No such file or directory in '/home/seancurtis/.cache/bazel/_bazel_seancurtis/c482a09839de82c7b8c8a508d26ab2c3/execroot/drake/bazel-out/k8-opt/bin/multibody/parsing/detail_usd_parser_test.runfiles/openusd_internal/pxr/usd/usd/@PLUG_INFO_ROOT@/@PLUG_INFO_LIBRARY_PATH@'
Coding Error: in Create at line 381 of external/openusd_internal/pxr/usd/ar/resolver.cpp -- Failed to load plugin usd for Usd_UsdzResolver

The test otherwise passes. Should this be a source of concern? Seems to me the plugin for "usd" would be critical.

Rico asked the same thing (unfold the other top-level discussion). Maybe I should add a comment disclaimer to the test.


This turns up a slew of previously-unseen errors in our OpenUSD build
system, which we also fix here.
Copy link
Collaborator Author

@jwnimmer-tri jwnimmer-tri left a comment

Choose a reason for hiding this comment

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

Reviewable status: 1 unresolved discussion

a discussion (no related file):

Previously, jwnimmer-tri (Jeremy Nimmer) wrote…

Rico asked the same thing (unfold the other top-level discussion). Maybe I should add a comment disclaimer to the test.

Done


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.

Reviewed 1 of 1 files at r4, all commit messages.
Reviewable status: :shipit: complete! all discussions resolved, LGTM from assignees rpoyner-tri(platform),SeanCurtis-TRI(platform)

Copy link
Contributor

@rpoyner-tri rpoyner-tri left a 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 r3, 1 of 1 files at r4, all commit messages.
Reviewable status: :shipit: complete! all discussions resolved, LGTM from assignees rpoyner-tri(platform),SeanCurtis-TRI(platform)

@rpoyner-tri rpoyner-tri merged commit 37a33c7 into RobotLocomotion:master Feb 27, 2024
10 checks passed
@jwnimmer-tri jwnimmer-tri deleted the openusd-linking branch February 28, 2024 04:35
jwnimmer-tri added a commit to jwnimmer-tri/drake that referenced this pull request Apr 1, 2024
This turns up a slew of previously-unseen errors in our OpenUSD build
system, which we also fix here.
RussTedrake pushed a commit to RussTedrake/drake that referenced this pull request Dec 15, 2024
This turns up a slew of previously-unseen errors in our OpenUSD build
system, which we also fix here.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: low release notes: none This pull request should not be mentioned in the release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants