-
Notifications
You must be signed in to change notification settings - Fork 38
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
Scraping error when building drake_ros
package via Bazel
#281
Comments
drake_ros
package via Bazel drake_ros
package via Bazel
Is this still an issue after #268? If so, it looks like there's an issue parsing plugin description xml files where possibly it's not valid XML. Maybe |
@sloretz Yes I am using the latest version of everything so I do believe this is still an issue. I agree 100% that's exactly what I suggested a while back on drake-slack I believe Suggested snippet. I can wrap it up in a PR if you'd like. Thanks!
|
do ya'll want me to PR it up as well? I can do that no worries! cc: @EricCousineau-TRI |
I hit this
as you can see I modified it to print the file name so I could isolate it, and it turns out in fact that is invalid XML
I'll just uninstall it to move on but given that its a distribution provided file ... not many great options. |
I suppose ros-perception/laser_filters#183 hasn't percolated into the repositories yet? |
No
|
Would it be desirable to somehow make the scraping more robust especially when the packages are not directly required for drake-ros to function? Otherwise any one package even if it's a non dependency doesn't allow the user to 1) use drake-ros via bazel 2) help them get around it without adding some logic (similar to the snippet I shared above) to find the root cause. IMO this affects the bazel workflow UX a lot. Just my 2 cents. I personally moved all my ros/drake-ros code away from bazel due to how often I ran into this problem. |
Yeah, ideally, we just squelch the scraping error if the package is not in the transitive set of deps specified - sorry you had to run into that! |
running into this again at bdai now hehe. Is there a possible near term solution? cc: @jwnimmer-tri @EricCousineau-TRI |
I think we would welcome any pull requests with improvements. As I understand it, this metadata is often unused anyway. So, it seems like reifying the error and proceeding would be the way to go. Something like this: --- a/bazel_ros2_rules/ros2/resources/ros2bzl/scraping/metadata.py
+++ b/bazel_ros2_rules/ros2/resources/ros2bzl/scraping/metadata.py
@@ -131,8 +131,14 @@ def collect_ros_package_metadata(name, prefix):
if not os.path.isabs(path_to_desc):
path_to_desc = os.path.join(prefix, path_to_desc)
if os.path.exists(path_to_desc):
- plugin_libraries.extend(parse_plugins_description_xml(
- path_to_desc)['plugin_libraries'])
+ try:
+ new_plugin_libraries = parse_plugins_description_xml(
+ path_to_desc)['plugin_libraries']
+ except Exception:
+ new_plugin_libraries = [
+ f"{path_to_desc}-had-a-parse-error-so-is.missing",
+ ]
+ plugin_libraries.extend(new_plugin_libraries)
if plugin_libraries:
metadata['plugin_libraries'] = plugin_libraries
A more nuanced approach would be to add a field like |
Or maybe even the calls to |
Hey everyone, I am running into to scraping errors when I try building with Bazel via
bazel build ...
I had been away for a bit from drake stuff, so I am not sure if I am missing anything. Thanks for all the help!
Setup -
apt-get
; path to source is/opt/ros/humble
)Error log -
The text was updated successfully, but these errors were encountered: