| 2019-02-25 16:08:12 |
simonschmeisser |
bug |
|
|
added bug |
| 2019-02-25 16:22:30 |
simonschmeisser |
bug watch added |
|
https://github.com/ros/urdfdom_headers/issues/45 |
|
| 2019-02-25 16:22:30 |
simonschmeisser |
bug task added |
|
urdfdom-headers |
|
| 2019-02-25 23:21:53 |
Bug Watch Updater |
urdfdom-headers: status |
Unknown |
New |
|
| 2019-02-26 10:17:33 |
simonschmeisser |
bug task added |
|
urdfdom (Ubuntu) |
|
| 2019-02-26 10:22:36 |
simonschmeisser |
description |
In the effort to reduce boost dependency in urdfdom and urdfdom-headers [2] regressions wrt locale handling were introduced. Parsing of floating point values in URDF (a XML language) was now locale dependent, ie on European systems were the decimal separator is "," instead of "." parsing would now fail due to std::stod expecting "1,23" instead of "1.23" (no, we do not localize xml files, see also [3] for a more technical summary). The regressed versions were released as 1.0. Shortly after the release (some of) the regressions were detected [3] and fixes merged [3][4]. Unfortunately those fixed merges were not released until a long time later, too late for Ubuntu 18.04 by far and potentially even for 20.04. More regressions were noticed and fixed later on (disclaimer: by me) [5].
urdfdom is used mostly if not exclusively in the ROS community (including gazebo) which is a somewhat slow adopter of newer Ubuntu (and ros) releases but more and more people are now seeing these regressions in various places. To save more peoples time I propose to do a patch version update and pull urdfdom_headers version 1.0.3 as well as urdfdom version 1.0.3 (once released into debian). I argue against applying a patch because the diff between 1.0.0 and 1.0.3 is almost identical to a set of patches fixing the regressions. Updating to the upstream patch release will therefore improve tracing of the ubuntu downstream version to the upstream versioning.
Both urdfdom_headers and urdfdom now have an extensive set of unit test which in the case of urdfdom are also run in a European (ie Dutch) locale [6]. Together with the small and homogeneous user base, application mostly in research and the fact that most of the library is (for unknown reasons) almost header-only make me estimate that scope, potential and impact of regressions introduced by this upgrade would be extremely low.
1: https://github.com/ros/urdfdom/commit/3dc7ee812827cc69ffa457ef01fe7b9623096aed
2: https://github.com/ros/urdfdom_headers/commit/9d2b421f3fcbc2a32af40b99ebd9c2cb2d088fb9
3: https://github.com/ros/urdfdom_headers/pull/42
4: https://github.com/ros/urdfdom/pull/105
5: https://github.com/ros/urdfdom_headers/pull/47
6: https://github.com/ros/urdfdom/pull/115
See also the following upstream bug reports:
https://github.com/ros/urdfdom_headers/issues/45
https://github.com/ros/urdfdom/issues/119
and a few downstream bug reports to the ubuntu package:
https://github.com/ros-planning/moveit/issues/1333
https://github.com/ros/urdf/issues/21
https://github.com/ros-visualization/rviz/issues/1249
https://github.com/ros-visualization/rviz/issues/1151
https://github.com/ros-planning/moveit/issues/1050
https://github.com/ros-visualization/rviz/issues/1298 |
In the effort to reduce boost dependency in urdfdom and urdfdom-headers [2] regressions wrt locale handling were introduced. Parsing of floating point values in URDF (a XML language) was now locale dependent, ie on European systems were the decimal separator is "," instead of "." parsing would now fail due to std::stod expecting "1,23" instead of "1.23" (no, we do not localize xml files, see also [3] for a more technical summary). The regressed versions were released as 1.0. Shortly after the release (some of) the regressions were detected [3] and fixes merged [3][4]. Unfortunately those fixed merges were not released until a long time later, too late for Ubuntu 18.04 by far and potentially even for 20.04. More regressions were noticed and fixed later on (disclaimer: by me) [5].
urdfdom is used mostly if not exclusively in the ROS community (including gazebo) which is a somewhat slow adopter of newer Ubuntu (and ros) releases but more and more people are now seeing these regressions in various places. To save more peoples time I propose to do a patch version update and pull urdfdom_headers version 1.0.3 as well as urdfdom version 1.0.3 from debian sid. I argue against applying a patch because the diff between 1.0.0 and 1.0.3 is almost identical to a set of patches fixing the regressions. Updating to the upstream patch release will therefore improve tracing of the ubuntu downstream version to the upstream versioning.
Both urdfdom_headers and urdfdom now have an extensive set of unit test which in the case of urdfdom are also run in a European (ie Dutch) locale [6]. Together with the small and homogeneous user base, application mostly in research and the fact that most of the library is (for unknown reasons) almost header-only make me estimate that scope, potential and impact of regressions introduced by this upgrade would be extremely low.
1: https://github.com/ros/urdfdom/commit/3dc7ee812827cc69ffa457ef01fe7b9623096aed
2: https://github.com/ros/urdfdom_headers/commit/9d2b421f3fcbc2a32af40b99ebd9c2cb2d088fb9
3: https://github.com/ros/urdfdom_headers/pull/42
4: https://github.com/ros/urdfdom/pull/105
5: https://github.com/ros/urdfdom_headers/pull/47
6: https://github.com/ros/urdfdom/pull/115
See also the following upstream bug reports:
https://github.com/ros/urdfdom_headers/issues/45
https://github.com/ros/urdfdom/issues/119
and a few downstream bug reports to the ubuntu package:
https://github.com/ros-planning/moveit/issues/1333
https://github.com/ros/urdf/issues/21
https://github.com/ros-visualization/rviz/issues/1249
https://github.com/ros-visualization/rviz/issues/1151
https://github.com/ros-planning/moveit/issues/1050
https://github.com/ros-visualization/rviz/issues/1298 |
|
| 2019-02-26 12:11:05 |
Launchpad Janitor |
urdfdom (Ubuntu): status |
New |
Confirmed |
|
| 2019-02-26 12:11:05 |
Launchpad Janitor |
urdfdom-headers (Ubuntu): status |
New |
Confirmed |
|
| 2019-03-14 18:46:39 |
Jose Luis Rivero |
bug |
|
|
added subscriber Jose Luis Rivero |
| 2019-07-31 08:29:13 |
Matthias |
bug |
|
|
added subscriber Matthias |
| 2019-08-09 18:54:36 |
Jose Luis Rivero |
bug watch added |
|
https://github.com/ros/urdfdom_headers/issues/41 |
|
| 2019-08-09 18:55:32 |
Jose Luis Rivero |
bug |
|
|
added subscriber Ubuntu Sponsors Team |
| 2019-08-14 01:57:13 |
Mathew Hodson |
urdfdom-headers: status |
New |
Unknown |
|
| 2019-08-14 01:57:13 |
Mathew Hodson |
urdfdom-headers: remote watch |
github.com/ros/urdfdom_headers/issues #45 |
github.com/ros/urdfdom_headers/issues #41 |
|
| 2019-08-14 01:57:27 |
Mathew Hodson |
bug watch removed |
https://github.com/ros/urdfdom_headers/issues/45 |
|
|
| 2019-08-14 02:06:08 |
Mathew Hodson |
description |
In the effort to reduce boost dependency in urdfdom and urdfdom-headers [2] regressions wrt locale handling were introduced. Parsing of floating point values in URDF (a XML language) was now locale dependent, ie on European systems were the decimal separator is "," instead of "." parsing would now fail due to std::stod expecting "1,23" instead of "1.23" (no, we do not localize xml files, see also [3] for a more technical summary). The regressed versions were released as 1.0. Shortly after the release (some of) the regressions were detected [3] and fixes merged [3][4]. Unfortunately those fixed merges were not released until a long time later, too late for Ubuntu 18.04 by far and potentially even for 20.04. More regressions were noticed and fixed later on (disclaimer: by me) [5].
urdfdom is used mostly if not exclusively in the ROS community (including gazebo) which is a somewhat slow adopter of newer Ubuntu (and ros) releases but more and more people are now seeing these regressions in various places. To save more peoples time I propose to do a patch version update and pull urdfdom_headers version 1.0.3 as well as urdfdom version 1.0.3 from debian sid. I argue against applying a patch because the diff between 1.0.0 and 1.0.3 is almost identical to a set of patches fixing the regressions. Updating to the upstream patch release will therefore improve tracing of the ubuntu downstream version to the upstream versioning.
Both urdfdom_headers and urdfdom now have an extensive set of unit test which in the case of urdfdom are also run in a European (ie Dutch) locale [6]. Together with the small and homogeneous user base, application mostly in research and the fact that most of the library is (for unknown reasons) almost header-only make me estimate that scope, potential and impact of regressions introduced by this upgrade would be extremely low.
1: https://github.com/ros/urdfdom/commit/3dc7ee812827cc69ffa457ef01fe7b9623096aed
2: https://github.com/ros/urdfdom_headers/commit/9d2b421f3fcbc2a32af40b99ebd9c2cb2d088fb9
3: https://github.com/ros/urdfdom_headers/pull/42
4: https://github.com/ros/urdfdom/pull/105
5: https://github.com/ros/urdfdom_headers/pull/47
6: https://github.com/ros/urdfdom/pull/115
See also the following upstream bug reports:
https://github.com/ros/urdfdom_headers/issues/45
https://github.com/ros/urdfdom/issues/119
and a few downstream bug reports to the ubuntu package:
https://github.com/ros-planning/moveit/issues/1333
https://github.com/ros/urdf/issues/21
https://github.com/ros-visualization/rviz/issues/1249
https://github.com/ros-visualization/rviz/issues/1151
https://github.com/ros-planning/moveit/issues/1050
https://github.com/ros-visualization/rviz/issues/1298 |
[Impact]
* The parsing done by the code of urdfdom truncate floating values on systems that are not using the locales defined by default on English systems. The users of this system are getting wrong values which are specially sensible on variables with a range between 0 and 1.
* The URDF schema defines doubles in URDF as xs:double. Looking at the XSD document it says that the mantissa for a double is represented as a "decimal" number. And looking at the XSD document a decimal number is a "finite-length sequence of decimal digits (#x30-#x39) separated by a period as a decimal indicator". Thus, the locale should have no effect on how the document is parsed, and using std::stod is incorrect. This patch switches the Vector3 class to using the stream operator, which seems to ignore locale.
* urdfdom-headers is a key part for the largest open source robotics framework, ROS (Robot Operative System). So it potentially affects a big part of the robotics open source community. We (the devs and maintainers of ROS have been receiving many different issues or bugs coming from this problem, as Simon wrote in the initial post of this bug.
[Test Case]
* We have prepared a test case that demonstrate the problem and the fix. Patched packages for both urdfdom and urdfdom_headers are in this ppa:
https://launchpad.net/~j-rivero/+archive/ubuntu/urdfdom-headers-sru2/+packages
* The dockerfile here https://github.com/j-rivero/sru_urdfdom/blob/master/Dockerfile tries to demonstrate the following:
- Setup the test case, install packages
- Import locale test from the latest development of urdfdom repo
- Run tests, failed (demonstration of the bug)
- Setup the PPA, install new version of packages
- Run test, success (demonstration of the fix)
* This is the testing build of the dockerfile
https://build.osrfoundation.org/job/_urdfdom_sru/15/console
[Regression Potential]
* The change affected the result of parsed values from using stof/stod functions to use stringstream. Although the patch is not small, the same pattern is replied on every chunk of code changed. Standard use case of users setting up digits in the form of integer or double is unlikely to generate big regressions. Alternative use cases of users with weird values in the input would trigger an exception from the code. This is no different than the exceptions thrown by previous stof/stod funtions (std::invalid_argument or std::out_of_range).
* The bug was reported in Nov 2017 (https://github.com/ros/urdfdom_headers/issues/41) the ROS community and fixes landed in upstream repositories few weeks after. Many users from different countries have been using new versions or these patches since them, some of them are commenting/voting in this bug. Note that we were also playing with upstream test suite to build the test case and no regression was found in there.
[Other Info]
* Upstream releases new patched versions with the fix that I’ve included into Debian Sid and have been synced by Ubuntu starting from Disco: 1.0.3-1
* debdiffs for versions from the PPA:
- urdfdom debdiff:
https://gist.github.com/j-rivero/9983dddc6dbbea99cd3c39774f1d0498
- urdfdom-headers debdiff
https://gist.github.com/j-rivero/bc563620802d78129d775b2d5aee69ac
https://github.com/ros/urdfdom/commit/3dc7ee812827cc69ffa457ef01fe7b9623096aed
https://github.com/ros/urdfdom_headers/commit/9d2b421f3fcbc2a32af40b99ebd9c2cb2d088fb9
https://github.com/ros/urdfdom_headers/pull/42
https://github.com/ros/urdfdom/pull/105
https://github.com/ros/urdfdom_headers/pull/47
https://github.com/ros/urdfdom/pull/115
See also the following upstream bug reports:
https://github.com/ros/urdfdom_headers/issues/45
https://github.com/ros/urdfdom/issues/119
and a few downstream bug reports to the ubuntu package:
https://github.com/ros-planning/moveit/issues/1333
https://github.com/ros/urdf/issues/21
https://github.com/ros-visualization/rviz/issues/1249
https://github.com/ros-visualization/rviz/issues/1151
https://github.com/ros-planning/moveit/issues/1050
https://github.com/ros-visualization/rviz/issues/1298 |
|
| 2019-08-14 02:10:00 |
Mathew Hodson |
nominated for series |
|
Ubuntu Bionic |
|
| 2019-08-14 02:10:00 |
Mathew Hodson |
bug task added |
|
urdfdom-headers (Ubuntu Bionic) |
|
| 2019-08-14 02:10:00 |
Mathew Hodson |
bug task added |
|
urdfdom (Ubuntu Bionic) |
|
| 2019-08-14 02:10:24 |
Mathew Hodson |
urdfdom (Ubuntu): status |
Confirmed |
Fix Released |
|
| 2019-08-14 02:10:28 |
Mathew Hodson |
urdfdom-headers (Ubuntu): status |
Confirmed |
Fix Released |
|
| 2019-08-14 02:31:38 |
Mathew Hodson |
bug |
|
|
added subscriber Mathew Hodson |
| 2019-09-12 19:54:30 |
Julian Andres Klode |
removed subscriber Ubuntu Sponsors Team |
|
|
|
| 2019-09-16 23:08:02 |
Jose Luis Rivero |
attachment added |
|
urdfdom.debdiff https://bugs.launchpad.net/ubuntu/+source/urdfdom-headers/+bug/1817595/+attachment/5289101/+files/urdfdom.debdiff |
|
| 2019-09-16 23:08:32 |
Jose Luis Rivero |
attachment added |
|
urdfdom-headers.debdiff https://bugs.launchpad.net/ubuntu/+source/urdfdom-headers/+bug/1817595/+attachment/5289102/+files/urdfdom-headers.debdiff |
|
| 2019-09-16 23:08:54 |
Jose Luis Rivero |
bug |
|
|
added subscriber Ubuntu Sponsors Team |
| 2019-09-19 13:36:59 |
Launchpad Janitor |
urdfdom (Ubuntu Bionic): status |
New |
Confirmed |
|
| 2019-09-19 13:36:59 |
Launchpad Janitor |
urdfdom-headers (Ubuntu Bionic): status |
New |
Confirmed |
|
| 2019-09-19 13:37:22 |
Joachim Schleicher |
bug |
|
|
added subscriber Joachim Schleicher |
| 2019-10-08 19:25:14 |
Julian Andres Klode |
urdfdom (Ubuntu Bionic): status |
Confirmed |
Fix Committed |
|
| 2019-10-08 19:25:16 |
Julian Andres Klode |
urdfdom-headers (Ubuntu Bionic): status |
Confirmed |
Fix Committed |
|
| 2019-10-30 00:27:58 |
Chris Halse Rogers |
bug |
|
|
added subscriber Ubuntu Stable Release Updates Team |
| 2019-10-30 00:28:03 |
Chris Halse Rogers |
bug |
|
|
added subscriber SRU Verification |
| 2019-10-30 00:28:08 |
Chris Halse Rogers |
tags |
|
verification-needed verification-needed-bionic |
|
| 2019-10-30 03:00:46 |
Mathew Hodson |
removed subscriber Mathew Hodson |
|
|
|
| 2019-10-30 03:02:32 |
Mathew Hodson |
urdfdom (Ubuntu): importance |
Undecided |
Low |
|
| 2019-10-30 03:02:35 |
Mathew Hodson |
urdfdom (Ubuntu Bionic): importance |
Undecided |
Low |
|
| 2019-10-30 03:02:38 |
Mathew Hodson |
urdfdom-headers (Ubuntu): importance |
Undecided |
Low |
|
| 2019-10-30 03:02:40 |
Mathew Hodson |
urdfdom-headers (Ubuntu Bionic): importance |
Undecided |
Low |
|
| 2019-10-31 22:06:30 |
simonschmeisser |
tags |
verification-needed verification-needed-bionic |
verification-done-bionic |
|
| 2019-11-07 09:45:27 |
Łukasz Zemczak |
removed subscriber Ubuntu Stable Release Updates Team |
|
|
|
| 2019-11-07 09:55:32 |
Launchpad Janitor |
urdfdom (Ubuntu Bionic): status |
Fix Committed |
Fix Released |
|
| 2019-11-07 09:55:34 |
Launchpad Janitor |
urdfdom-headers (Ubuntu Bionic): status |
Fix Committed |
Fix Released |
|
| 2019-11-08 15:45:29 |
Martin Pecka |
bug |
|
|
added subscriber Martin Pecka |
| 2021-07-21 01:43:41 |
Bug Watch Updater |
urdfdom-headers: status |
Unknown |
Fix Released |
|
