⚠ This page is served via a proxy. Original site: https://github.com
This service does not collect credentials or authentication data.
Skip to content

sort: gnu coreutils compatibility (sort.pl) #9862

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.

Sign up for GitHub

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

Jump to bottom
Open
mattsu2020 wants to merge 2 commits into
base: main
Choose a base branch
from
Open

sort: gnu coreutils compatibility (sort.pl) #9862

mattsu2020 wants to merge 2 commits into from
+39 −9

Conversation

@mattsu2020
Copy link
Contributor

@mattsu2020 mattsu2020 commented Dec 26, 2025

We will make the necessary changes to ensure the GNU coreutils tests pass.

@github-actions
Copy link

github-actions bot commented Dec 26, 2025

GNU testsuite comparison:

Congrats! The gnu test tests/sort/sort is no longer failing!

sylvestre
sylvestre reviewed Dec 26, 2025
View reviewed changes
@sylvestre
Copy link
Contributor

sylvestre commented Dec 26, 2025

it regressed a test

ChrisDryden
ChrisDryden reviewed Dec 26, 2025
View reviewed changes
tests/by-util/test_sort.rs
fn test_keys_invalid_field() {
new_ucmd!()
.args(&["-k", "1."])
.fails()
Copy link
Collaborator

@ChrisDryden ChrisDryden Dec 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd love to add .matches_gnu() to these: #9660

Its harder to review manually to validate that these tests are made in a way that matches the original.

@ChrisDryden ChrisDryden mentioned this pull request Dec 26, 2025
Open
@github-actions
Copy link

github-actions bot commented Dec 27, 2025

GNU testsuite comparison:

Congrats! The gnu test tests/sort/sort is no longer failing!

1 similar comment
@github-actions
Copy link

github-actions bot commented Dec 27, 2025

GNU testsuite comparison:

Congrats! The gnu test tests/sort/sort is no longer failing!

@sylvestre
Copy link
Contributor

sylvestre commented Dec 27, 2025

Some jobs are failing

@github-actions
Copy link

github-actions bot commented Dec 27, 2025

GNU testsuite comparison:

Congrats! The gnu test tests/sort/sort is no longer failing!

@github-actions
Copy link

github-actions bot commented Dec 28, 2025

GNU testsuite comparison:

Skipping an intermittent issue tests/misc/tee (passes in this run but fails in the 'main' branch)
Congrats! The gnu test tests/sort/sort is no longer failing!
Congrats! The gnu test tests/tty/tty-eof is no longer failing!

@mattsu2020 mattsu2020 requested a review from sylvestre January 5, 2026 06:32
@sylvestre
Copy link
Contributor

sylvestre commented Jan 5, 2026

this huge pr is hard to review :(
would it be possible to split a bit ? thanks

@mattsu2020 mattsu2020 mentioned this pull request Jan 5, 2026
Closed
@mattsu2020
Copy link
Contributor Author

mattsu2020 commented Jan 5, 2026 •
edited
Loading

this huge pr is hard to review :( would it be possible to split a bit ? thanks

split the PR.
#10066
this PR is Unnecessary

@mattsu2020 mattsu2020 mentioned this pull request Jan 5, 2026
Merged
@mattsu2020
Copy link
Contributor Author

mattsu2020 commented Jan 5, 2026

split
#10068

@github-actions
Copy link

github-actions bot commented Jan 5, 2026

GNU testsuite comparison:

Congrats! The gnu test tests/sort/sort is no longer failing!

@github-actions
Copy link

github-actions bot commented Jan 14, 2026

GNU testsuite comparison:

Skipping an intermittent issue tests/tty/tty-eof (passes in this run but fails in the 'main' branch)
Congrats! The gnu test tests/sort/sort is no longer failing!

@mattsu2020
fix: allow duplicate -o options for same output file in sort
c09ee08 
Modify sort command to permit multiple -o/--output flags specifying the same file,
matching GNU sort behavior. Previously, any multiple -o flags would error, but now
only differing output files are rejected. Added test case for duplicate outputs.
@mattsu2020
fix(sort): fix case-insensitive sorting to order punctuation after le…
a209604 
…tters

Changed ascii_case_insensitive_cmp to fold characters to uppercase instead of lowercase, ensuring that in ASCII case-insensitive sorting, letters are ordered before punctuation. Added tests to verify the correct behavior.
@codspeed-hq
Copy link

codspeed-hq bot commented Jan 16, 2026 •
edited
Loading

CodSpeed Performance Report

Merging this PR will improve performance by 8.49%

Comparing mattsu2020:sort_sort.pl (a209604) with main (5fd3459)

Summary

⚡ 2 improved benchmarks
✅ 280 untouched benchmarks
⏩ 38 skipped benchmarks1

Performance Changes

Mode Benchmark BASE HEAD Efficiency
⚡ Memory du_deep_tree[(100, 3)] 155.8 KB 143.6 KB +8.49%
⚡ Memory numfmt_large_numbers_si[10000] 4.9 MB 4.7 MB +4.01%

Footnotes

  1. 38 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ChrisDryden ChrisDryden ChrisDryden left review comments

@sylvestre sylvestre Awaiting requested review from sylvestre

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@mattsu2020 @sylvestre @ChrisDryden