Skip to content

chmod/chown: symlink cycles detection#11805

Open
nikolalukovic wants to merge 2 commits intouutils:mainfrom
nikolalukovic:fix/11614
Open

chmod/chown: symlink cycles detection#11805
nikolalukovic wants to merge 2 commits intouutils:mainfrom
nikolalukovic:fix/11614

Conversation

@nikolalukovic
Copy link
Copy Markdown
Contributor

Attempts to fix: #11614

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Apr 14, 2026

GNU testsuite comparison:

Skip an intermittent issue tests/date/date-locale-hour (fails in this run but passes in the 'main' branch)
Skip an intermittent issue tests/tail/symlink (fails in this run but passes in the 'main' branch)
Congrats! The gnu test tests/misc/io-errors is no longer failing!
Congrats! The gnu test tests/seq/seq-epipe is no longer failing!
Note: The gnu test tests/tail/tail-n0f is now being skipped but was previously passing.

@nikolalukovic nikolalukovic force-pushed the fix/11614 branch 3 times, most recently from e17c9e9 to 1990e91 Compare April 14, 2026 11:49
@nikolalukovic nikolalukovic marked this pull request as ready for review April 14, 2026 12:12
@nikolalukovic
Copy link
Copy Markdown
Contributor Author

nikolalukovic commented Apr 14, 2026

Failing tests are unrelated to these changes.
All tests pass now.

@nikolalukovic nikolalukovic force-pushed the fix/11614 branch 2 times, most recently from 168aac1 to 16ecfdd Compare April 14, 2026 13:42
@nikolalukovic
Copy link
Copy Markdown
Contributor Author

@sylvestre can you please take a look?

Comment thread src/uu/chmod/src/chmod.rs
}
if self.recursive {
r = self.walk_dir_with_context(file, true).and(r);
let mut ancestors = HashSet::new();
Copy link
Copy Markdown

@blakshya blakshya Apr 22, 2026

Choose a reason for hiding this comment

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

Hi,
I'm new here and found this PR when looking at how first issues are handled. Please correct me if i'm missing something.

Looking at GNU implementation, they push down the onus of cycle detection to a generic read function and only reference a cycle detection flag. That looks modular enough to not pass a new hash set everytime?

How would this be extended extended to other places when required?

@nikolalukovic
Copy link
Copy Markdown
Contributor Author

@sylvestre bump

@nikolalukovic
Copy link
Copy Markdown
Contributor Author

bump² @sylvestre

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.

chmod and chown do not detect symlink cycles

2 participants