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

ign command version comparison needs natural sort order #48

Closed
peci1 opened this issue Apr 5, 2021 · 3 comments · Fixed by #50
Closed

ign command version comparison needs natural sort order #48

peci1 opened this issue Apr 5, 2021 · 3 comments · Fixed by #50
Assignees
Labels
bug Something isn't working

Comments

@peci1
Copy link

peci1 commented Apr 5, 2021

Environment

  • OS Version: Ubuntu 18.04
  • Source or binary build? Binary 1.1.0

Description

  • Expected behavior: ign sdf launches the latest installed sdformat command
  • Actual behavior: It launches an older one.

Steps to reproduce

  1. install libsdformat8-dev
  2. install libsdformat10-dev
  3. take a SDF 1.7 model and pass it to ign sdf --check.
  4. The check fails saying that SDF 1.7 is too new for this tool to work with. That points at sdformat 8 being used.

I bet the problem lies in

https://github.com/ignitionrobotics/ign-tools/blob/d32853a9b4ebf21c3421f7e903e51a9c45e15849/src/ign.in#L145

which looks like C-order rather than natural order. But as my Ruby knowledge is limited, I don't know how to fix it correctly.

My system reports these version of sdformat:

8.9.1
10.3.0
@peci1 peci1 added the bug Something isn't working label Apr 5, 2021
@osrf-triage osrf-triage added this to Inbox in Core development Apr 5, 2021
@chapulina chapulina moved this from Inbox to To do in Core development Apr 12, 2021
@scpeters
Copy link
Member

scpeters commented May 1, 2021

we can copy some of the ruby code from homebrew's version.rb file to handle version parsing and sorting

or we could try to add swig bindings for the ignition::math::SemanticVersion class

@chapulina
Copy link
Contributor

add swig bindings for the ignition::math::SemanticVersion class

That's part of gazebosim/gz-math#101

Note that it would add a dependency to ign-math, and this may cause a circular dependency if we ever decide to add ign-tools to ign-math as part of gazebosim/gz-math#198.

@caguero
Copy link
Contributor

caguero commented May 3, 2021

See pull request #50 .

@adlarkin adlarkin mentioned this issue May 3, 2021
7 tasks
Core development automation moved this from To do to Done May 6, 2021
@j-rivero j-rivero removed this from Done in Core development May 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants