Skip to content

feat: optimize iter matching#7702

Merged
sylvestre merged 4 commits intouutils:mainfrom
nyurik:iters
Apr 15, 2025
Merged

feat: optimize iter matching#7702
sylvestre merged 4 commits intouutils:mainfrom
nyurik:iters

Conversation

@nyurik
Copy link
Contributor

@nyurik nyurik commented Apr 8, 2025 •
edited
Loading

  • There is no need to allocate a vector when matching a sequence of items.
  • This also removes an unused InvalidUserspec error

@github-actions
Copy link

github-actions bot commented Apr 8, 2025

GNU testsuite comparison:

Skip an intermittent issue tests/timeout/timeout (fails in this run but passes in the 'main' branch)

cakebaker
cakebaker reviewed Apr 9, 2025
View reviewed changes
src/uu/chroot/src/chroot.rs Outdated
/// error is returned.
fn parse_userspec(spec: &str) -> UResult<UserSpec> {
match &spec.splitn(2, ':').collect::<Vec<&str>>()[..] {
let mut parts = spec.splitn(2, ':');
Copy link
Contributor

@cakebaker cakebaker Apr 9, 2025

Choose a reason for hiding this comment

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

Wouldn't it be cleaner to use split_once?

Copy link
Contributor Author

@nyurik nyurik Apr 9, 2025

Choose a reason for hiding this comment

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

@cakebaker thx, good idea. The issue that I uncovered though is that the ChrootError::InvalidUserspec will never be produced -- all userspec values are valid?!

@nyurik nyurik force-pushed the iters branch 2 times, most recently from f255cc7 to 7c722ae Compare April 9, 2025 16:04
@github-actions
Copy link

github-actions bot commented Apr 9, 2025

GNU testsuite comparison:

Skip an intermittent issue tests/misc/stdbuf (fails in this run but passes in the 'main' branch)
Skipping an intermittent issue tests/timeout/timeout (passes in this run but fails in the 'main' branch)

sylvestre
sylvestre reviewed Apr 10, 2025
View reviewed changes
@github-actions
Copy link

github-actions bot commented Apr 10, 2025

GNU testsuite comparison:

Skip an intermittent issue tests/misc/stdbuf (fails in this run but passes in the 'main' branch)

@nyurik
Copy link
Contributor Author

nyurik commented Apr 10, 2025

@sylvestre @cakebaker so what's the proper solution for the bug uncovered by this PR, i.e. that ChrootError::InvalidUserspec is never triggered?

@nyurik nyurik force-pushed the iters branch 2 times, most recently from b771d98 to 27f41a5 Compare April 13, 2025 04:24
@github-actions
Copy link

github-actions bot commented Apr 13, 2025

GNU testsuite comparison:

Skipping an intermittent issue tests/timeout/timeout (passes in this run but fails in the 'main' branch)

@tertsdiepraam
Copy link
Collaborator

tertsdiepraam commented Apr 14, 2025

so what's the proper solution for the bug uncovered by this PR, i.e. that ChrootError::InvalidUserspec is never triggered?

I don't think I'm able to get anything but "invalid user" or "invalid group" from GNU here, so it's probably fine to let the group parsing later deal with it.

@github-actions
Copy link

github-actions bot commented Apr 14, 2025

GNU testsuite comparison:

Skipping an intermittent issue tests/misc/stdbuf (passes in this run but fails in the 'main' branch)

RenjiSann
RenjiSann approved these changes Apr 14, 2025
View reviewed changes
Copy link
Collaborator

@RenjiSann RenjiSann left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Looks good to me 👍

@github-actions
Copy link

github-actions bot commented Apr 14, 2025

GNU testsuite comparison:

Skip an intermittent issue tests/timeout/timeout (fails in this run but passes in the 'main' branch)
Congrats! The gnu test tests/misc/sleep is no longer failing!

nyurik and others added 4 commits April 15, 2025 12:43
@nyurik
feat: optimize iter matching
83ecc2a 
There is no need to allocate a vector when matching a sequence of items
@nyurik
use split_once
fd74977
@sylvestre @nyurik
fix a clippy warning
090949d
@nyurik
rm unused error
de4c2fd
@github-actions
Copy link

github-actions bot commented Apr 15, 2025

GNU testsuite comparison:

Skipping an intermittent issue tests/timeout/timeout (passes in this run but fails in the 'main' branch)

@sylvestre sylvestre merged commit 8a5a2ee into uutils:main Apr 15, 2025
69 of 70 checks passed
@nyurik nyurik deleted the iters branch April 15, 2025 17:56
@BrewTestBot BrewTestBot mentioned this pull request May 24, 2025
Merged
@moonfruit moonfruit mentioned this pull request May 26, 2025
Closed
nickorlow pushed a commit to nickorlow/coreutils that referenced this pull request Jul 17, 2025
@nyurik @nickorlow
Merge pull request uutils#7702 from nyurik/iters
75e75ba 
feat: optimize iter matching
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cakebaker cakebaker cakebaker left review comments

@sylvestre sylvestre sylvestre left review comments

@RenjiSann RenjiSann RenjiSann approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@nyurik @tertsdiepraam @cakebaker @sylvestre @RenjiSann