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

[rosidl] Ensure *_defs data is added to *_py and *_cc targets #305

Conversation

EricCousineau-TRI
Copy link
Collaborator

@EricCousineau-TRI EricCousineau-TRI commented Oct 26, 2023

Towards Anzu bugfix


This change is Reviewable

Copy link
Collaborator Author

@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.

+@kunimatsu-tri for review, please!

Reviewable status: 0 of 2 files reviewed, all discussions resolved (waiting on @kunimatsu-tri)

@EricCousineau-TRI
Copy link
Collaborator Author

Bazel Drake-ROS Continuous Integration tests pass locally for me.

Copy link
Collaborator Author

@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.

Reviewable status: 0 of 2 files reviewed, 1 unresolved discussion (waiting on @EricCousineau-TRI and @kunimatsu-tri)

a discussion (no related file):
Working: Certain Anzu tests fail with something like:

  File "{runfiles}/bazel_ros2_rules/ros2/tools/dload_shim.py", line 58, in do_dload_shim
    os.execv(real_executable_path, argv)
OSError: [Errno 7] Argument list too long

Copy link
Collaborator Author

@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.

Reviewable status: 0 of 2 files reviewed, 1 unresolved discussion (waiting on @EricCousineau-TRI and @kunimatsu-tri)

a discussion (no related file):

Previously, EricCousineau-TRI (Eric Cousineau) wrote…

Working: Certain Anzu tests fail with something like:

  File "{runfiles}/bazel_ros2_rules/ros2/tools/dload_shim.py", line 58, in do_dload_shim
    os.execv(real_executable_path, argv)
OSError: [Errno 7] Argument list too long

The AMENT_INDEX_PATH environment variable has ~800 duplicate entries for rosidl_generate_ament_index_entry

These should be deduplicated


@kunimatsu-tri
Copy link

bazel_ros2_rules/ros2/rosidl.bzl line 402 at r1 (raw file):

        interfaces,
        includes = [],
        data = [],

nit Can I ask you to add a corresponding doc string to the data argument?

Ditto for all the interface changes.

@kunimatsu-tri
Copy link

Previously, EricCousineau-TRI (Eric Cousineau) wrote…

The AMENT_INDEX_PATH environment variable has ~800 duplicate entries for rosidl_generate_ament_index_entry

These should be deduplicated

