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

sort: version_cmp should keep the trailing 0 for comparison #4325

Closed
wants to merge 1 commit into from

Conversation

sylvestre
Copy link
Contributor

Fix misc/sort-version.sh

This is the demo that I did at fosdem
https://www.youtube.com/watch?v=90Q5N1qT7BQ

@tertsdiepraam
Copy link
Member

tertsdiepraam commented Feb 7, 2023

I wonder if this really addresses the core of the issue. I think the problem is with stable vs unstable sorting:

# Without --stable, 04 always comes before 4echo 04\n4 | sort --sort=version
04
4
❯ echo 4\n04 | sort --sort=version
04
4
# With --stable, the order is preservedecho 04\n4 | sort --sort=version --stable
4
04
❯ echo 4\n04 | sort --sort=version --stable
04
4

I think ls then uses an unstable sort, so that the it looks like it's considering the zeros to be significant. I wonder where this behaviour comes from because it does seem to always put 04 before 4 even with many elements. If the sort was truly unstable I'd expect more random results.

echo 4\n04\n4\n04\n04\n04\n4\n4\n04\n04 | sort --sort=version
04
04
04
04
04
04
4
4
4
4

@timvisee
Copy link

timvisee commented Feb 7, 2023

@tertsdiepraam That's weird. Doesn't that imply that 04 and 4 are both equal and non-equal depending on whether using stable sorting?

By the way, with an older GNU sort version 04 always comes first, so this did change some time ago:

$ echo 04\n4 | sort --sort=version --stable
04
4

$ sort --version
sort (GNU coreutils) 8.32
...

@tertsdiepraam
Copy link
Member

@timvisee That's right. I think this part of the source code is to blame:

https://github.com/coreutils/coreutils/blob/5450c7f8d33f7a9ee10a2700cad9ccc6cec3626e/src/sort.c#L2822-L2829

It seems to fall back on a default order if the specified sort compares equal and --stable is not passed. Not sure why they have this behaviour.

@sylvestre
Copy link
Contributor Author

And my patch doesn't even work ;)

-string start 5.04.0 end of str
 string start 5.5.0 end of str
 string start 5.6.0 end of str
 string start 5.7.0 end of str
 string start 5.8.0 end of str
 string start 5.9.0 end of str
+string start 5.04.0 end of str
 string start 5.10.0 end of str

@tertsdiepraam
Copy link
Member

Found the commit: coreutils/coreutils@d8047ae#diff-e0705db8518514c907d220d1879e28500a0fb802065a65d471ba9520235136ea

Seems like it was intended behaviour to ensure a total order. Funnily enough, it seems to have been introduced based on a bug report by @miDeb which he filed while working on uutils sort, so we brought this upon ourselves 😄

@sylvestre
Copy link
Contributor Author

ah ah, it would have been fun to share that in the talk at FOSDEM :)

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

Successfully merging this pull request may close these issues.

3 participants