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

Conflicting version requirements urdfdom #138

Closed
Tobias-Fischer opened this issue Jun 7, 2021 · 8 comments
Closed

Conflicting version requirements urdfdom #138

Tobias-Fischer opened this issue Jun 7, 2021 · 8 comments

Comments

@Tobias-Fischer
Copy link
Collaborator

Tobias-Fischer commented Jun 7, 2021

Hi @wolfv,
We currently have conflicting requirements of urdfdom:

 gazebo       11.5.1  haa37ec6_2    libsdformat >=9.3.0,<10.0a0  conda-forge/linux-64
 libsdformat  9.3.0   hf765594_3     urdfdom >=2.3.3,<2.4.0a0     conda-forge/linux-64

&

 ros-noetic-rviz 1.14.7  py38h7d895da_9 urdfdom >=1.0.4,<1.1.0a0 robostack/linux-64  

Is there any reason why we pin urdfdom to version 1 in RoboStack?

@Tobias-Fischer
Copy link
Collaborator Author

I now realize that urdfdom 1 is used in ROS 1 .. so we need it here. That in turn means we need to build libsdformat, gazebo etc for both urdfdom 1 and urdfdom 2, is that right @traversaro @wolfv?

@traversaro
Copy link
Member

Related PRs (for generating back-links):

@traversaro
Copy link
Member

I now realize that urdfdom 1 is used in ROS 1 .. so we need it here. That in turn means we need to build libsdformat, gazebo etc for both urdfdom 1 and urdfdom 2, is that right @traversaro @wolfv?

The situation with urdfdom is unfortunately a "bit" complicated. The main reason for this is due to the history of urdfdom being a ROS package that at some point was transformed in a ROS-agnostic package to permit to package it in Debian/Ubuntu main repos so that it could become a dependency of sdformat. Unfortunately, this decision was never well received by some members of the community, as anything that is packaged in Debian/Ubuntu main repos cannot be easily (and quickly) improved/updated, due to the fact that most ROS users use Ubuntu LTS versions (this is exactly one of the advantages of using ROS on top of a rolling release distribution such as conda-forge : ) ): ros/urdfdom_headers#59 .

At the time when ROS2 was first developed, urdfdom was forked in https://github.com/ros2/urdfdom, and it effectively became a ROS package again, distributed via rosdistro/bloom, and with the potential of being installed alongside with the system-installed urdfdom, see ros/urdfdom_headers#59 (comment) . At that point, @clalancette did a great job in removing the fork ros2/urdfdom, see ros/urdfdom#148 (comment) . However, at the moment two branches exist of urdfdom, the master branch from which the 1.x releases are tagged, and the ros2 from which the 2.x releases are tagged. Note that at moment in Debian/Ubuntu base packages only the 1.x tags are released (see https://packages.debian.org/source/unstable/urdfdom, fyi @j-rivero).

Having said that, however, in all this changes (unless I lost something) the only change that I can remember in the actual C++ code (beside CMake change) was to remove some methods that use TinyXML directly: ros/urdf#24 . If in ROS1 no one is actually using those methods, perhaps we can give it a shot of building all of ROS1 with urdfdom 2, as aside from that methods being removed the API/ABI of urdfdom 1 and 2 should be the same.

So the options are:

  • Try to rebuild all of ROS1 with urdfdom 2 . This would avoid us the need to maintain two variants for all the packages that depend on urdfdom in conda-forge. However, I am afraid this does not work, as otherwise the pin for urdfdom would have never been inserted.
  • Build any package that depends on urdfdom in conda-forge for both urdfdom 1 and urdfdom 2, as suggestd by @Tobias-Fischer . This would mean having to debug and fix funny CMake issues such as Build for multiple versions of urdfdom conda-forge/dartsim-feedstock#13 (comment), but that's life.

@traversaro
Copy link
Member

  • Try to rebuild all of ROS1 with urdfdom 2 . This would avoid us the need to maintain two variants for all the packages that depend on urdfdom in conda-forge. However, I am afraid this does not work, as otherwise the pin for urdfdom would have never been inserted.

Note that the problematic methods were deprecated in melodic and should have been removed in Noetic, see https://wiki.ros.org/melodic/Migration#urdf . While I am not sure if the removal of those methods actually happened in noetic, perhaps that pin to urdfdom 1 was a Melodic legacy?

@Tobias-Fischer
Copy link
Collaborator Author

I see. I tried compiling stuff with urdfdom2 and ran into some issues, which I then resolved with conda-forge/urdfdom-feedstock#16 and was able to build the downstream packages with urdfdom2 after applying that patch. So we should be good to rebuild the packages that depend on urdfdom once conda-forge/urdfdom-feedstock#16 is merged.

@traversaro
Copy link
Member

Great news, thanks! Unfortunatly urdfdom still uses the legacy manually written urdfdom-config.cmake.in that results in all those problems. I wanted to eventually fix this, but I was kind of waiting for urdfdom to migrate to TinyXML2 as that is easier to handle on the CMake level, but I am afraid this will not be happening anytime soon (see ros/urdfdom#99).

@traversaro
Copy link
Member

traversaro commented Jun 17, 2021

If in ROS1 no one is actually using those methods, perhaps we can give it a shot of building all of ROS1 with urdfdom 2, as aside from that methods being removed the API/ABI of urdfdom 1 and 2 should be the same.

W.r.t. to this, I actually did not remember correctly. In particular, urdfdom 1.0.4 has some symbols/classes (in particular URDFVersion, see https://github.com/ros/urdfdom/blob/1.0.4/urdf_parser/include/urdf_parser/urdf_parser.h#L61) that are not included in urdfdom 2.3.5 (see https://github.com/ros/urdfdom/blob/2.3.5/urdf_parser/include/urdf_parser/urdf_parser.h). So, in a sense 1.0.4 has more features then 2.3.5 . Fortunatly, I don't think any open source project is actually using the URDFVersion class (see https://github.com/search?q=URDFVersion&type=code). The URDFVersion class should be ported urdfdom 2.* in ros/urdfdom#146 .

@Tobias-Fischer
Copy link
Collaborator Author

This has been fixed in conda-forge/urdfdom-feedstock#16 & 0bc4efe & 30d6d14

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

No branches or pull requests

2 participants