:(

Copy link
Collaborator Author

@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.

Reviewable status: 0 of 3 files reviewed, all discussions resolved (waiting on @kunimatsu-tri)

a discussion (no related file):

Previously, kunimatsu-tri (Kunimatsu Hashimoto) wrote…

:(

Done.



bazel_ros2_rules/ros2/rosidl.bzl line 402 at r1 (raw file):

Previously, kunimatsu-tri (Kunimatsu Hashimoto) wrote…

nit Can I ask you to add a corresponding doc string to the data argument?

Ditto for all the interface changes.

Done.

Copy link

@kunimatsu-tri kunimatsu-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 2 files at r1, 1 of 2 files at r2, all commit messages.
Reviewable status: 2 of 3 files reviewed, all discussions resolved

@kunimatsu-tri
Copy link

Previously, EricCousineau-TRI (Eric Cousineau) wrote…

Done.

:)

Copy link

@kunimatsu-tri kunimatsu-tri left a comment

Choose a reason for hiding this comment

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

I confirmed that custom_message_list_test passes locally.

Thanks for this quick fix! :lgtm:

Reviewable status: 2 of 3 files reviewed, all discussions resolved

@kunimatsu-tri
Copy link

Build drake_ros in CI failed with the following error:

ERROR: /home/runner/.cache/bazel/\_bazel\_runner/d07b19e4bc14c7eead45599d9bb941ba/external/vtk/BUILD.bazel:31:11: Middleman \_middlemen/@vtk\_S\_S\_Cvtkkwiml-cc\_library-compile failed: missing input file '@vtk//:include/vtk-9.1/vtk\_kwiml.h'

[136](https://github.com/RobotLocomotion/drake-ros/actions/runs/6661541550/job/18104550754#step:13:137)ERROR: /home/runner/.cache/bazel/\_bazel\_runner/d07b19e4bc14c7eead45599d9bb941ba/external/vtk/BUILD.bazel:31:11: Middleman \_middlemen/@vtk\_S\_S\_Cvtkkwiml-cc\_library-compile failed: missing input file '@vtk//:include/vtk-9.1/vtkkwiml/abi.h'

[137](https://github.com/RobotLocomotion/drake-ros/actions/runs/6661541550/job/18104550754#step:13:138)ERROR: /home/runner/.cache/bazel/\_bazel\_runner/d07b19e4bc14c7eead45599d9bb941ba/external/vtk/BUILD.bazel:31:11: Middleman \_middlemen/@vtk\_S\_S\_Cvtkkwiml-cc\_library-compile failed: missing input file '@vtk//:include/vtk-9.1/vtkkwiml/int.h'

[138](https://github.com/RobotLocomotion/drake-ros/actions/runs/6661541550/job/18104550754#step:13:139)ERROR: /home/runner/.cache/bazel/\_bazel\_runner/d07b19e4bc14c7eead45599d9bb941ba/external/vtk/BUILD.bazel:31:11: Middleman \_middlemen/@vtk\_S\_S\_Cvtkkwiml-cc\_library-compile failed: 3 input file(s) do not exist

Copy link
Contributor

@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: 2 of 3 files reviewed, 1 unresolved discussion (waiting on @kunimatsu-tri)

a discussion (no related file):

Previously, kunimatsu-tri (Kunimatsu Hashimoto) wrote…

Build drake_ros in CI failed with the following error:

ERROR: /home/runner/.cache/bazel/\_bazel\_runner/d07b19e4bc14c7eead45599d9bb941ba/external/vtk/BUILD.bazel:31:11: Middleman \_middlemen/@vtk\_S\_S\_Cvtkkwiml-cc\_library-compile failed: missing input file '@vtk//:include/vtk-9.1/vtk\_kwiml.h'

[136](https://github.com/RobotLocomotion/drake-ros/actions/runs/6661541550/job/18104550754#step:13:137)ERROR: /home/runner/.cache/bazel/\_bazel\_runner/d07b19e4bc14c7eead45599d9bb941ba/external/vtk/BUILD.bazel:31:11: Middleman \_middlemen/@vtk\_S\_S\_Cvtkkwiml-cc\_library-compile failed: missing input file '@vtk//:include/vtk-9.1/vtkkwiml/abi.h'

[137](https://github.com/RobotLocomotion/drake-ros/actions/runs/6661541550/job/18104550754#step:13:138)ERROR: /home/runner/.cache/bazel/\_bazel\_runner/d07b19e4bc14c7eead45599d9bb941ba/external/vtk/BUILD.bazel:31:11: Middleman \_middlemen/@vtk\_S\_S\_Cvtkkwiml-cc\_library-compile failed: missing input file '@vtk//:include/vtk-9.1/vtkkwiml/int.h'

[138](https://github.com/RobotLocomotion/drake-ros/actions/runs/6661541550/job/18104550754#step:13:139)ERROR: /home/runner/.cache/bazel/\_bazel\_runner/d07b19e4bc14c7eead45599d9bb941ba/external/vtk/BUILD.bazel:31:11: Middleman \_middlemen/@vtk\_S\_S\_Cvtkkwiml-cc\_library-compile failed: 3 input file(s) do not exist

BTW That's because #304 upgraded the Drake pin but did not account for the VTK deprecation warning. The contact_markers_system.cc is doing #include <vtkImageData.h> using Drake's deprecated VTK repository rule, but somehow has not installed libvtk9-dev? However, I do see drake_ros/package.xml has <depend>libvtk9-dev</depend> so I'm not sure why that package apparently isn't installed in CI.


@EricCousineau-TRI
Copy link
Collaborator Author

Ah, nuts. For now, I'll merge this PR (since testing passed locally) and fix the vtk issue in a separate PR.

@EricCousineau-TRI EricCousineau-TRI merged commit 8e8f411 into RobotLocomotion:main Oct 27, 2023
6 of 7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants