Skip to content

Get 'test' working properly with acls and supplementary groups - #9149

Closed
jefft wants to merge 6 commits into
uutils:mainfrom
jefft:fix/8960-9147-test-work-with-acls-and-supplementary-groups
Closed

jefft wants to merge 6 commits into
uutils:mainfrom
jefft:fix/8960-9147-test-work-with-acls-and-supplementary-groups

Conversation

@jefft

@jefft jefft commented Nov 5, 2025

Copy link
Copy Markdown

Fixes #8960 and #9147. Delegate to libc euidaccess to avoid bugs (#9147) and respect ACLs (#8960).

Note, the tests are independent commits that can be applied first, where they should fail before the 'fix' commit is applied. Run the tests with:

cargo test --test tests test_permission_with_acl
cargo test --test tests test_permission_with_supplementary_group

Warning: code is partly Claude-generated as I'm not that familiar with Rust.

@codspeed

codspeed Bot commented Nov 5, 2025 •

Copy link
Copy Markdown

Merging this PR will degrade performance by 3.7%

⚡ 1 improved benchmark
❌ 1 regressed benchmark
✅ 279 untouched benchmarks
⏩ 38 skipped benchmarks1

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Mode Benchmark BASE HEAD Efficiency
⚡ Memory dd_copy_default 130 KB 124.8 KB +4.17%
❌ Memory du_wide_tree[(5000, 500)] 1.2 MB 1.3 MB -3.7%

Comparing jefft:fix/8960-9147-test-work-with-acls-and-supplementary-groups (e3d63d7) with main (4bfbbdd)

Open in CodSpeed

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. ↩

@github-actions

github-actions Bot commented Nov 5, 2025

Copy link
Copy Markdown

GNU testsuite comparison:

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

@sylvestre sylvestre left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

dunno if you saw it but many jobs are failing, this is why it hasn't been reviewed yet

@jefft
jefft force-pushed the fix/8960-9147-test-work-with-acls-and-supplementary-groups branch from d56a7d6 to e06edcb Compare December 28, 2025 04:53
@github-actions

Copy link
Copy Markdown

GNU testsuite comparison:

GNU test failed: tests/cp/cp-mv-enotsup-xattr. tests/cp/cp-mv-enotsup-xattr is passing on 'main'. Maybe you have to rebase?
Note: The gnu test tests/csplit/csplit-io-err was skipped on 'main' but is now failing.

@github-actions

Copy link
Copy Markdown

GNU testsuite comparison:

Congrats! The gnu test tests/tail/assert is no longer failing!

@jefft
jefft force-pushed the fix/8960-9147-test-work-with-acls-and-supplementary-groups branch 2 times, most recently from a7881d7 to dc664b3 Compare December 29, 2025 12:20
@jefft
jefft marked this pull request as draft December 30, 2025 07:31
@jefft
jefft force-pushed the fix/8960-9147-test-work-with-acls-and-supplementary-groups branch 2 times, most recently from 24c7c88 to 72aa2ee Compare January 17, 2026 04:57
@github-actions

Copy link
Copy Markdown

GNU testsuite comparison:

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

@jefft
jefft force-pushed the fix/8960-9147-test-work-with-acls-and-supplementary-groups branch 2 times, most recently from f8a5524 to 7fbfd9f Compare January 18, 2026 11:07
@github-actions

Copy link
Copy Markdown

GNU testsuite comparison:

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

@jefft
jefft force-pushed the fix/8960-9147-test-work-with-acls-and-supplementary-groups branch from 7fbfd9f to 7302d2d Compare January 18, 2026 13:30
@github-actions

Copy link
Copy Markdown

GNU testsuite comparison:

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

@jefft
jefft force-pushed the fix/8960-9147-test-work-with-acls-and-supplementary-groups branch from 7302d2d to a32b3fd Compare January 18, 2026 23:18
@github-actions

Copy link
Copy Markdown

GNU testsuite comparison:

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

@sylvestre

Copy link
Copy Markdown
Contributor

it is still marked as draft :)

jefft added 6 commits January 19, 2026 18:55
…ls#9147

Run with:

cargo test --test tests test_permission_with_supplementary_group
Run with:

cargo test --test tests test_permission_with_acl
Replace manual permission checking logic with platform-specific libc functions that properly handle supplementary groups and ACLs:

- Linux, Hurd, Redox, Cygwin, Solaris, Illumos: euidaccess()
- FreeBSD: eaccess()
- macOS, iOS, NetBSD, OpenBSD, DragonFly: faccessat() with AT_EACCESS
- Android: faccessat()
- Other OSes: fall back to libc:access()

These functions delegate permission checks to the kernel, which correctly evaluates all of the user's group memberships (both primary and supplementary), as well as ACLs and other advanced permission features.

Fixes uutils#8960
Fixes uutils#9147
@jefft
jefft force-pushed the fix/8960-9147-test-work-with-acls-and-supplementary-groups branch from a32b3fd to e3d63d7 Compare January 19, 2026 07:56
@jefft

jefft commented Jan 19, 2026

Copy link
Copy Markdown
Author

Soo many tests..

I think a few tests are flakey, e.g. whatever caused the last failure (https://github.com/uutils/coreutils/actions/runs/21120355090), and I've rebased my patch just to trigger a fresh run. I think I've addressed all the actual PR defects (failure on sudo-less environments, fmt and clippy violations). I'll un-draft this PR once tests are passing.

@github-actions

Copy link
Copy Markdown

GNU testsuite comparison:

Skip an intermittent issue tests/shuf/shuf-reservoir (fails in this run but passes in the 'main' branch)
Skip an intermittent issue tests/sort/sort-stale-thread-mem (fails in this run but passes in the 'main' branch)

@jefft
jefft marked this pull request as ready for review January 19, 2026 09:17
@jefft
jefft requested a review from sylvestre January 19, 2026 09:17
@jefft

jefft commented Jan 19, 2026

Copy link
Copy Markdown
Author

I think this PR is good to review now, despite 2 failing checks:
image

@anastygnome

Copy link
Copy Markdown
Contributor

Superseded by #14966

@anastygnome

Copy link
Copy Markdown
Contributor

@cakebaker can you close this as fixed in the last test PR

@cakebaker cakebaker closed this Oct 3, 2026
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.

The test command does not work properly

4 participants