`mkdir`: added `acl` permissions inheritance for subdirectories #6676

AnirbanHalder654322 · 2024-09-02T15:32:58Z

This will fix mkdir/p-acl gnu test

src/uu/mkdir/Cargo.toml

sylvestre · 2024-09-02T15:36:40Z

src/uu/mkdir/src/mkdir.rs

+/// Only default acl entries get inherited by objects under the path i.e. if child directories
+/// will have their permissions modified.
+#[cfg(not(windows))]
+fn get_acl_perm_bits(path: &Path) -> u32 {


we have this function:

coreutils/src/uucore/src/lib/features/fsxattr.rs

Line 41 in 8cac375

pub fn retrieve_xattrs<P: AsRef<Path>>(source: P) -> std::io::Result<HashMap<OsString, Vec<u8>>> {

maybe use it ?

I tried it but i couldn't figure out how to correctly and reliably interpret the Vec<u8> value , the key is system.posix_acl_default for a file when i do setfacl -d -m group::rmx test_file so that's fine. I just need to reliably parse the byte sequences to extract the permission bits.

I will try to look through the fsxattr source code in https://github.com/torvalds/linux/blob/master/fs/xattr.c to check how they interpret things. I would definitely appreciate some help or some other resources you can point me to.

ok, makes sense
could you please move the function into fsxattr ?

ok, makes sense could you please move the function into fsxattr ?

Okay, will do that.

github-actions · 2024-09-02T16:02:11Z

GNU testsuite comparison:

Congrats! The gnu test tests/mkdir/p-acl is no longer failing!

github-actions · 2024-09-04T15:19:07Z

GNU testsuite comparison:

Skip an intermittent issue tests/tail/inotify-dir-recreate (fails in this run but passes in the 'main' branch)
Congrats! The gnu test tests/mkdir/p-acl is no longer failing!
Skipping an intermittent issue tests/rm/rm1 (passes in this run but fails in the 'main' branch)

src/uu/mkdir/src/mkdir.rs

github-actions · 2024-09-05T13:10:55Z

GNU testsuite comparison:

Congrats! The gnu test tests/mkdir/p-acl is no longer failing!
Skipping an intermittent issue tests/rm/rm1 (passes in this run but fails in the 'main' branch)

AnirbanHalder654322 · 2024-09-05T14:24:24Z

Changes:

Moved get_acl_perm_bits_from_xattr to uucore::fsxattr.
Disabled the test for for macos since fsxattr is not fully compatible with macos.
Added a todo for making a macos compatible get_acl_perm_bits_from_xattr function and thus extend the feature for it.

github-actions · 2024-09-05T14:54:29Z

GNU testsuite comparison:

Congrats! The gnu test tests/mkdir/p-acl is no longer failing!
Skipping an intermittent issue tests/rm/rm1 (passes in this run but fails in the 'main' branch)

sylvestre · 2024-09-05T16:21:15Z

tests/by-util/test_mkdir.rs

+
+    let mut map: HashMap<OsString, Vec<u8>> = HashMap::new();
+    let xattr_val: Vec<u8> = vec![
+        2, 0, 0, 0, 1, 0, 7, 0, 255, 255, 255, 255, 4, 0, 7, 0, 255, 255, 255, 255, 32, 0, 5, 0,


please add a comment explaining these values and how to get them

AnirbanHalder654322 · 2024-09-06T18:59:18Z

Waiting on a fix for this rust-lang/rust-bindgen#2926

github-actions · 2024-09-07T12:14:33Z

GNU testsuite comparison:

Congrats! The gnu test tests/mkdir/p-acl is no longer failing!

sylvestre · 2024-09-07T13:37:56Z

you should disable it on android too:


[2024-09-07 12:05:22] --- TRY 3 STDOUT:        coreutils::tests test_mkdir::test_mkdir_acl ---
[2024-09-07 12:05:22] 
[2024-09-07 12:05:22] running 1 test
[2024-09-07 12:05:22] mkdir: /data/data/com.termux/files/usr/tmp/.tmpwg3OjG/a
[2024-09-07 12:05:22] test test_mkdir::test_mkdir_acl ... FAILED
[2024-09-07 12:05:22] 
[2024-09-07 12:05:22] failures:
[2024-09-07 12:05:22] 
[2024-09-07 12:05:22] failures:
[2024-09-07 12:05:22]     test_mkdir::test_mkdir_acl
[2024-09-07 12:05:22] 
[2024-09-07 12:05:22] test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 3270 filtered out; finished in 0.00s
[2024-09-07 12:05:22] 
[2024-09-07 12:05:22] 
[2024-09-07 12:05:22] --- TRY 3 STDERR:        coreutils::tests test_mkdir::test_mkdir_acl ---
[2024-09-07 12:05:22] thread 'test_mkdir::test_mkdir_acl' panicked at tests/by-util/test_mkdir.rs:254:54:
[2024-09-07 12:05:22] called `Result::unwrap()` on an `Err` value: Os { code: 95, kind: Uncategorized, message: "Operation not supported on transport endpoint" }

and maybe freebsd:

  --- TRY 2 STDERR:        coreutils::tests test_mkdir::test_mkdir_acl ---
  thread 'test_mkdir::test_mkdir_acl' panicked at tests/by-util/test_mkdir.rs:254:54:
  called `Result::unwrap()` on an `Err` value: Os { code: 1, kind: PermissionDenied, message: "Operation not permitted" }
  stack backtrace:
     0: rust_begin_unwind
               at /rustc/eeb90cda1969383f56a2637cbd3037bdf598841c/library/std/src/panicking.rs:665:5
     1: core::panicking::panic_fmt
               at /rustc/eeb90cda1969383f56a2637cbd3037bdf598841c/library/core/src/panicking.rs:74:14
     2: core::result::unwrap_failed
               at /rustc/eeb90cda1969383f56a2637cbd3037bdf598841c/library/core/src/result.rs:1679:5
     3: core::result::Result<T,E>::unwrap
               at /rustc/eeb90cda1969383f56a2637cbd3037bdf598841c/library/core/src/result.rs:1102:23
     4: tests::test_mkdir::test_mkdir_acl
               at ./tests/by-util/test_mkdir.rs:254:5
     5: tests::test_mkdir::test_mkdir_acl::{{closure}}
               at ./tests/by-util/test_mkdir.rs:214:20
     6: core::ops::function::FnOnce::call_once
               at /rustc/eeb90cda1969383f56a2637cbd3037bdf598841c/library/core/src/ops/function.rs:250:5
     7: core::ops::function::FnOnce::call_once
               at /rustc/eeb90cda1969383f56a2637cbd3037bdf598841c/library/core/src/ops/function.rs:250:5
  note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

AnirbanHalder654322 · 2024-09-07T17:47:03Z

Changes since last push

Disabled test for android
Disabled test for freebsd

Freebsd acl entries have differences, which would be better implemented by someone with a bsd system as they could check if the bindgen is creating wrong entries.

github-actions · 2024-09-07T18:12:43Z

GNU testsuite comparison:

Congrats! The gnu test tests/mkdir/p-acl is no longer failing!

sylvestre · 2024-09-08T07:53:25Z

src/uu/mkdir/src/mkdir.rs

+
+            // TODO: Make this macos and freebsd compatible by creating a function to get permission bits from
+            // acl in extended attributes
+            #[cfg(not(any(target_os = "windows", target_os = "macos", target_os = "freebsd")))]


what about ?

it makes it clear that the call is always chmod and we just have different mode for each platform?

#[cfg(not(any(target_os = "windows", target_os = "macos", target_os = "freebsd")))] Comment let new_mode = if is_parent { (!mode::get_umask() & 0o777) | 0o300 } else { mode }; #[cfg(not(any(target_os = "windows", target_os = "macos", target_os = "freebsd")))] let new_mode = if !path_exists { let acl_perm_bits = uucore::fsxattr::get_acl_perm_bits_from_xattr(path); new_mode | acl_perm_bits } else { new_mode }; #[cfg(windows)] let new_mode = mode; chmod(path, new_mode)?; Ok(())

Yeah that feels more deliberate.

We replace the large os exceptions with this

#[cfg(all(unix,not(target_os="linux")))] Comment let new_mode = if is_parent { (!mode::get_umask() & 0o777) | 0o300 } else { mode }; #[cfg(all(unix,target_os="linux"))] let new_mode = if !path_exists { let acl_perm_bits = uucore::fsxattr::get_acl_perm_bits_from_xattr(path); new_mode | acl_perm_bits } else { new_mode }; #[cfg(windows)] let new_mode = mode; chmod(path, new_mode)?;

From the earlier CI checks , i don't think android xattr works either, it does seem its because the tag values i am using is different for android so the test kept failing. There seems to be some differences in every platform, which would require reading into their source code. Ideally we could have used the "exacl" crate but it wouldn't work on most systems as most distros ship without libacl.

Edit 1: If this code snippet looks okay, then please tell me, i will commit it. It feels like kinda bad to make a linux specific change.

Edit 2: If we merge this , we should create a tracking issue to extend acl support to other platforms. And then work through it, i guess.

github-actions · 2024-09-10T14:22:05Z

GNU testsuite comparison:

Congrats! The gnu test tests/mkdir/p-acl is no longer failing!

sylvestre · 2024-09-10T19:58:36Z

fails on freebsd:


2024-09-10T14:01:57.0244050Z 
2024-09-10T14:01:57.0244197Z failures:
2024-09-10T14:01:57.0244630Z     features::fsxattr::tests::test_get_perm_bits_from_xattrs
2024-09-10T14:01:57.0245085Z 
2024-09-10T14:01:57.0245607Z test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 128 filtered out; finished in 0.03s
2024-09-10T14:01:57.0246330Z 
2024-09-10T14:01:57.0246485Z �[0m
2024-09-10T14:01:57.0247767Z �[35;1m--- �[0m�[35;1mTRY�[0m �[35;1m2�[0m �[35;1mSTDERR:        �[0m�[35;1muucore�[0m �[36mfeatures::fsxattr::tests�[0m�[36m::�[0m�[34;1mtest_get_perm_bits_from_xattrs�[0m�[35;1m ---�[0m
2024-09-10T14:01:57.0249718Z �[0m�[31;1mthread 'features::fsxattr::tests::test_get_perm_bits_from_xattrs' panicked at src/uucore/src/lib/features/fsxattr.rs:229:68:�[0m
2024-09-10T14:01:57.0251594Z �[31;1mcalled `Result::unwrap()` on an `Err` value: Os { code: 1, kind: PermissionDenied, message: "Operation not permitted" }�[0m
2024-09-10T14:01:57.0252587Z stack backtrace:
2024-09-10T14:01:57.0252922Z    0: rust_begin_unwind
2024-09-10T14:01:57.0253650Z              at /rustc/eeb90cda1969383f56a2637cbd3037bdf598841c/library/std/src/panicking.rs:665:5
2024-09-10T14:01:57.0254631Z    1: core::panicking::panic_fmt
2024-09-10T14:01:57.0255395Z              at /rustc/eeb90cda1969383f56a2637cbd3037bdf598841c/library/core/src/panicking.rs:74:14
2024-09-10T14:01:57.0256220Z    2: core::result::unwrap_failed
2024-09-10T14:01:57.0256986Z              at /rustc/eeb90cda1969383f56a2637cbd3037bdf598841c/library/core/src/result.rs:1679:5
2024-09-10T14:01:57.0257795Z    3: core::result::Result<T,E>::unwrap
2024-09-10T14:01:57.0258598Z              at /rustc/eeb90cda1969383f56a2637cbd3037bdf598841c/library/core/src/result.rs:1102:23
2024-09-10T14:01:57.0259570Z    4: uucore::features::fsxattr::tests::test_get_perm_bits_from_xattrs
2024-09-10T14:01:57.0260258Z              at ./src/lib/features/fsxattr.rs:229:9
2024-09-10T14:01:57.0261024Z    5: uucore::features::fsxattr::tests::test_get_perm_bits_from_xattrs::{{closure}}
2024-09-10T14:01:57.0262040Z              at ./src/lib/features/fsxattr.rs:192:40
2024-09-10T14:01:57.0262603Z    6: core::ops::function::FnOnce::call_once
2024-09-10T14:01:57.0263444Z              at /rustc/eeb90cda1969383f56a2637cbd3037bdf598841c/library/core/src/ops/function.rs:250:5
2024-09-10T14:01:57.0264473Z    7: core::ops::function::FnOnce::call_once
2024-09-10T14:01:57.0265320Z              at /rustc/eeb90cda1969383f56a2637cbd3037bdf598841c/library/core/src/ops/function.rs:250:5
2024-09-10T14:01:57.0266438Z note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

github-actions · 2024-09-11T08:36:32Z

GNU testsuite comparison:

Skip an intermittent issue tests/tail/inotify-dir-recreate (fails in this run but passes in the 'main' branch)
Congrats! The gnu test tests/mkdir/p-acl is no longer failing!

github-actions · 2024-09-11T09:08:15Z

GNU testsuite comparison:

Congrats! The gnu test tests/mkdir/p-acl is no longer failing!

sylvestre · 2024-09-11T19:38:34Z

well done :)

mkdir: added acl permissions inheritance for subdirectories

a84b0f1

sylvestre reviewed Sep 2, 2024

View reviewed changes

src/uu/mkdir/Cargo.toml Outdated Show resolved Hide resolved

sylvestre reviewed Sep 2, 2024

View reviewed changes

AnirbanHalder654322 force-pushed the mkdir_acl_test branch from 6fa394b to 50bbac0 Compare September 4, 2024 14:46

Use xattr entry to calculate perm bits

78cc1c3

AnirbanHalder654322 force-pushed the mkdir_acl_test branch from 50bbac0 to 78cc1c3 Compare September 4, 2024 14:51

sylvestre reviewed Sep 5, 2024

View reviewed changes

src/uu/mkdir/src/mkdir.rs Outdated Show resolved Hide resolved

Move calc_acl_bits_from_xattr function to uucore::fsxattr

6196422

AnirbanHalder654322 force-pushed the mkdir_acl_test branch 2 times, most recently from d651140 to de4a0cb Compare September 5, 2024 12:43

AnirbanHalder654322 force-pushed the mkdir_acl_test branch 2 times, most recently from e2642de to 04befe4 Compare September 5, 2024 14:22

disable test for macos as fsxattr doesn't support macos yet

6ed128a

AnirbanHalder654322 force-pushed the mkdir_acl_test branch from 04befe4 to 6ed128a Compare September 5, 2024 14:27

sylvestre reviewed Sep 5, 2024

View reviewed changes

Added comments explaining xattr_values

cbde96a

AnirbanHalder654322 force-pushed the mkdir_acl_test branch from ca7f284 to cbde96a Compare September 7, 2024 11:47

Disabled test for both android and freebsd

c6a9e66

AnirbanHalder654322 force-pushed the mkdir_acl_test branch from 3e84528 to c6a9e66 Compare September 7, 2024 17:46

sylvestre reviewed Sep 8, 2024

View reviewed changes

Small refactor

2f2680c

AnirbanHalder654322 force-pushed the mkdir_acl_test branch from 7885343 to 2f2680c Compare September 10, 2024 13:26

AnirbanHalder654322 marked this pull request as draft September 10, 2024 13:27

AnirbanHalder654322 added 2 commits September 10, 2024 19:03

Merge branch 'main' into mkdir_acl_test

5fdf408

Update mkdir.rs

26ae14c

AnirbanHalder654322 marked this pull request as ready for review September 10, 2024 13:39

AnirbanHalder654322 force-pushed the mkdir_acl_test branch 2 times, most recently from cbef610 to c5d833c Compare September 11, 2024 08:09

fix clippy and disable test for freebsd

8415b88

AnirbanHalder654322 force-pushed the mkdir_acl_test branch from c5d833c to 8415b88 Compare September 11, 2024 08:41

sylvestre merged commit 7430856 into uutils:main Sep 11, 2024
68 checks passed

AnirbanHalder654322 deleted the mkdir_acl_test branch September 12, 2024 10:35

BrewTestBot mentioned this pull request Nov 16, 2024

uutils-coreutils 0.0.28 Homebrew/homebrew-core#197947

Merged

moonfruit mentioned this pull request Nov 22, 2024

uutils-selected 0.0.28 moonfruit/homebrew-tap#178

Closed

sylvestre mentioned this pull request Dec 2, 2024

mkdir -p: when an acl is found on a directory, don't set the umask #5839

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

`mkdir`: added `acl` permissions inheritance for subdirectories #6676

`mkdir`: added `acl` permissions inheritance for subdirectories #6676

AnirbanHalder654322 commented Sep 2, 2024

sylvestre Sep 2, 2024

AnirbanHalder654322 Sep 2, 2024 •

edited

Loading

sylvestre Sep 4, 2024

AnirbanHalder654322 Sep 4, 2024

github-actions bot commented Sep 2, 2024

github-actions bot commented Sep 4, 2024

github-actions bot commented Sep 5, 2024

AnirbanHalder654322 commented Sep 5, 2024 •

edited

Loading

github-actions bot commented Sep 5, 2024

sylvestre Sep 5, 2024

AnirbanHalder654322 commented Sep 6, 2024

github-actions bot commented Sep 7, 2024

sylvestre commented Sep 7, 2024

AnirbanHalder654322 commented Sep 7, 2024

github-actions bot commented Sep 7, 2024

sylvestre Sep 8, 2024 •

edited

Loading

AnirbanHalder654322 Sep 8, 2024 •

edited

Loading

github-actions bot commented Sep 10, 2024

sylvestre commented Sep 10, 2024

github-actions bot commented Sep 11, 2024

github-actions bot commented Sep 11, 2024

sylvestre commented Sep 11, 2024

mkdir: added acl permissions inheritance for subdirectories #6676

mkdir: added acl permissions inheritance for subdirectories #6676

Conversation

AnirbanHalder654322 commented Sep 2, 2024

sylvestre Sep 2, 2024

Choose a reason for hiding this comment

AnirbanHalder654322 Sep 2, 2024 • edited Loading

Choose a reason for hiding this comment

sylvestre Sep 4, 2024

Choose a reason for hiding this comment

AnirbanHalder654322 Sep 4, 2024

Choose a reason for hiding this comment

github-actions bot commented Sep 2, 2024

github-actions bot commented Sep 4, 2024

github-actions bot commented Sep 5, 2024

AnirbanHalder654322 commented Sep 5, 2024 • edited Loading

github-actions bot commented Sep 5, 2024

sylvestre Sep 5, 2024

Choose a reason for hiding this comment

AnirbanHalder654322 commented Sep 6, 2024

github-actions bot commented Sep 7, 2024

sylvestre commented Sep 7, 2024

AnirbanHalder654322 commented Sep 7, 2024

github-actions bot commented Sep 7, 2024

sylvestre Sep 8, 2024 • edited Loading

Choose a reason for hiding this comment

AnirbanHalder654322 Sep 8, 2024 • edited Loading

Choose a reason for hiding this comment

github-actions bot commented Sep 10, 2024

sylvestre commented Sep 10, 2024

github-actions bot commented Sep 11, 2024

github-actions bot commented Sep 11, 2024

sylvestre commented Sep 11, 2024

`mkdir`: added `acl` permissions inheritance for subdirectories #6676

`mkdir`: added `acl` permissions inheritance for subdirectories #6676

AnirbanHalder654322 Sep 2, 2024 •

edited

Loading

AnirbanHalder654322 commented Sep 5, 2024 •

edited

Loading

sylvestre Sep 8, 2024 •

edited

Loading

AnirbanHalder654322 Sep 8, 2024 •

edited

Loading