Repository navigation
ls: show the ACL '+' on macOS - #15344
Conversation
On macOS the ACL isn't stored as a listed xattr, and most files carry some xattr (e.g. com.apple.provenance), so ls -l hard-coded is_acl_set = false. Check for an extended ACL with acl_get_file / acl_get_link_np instead and enable the check on macOS. Fixes uutils#15273
An xattr alone is no longer reported as an ACL on macOS, so expect false there, and add a macOS test that sets a real ACL with chmod +a.
Merging this PR will degrade performance by 1.49%
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ❌ | Simulation | factor_five_38_bit_primes |
1.6 s | 1.8 s | -8.82% |
| ⚡ | Simulation | factor_thirteen_39_bit_primes |
9.5 s | 8.9 s | +6.41% |
Tip
Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.
Comparing ozenyunus35:ls-macos-acl-plus (3642a86) with main (c1565db)
Footnotes
-
70 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. ↩
| @@ -258,19 +258,70 @@ pub fn apply_xattrs_fd_ignore_unsupported( | |||
| /// | |||
| /// `true` if the file has extended attributes (indicating an ACL), `false` otherwise. | |||
There was a problem hiding this comment.
this doc is no longer true on macOS, could you please update it?
There was a problem hiding this comment.
Updated, the doc now says macOS asks the ACL API and other platforms look at the xattrs.
| #[cfg(not(target_os = "macos"))] | ||
| assert!(has_acl(&file_path, true)); | ||
| #[cfg(target_os = "macos")] | ||
| assert!(!has_acl(&file_path, true)); |
There was a problem hiding this comment.
could be assert_eq!(has_acl(&file_path, true), cfg!(not(target_os = "macos")));, no?
| .args(["+a", "everyone allow read", &at.plus_as_string("with_acl")]) | ||
| .status(); | ||
| if !status.is_ok_and(|s| s.success()) { | ||
| println!("test skipped: chmod +a failed"); |
There was a problem hiding this comment.
macOS CI supports chmod +a, do we need the skip? i would rather see it fail
There was a problem hiding this comment.
Removed the skip, it now asserts that chmod +a succeeded.
|
Thanks for your PR |
2 participants
Fixes #15273
On macOS
ls -lhard-codedis_acl_set = false, becausehas_aclcounts xattrs and most macOS files have some (com.apple.provenance), while the ACL itself isn't a listed xattr.has_aclnow asks the ACL API on macOS:acl_get_file(oracl_get_link_npwithout dereference) withACL_TYPE_EXTENDED, and it reports an ACL only when the first entry exists. The check is enabled inlsfor macOS like on Linux/FreeBSD/NetBSD.This is a per-file check like the other platforms. It doesn't copy GNU's "stop checking after the first file without an ACL on this filesystem" behavior described in the issue.
Tests: a new macOS test in
test_ls.rsthat sets an ACL withchmod +aand checks+shows up only on that file (it fails without the change). Infsxattr.rsthe existingtest_file_has_aclnow expects an xattr alone not to count on macOS, plus a macOS test with a real ACL. Thelstests (184) and thefsxattrtests pass, and clippy and fmt are clean.I used an AI assistant to help draft parts of the code and tests; I reviewed the changes and ran the tests myself.