Open adeeb10abbas opened 1 year ago
sloretz
commented
1 year ago Is this still an issue after https://github.com/RobotLocomotion/drake-ros/pull/268?
If so, it looks like there's an issue parsing plugin description xml files where possibly it's not valid XML. Maybe parse_plugins_description_xml here should error with the file name and content when a parse error occurs?
adeeb10abbas
commented
1 year ago @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!
def parse_plugins_description_xml(path_to_plugins_description_xml):
try:
plugins_description_xml = ET.parse(path_to_plugins_description_xml)
except ET.ParseError as e:
raise ValueError(f"Failed to parse XML file '{path_to_plugins_description_xml}': {e}")
root = plugins_description_xml.getroot()
libraries = []
if root.tag not in ['class_libraries', 'library']:
raise ValueError(f"Invalid root tag '{root.tag}' in '{path_to_plugins_description_xml}', expected 'class_libraries' or 'library'")
if 'class_libraries' == root.tag:
for child in root.findall('library'):
libraries.append(child.attrib['path'])
else:
libraries.append(root.attrib['path'])
if not libraries:
raise ValueError(f"No libraries found in '{path_to_plugins_description_xml}'")
return dict(plugin_libraries=libraries)do ya'll want me to PR it up as well? I can do that no worries! cc: @EricCousineau-TRI
anrp-tri
commented
1 year ago I hit this
--- captured stdout ---
/opt/ros/humble/share/rqt_gui_cpp/plugin.xml
/opt/ros/humble/share/rosbag2_compression_zstd/plugin_description.xml
/opt/ros/humble/share/compressed_image_transport/compressed_plugins.xml
/opt/ros/humble/share/laser_filters/laser_filters_plugins.xml
--- captured stderr ---
Traceback (most recent call last):
File "/home/anrp/.cache/bazel/_bazel_anrp/c23bb5f1355794ee7d894887257ca9d6/external/bazel_ros2_rules/ros2/scrape_distribution.py", line 54, in <module>
main()
File "/home/anrp/.cache/bazel/_bazel_anrp/c23bb5f1355794ee7d894887257ca9d6/external/bazel_ros2_rules/ros2/scrape_distribution.py", line 46, in main
distro = scrape_distribution(
File "/home/anrp/.cache/bazel/_bazel_anrp/c23bb5f1355794ee7d894887257ca9d6/external/ros2/resources/ros2bzl/scraping/__init__.py", line 96, in scrape_distribution
index_all_packages(), include, exclude)
File "/home/anrp/.cache/bazel/_bazel_anrp/c23bb5f1355794ee7d894887257ca9d6/external/ros2/resources/ros2bzl/scraping/__init__.py", line 29, in index_all_packages
packages = {
File "/home/anrp/.cache/bazel/_bazel_anrp/c23bb5f1355794ee7d894887257ca9d6/external/ros2/resources/ros2bzl/scraping/__init__.py", line 30, in <dictcomp>
name: collect_ros_package_metadata(name, prefix)
File "/home/anrp/.cache/bazel/_bazel_anrp/c23bb5f1355794ee7d894887257ca9d6/external/ros2/resources/ros2bzl/scraping/metadata.py", line 135, in collect_ros_package_metadata
plugin_libraries.extend(parse_plugins_description_xml(
File "/home/anrp/.cache/bazel/_bazel_anrp/c23bb5f1355794ee7d894887257ca9d6/external/ros2/resources/ros2bzl/scraping/metadata.py", line 36, in parse_plugins_description_xml
plugins_description_xml = ET.parse(path_to_plugins_description_xml)
File "/usr/lib/python3.10/xml/etree/ElementTree.py", line 1222, in parse
tree.parse(source, parser)
File "/usr/lib/python3.10/xml/etree/ElementTree.py", line 580, in parse
self._root = parser._parse_whole(source)
xml.etree.ElementTree.ParseError: not well-formed (invalid token): line 4, column 41as 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
<class_libraries>
<library path="laser_scan_filters">
<class name="laser_filters/LaserArrayFilter" type="laser_filters::LaserArrayFilter"
base_class_type="filters::FilterBase<sensor_msgs::msg::LaserScan>">
<description>
This is a filter which runs two internal MultiChannelFilterChain filters on the range and intensity measurements.
</description>
</class>
<class name="laser_filters/LaserScanIntensityFilter" type="laser_filters::LaserScanIntensityFilter"
base_class_type="filters::FilterBase<sensor_msgs::msg::LaserScan>">I'll just uninstall it to move on but given that its a distribution provided file ... not many great options. (Specifically an XML attribute cannot have an unescaped < inside double quotes, it is supposed to be < etc.)
jwnimmer-tri
commented
1 year ago I suppose https://github.com/ros-perception/laser_filters/issues/183 hasn't percolated into the repositories yet?
anrp-tri
commented
1 year ago No
Package: ros-humble-laser-filters
Version: 2.0.6-2jammy.20230721.220613
Architecture: amd64
Maintainer: Jon Binney <jon.binney@gmail.com>
Installed-Size: 2381
Depends: libc6 (>= 2.34), libconsole-bridge1.0 (>= 1.0.1+dfsg2), libgcc-s1 (>= 3.3.1), libstdc++6 (>= 11), libtinyxml2-9 (>= 8.0.0), ros-humble-angles, ros-humble-filters, ros-humble-laser-geometry, ros-humble-message-filters, ros-humble-pluginlib, ros-humble-rclcpp, ros-humble-rclcpp-lifecycle, ros-humble-sensor-msgs, ros-humble-tf2, ros-humble-tf2-ros, ros-humble-ros-workspace
Homepage: http://ros.org/wiki/laser_filters
Priority: optional
Section: misc
Filename: pool/main/r/ros-humble-laser-filters/ros-humble-laser-filters_2.0.6-2jammy.20230721.220613_amd64.deb
Size: 494294
SHA256: 4457ecc315cf0cd22a00fd3ab1055202854161b97c3131976289db81ae5549f0
SHA1: 8395e66830a9afb751f0f18a6d5e718e5a92d4dc
MD5sum: 622e94c178baa7207d5917bd11b648bf
Description: Assorted filters designed to operate on 2D planar laser scanners, which use the sensor_msgs/LaserScan type.
Description-md5: 6b0af53bda18651428864370f05f3902
root@jammy-ros2:~# apt-get install !$
apt-get install ros-humble-laser-filters
[...]
The following NEW packages will be installed:
ros-humble-laser-filters
0 upgraded, 1 newly installed, 0 to remove and 336 not upgraded.
Need to get 494 kB of archives.
After this operation, 2,438 kB of additional disk space will be used.
Get:1 http://packages.ros.org/ros2/ubuntu jammy/main amd64 ros-humble-laser-filters amd64 2.0.6-2jammy.20230721.220613 [494 kB]
Fetched 494 kB in 14s (35.4 kB/s)
Selecting previously unselected package ros-humble-laser-filters.
(Reading database ... 193806 files and directories currently installed.)
Preparing to unpack .../ros-humble-laser-filters_2.0.6-2jammy.20230721.220613_amd64.deb ...
Unpacking ros-humble-laser-filters (2.0.6-2jammy.20230721.220613) ...
Setting up ros-humble-laser-filters (2.0.6-2jammy.20230721.220613) ...
root@jammy-ros2:~# head /opt/ros/humble/share/laser_filters/laser_filters_plugins.xml
<class_libraries>
<library path="laser_scan_filters">
<class name="laser_filters/LaserArrayFilter" type="laser_filters::LaserArrayFilter"
base_class_type="filters::FilterBase<sensor_msgs::msg::LaserScan>">
<description>
This is a filter which runs two internal MultiChannelFilterChain filters on the range and intensity measurements.
</description>
</class>
<class name="laser_filters/LaserScanIntensityFilter" type="laser_filters::LaserScanIntensityFilter"
base_class_type="filters::FilterBase<sensor_msgs::msg::LaserScan>">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.
EricCousineau-TRI
commented
1 year ago 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!
ali-bdai
commented
7 months ago running into this again at bdai now hehe. Is there a possible near term solution? cc: @jwnimmer-tri @EricCousineau-TRI
jwnimmer-tri
commented
7 months ago 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 metadata[errors] = [...] with detailed error messages, and then all of the users of the metadata would be required to check for an errors entry before proceeding. Or maybe index_all_packages() could check for (and print out?) any errors, and then cull any error-ful packages from the full index.
jwnimmer-tri
commented
7 months ago Or maybe even the calls to collect_ros_package_metadata should be what's wrapped in a try-except. I don't think we want any scraping error to bomb the whole thing.
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 -