Merge pull request #5835 from sylvestre/mv-acl

mv: preserve the xattr

.vscode/cspell.dictionaries/jargon.wordlist.txt

@@ -42,6 +42,7 @@ fileio
 filesystem
 filesystems
 flamegraph
+fsxattr
 fullblock
 getfacl
 gibi
@@ -133,6 +134,7 @@ urand
 whitespace
 wordlist
 wordlists
+xattrs
 
 # * abbreviations
 consts

Cargo.lock

@@ -2608,7 +2608,6 @@ dependencies = [
  "unicode-width",
  "uucore",
  "uutils_term_grid",
- "xattr",
 ]
 
 [[package]]
@@ -3209,6 +3208,7 @@ dependencies = [
  "wild",
  "winapi-util",
  "windows-sys 0.48.0",
+ "xattr",
  "z85",
 ]
 

src/uu/ls/Cargo.toml

@@ -27,6 +27,7 @@ uucore = { workspace = true, features = [
   "colors",
   "entries",
   "fs",
+  "fsxattr",
   "quoting-style",
   "version-cmp",
 ] }
@@ -34,9 +35,6 @@ once_cell = { workspace = true }
 selinux = { workspace = true, optional = true }
 hostname = { workspace = true }
 
-[target.'cfg(unix)'.dependencies]
-xattr = { workspace = true }
-
 [[bin]]
 name = "ls"
 path = "src/main.rs"

src/uu/ls/src/ls.rs

@@ -3,7 +3,7 @@
 // For the full copyright and license information, please view the LICENSE
 // file that was distributed with this source code.
 
-// spell-checker:ignore (ToDO) somegroup nlink tabsize dired subdired dtype colorterm getxattr
+// spell-checker:ignore (ToDO) somegroup nlink tabsize dired subdired dtype colorterm
 
 use clap::{
     builder::{NonEmptyStringValueParser, ValueParser},
@@ -36,7 +36,8 @@ use std::{
 };
 use term_grid::{Cell, Direction, Filling, Grid, GridOptions};
 use unicode_width::UnicodeWidthStr;
-
+#[cfg(all(unix, not(any(target_os = "android", target_os = "macos"))))]
+use uucore::fsxattr::has_acl;
 #[cfg(any(
     target_os = "linux",
     target_os = "macos",
@@ -2621,18 +2622,6 @@ fn display_grid(
     Ok(())
 }
 
-#[cfg(all(unix, not(any(target_os = "android", target_os = "macos"))))]
-fn file_has_acl<P: AsRef<Path>>(file: P) -> bool {
-    // don't use exacl here, it is doing more getxattr call then needed
-    match xattr::list(file) {
-        Ok(acl) => {
-            // if we have extra attributes, we have an acl
-            acl.count() > 0
-        }
-        Err(_) => false,
-    }
-}
-
 /// This writes to the BufWriter out a single string of the output of `ls -l`.
 ///
 /// It writes the following keys, in order:
@@ -2680,7 +2669,7 @@ fn display_item_long(
         // TODO: See how Mac should work here
         let is_acl_set = false;
         #[cfg(all(unix, not(any(target_os = "android", target_os = "macos"))))]
-        let is_acl_set = file_has_acl(item.display_name.as_os_str());
+        let is_acl_set = has_acl(item.display_name.as_os_str());
         write!(
             output_display,
             "{}{}{} {}",

src/uu/mv/Cargo.toml

@@ -21,6 +21,7 @@ indicatif = { workspace = true }
 uucore = { workspace = true, features = [
   "backup-control",
   "fs",
+  "fsxattr",
   "update-control",
 ] }
 

src/uu/mv/src/mv.rs

@@ -27,7 +27,10 @@ use uucore::fs::{
     are_hardlinks_or_one_way_symlink_to_same_file, are_hardlinks_to_same_file,
     path_ends_with_terminator,
 };
+#[cfg(all(unix, not(target_os = "macos")))]
+use uucore::fsxattr;
 use uucore::update_control;
+
 // These are exposed for projects (e.g. nushell) that want to create an `Options` value, which
 // requires these enums
 pub use uucore::{backup_control::BackupMode, update_control::UpdateMode};
@@ -631,6 +634,10 @@ fn rename_with_fallback(
                     None
                 };
 
+            #[cfg(all(unix, not(target_os = "macos")))]
+            let xattrs =
+                fsxattr::retrieve_xattrs(from).unwrap_or_else(|_| std::collections::HashMap::new());
+
             let result = if let Some(ref pb) = progress_bar {
                 move_dir_with_progress(from, to, &options, |process_info: TransitProcess| {
                     pb.set_position(process_info.copied_bytes);
@@ -641,6 +648,9 @@ fn rename_with_fallback(
                 move_dir(from, to, &options)
             };
 
+            #[cfg(all(unix, not(target_os = "macos")))]
+            fsxattr::apply_xattrs(to, xattrs).unwrap();
+
             if let Err(err) = result {
                 return match err.kind {
                     fs_extra::error::ErrorKind::PermissionDenied => Err(io::Error::new(
@@ -651,6 +661,11 @@ fn rename_with_fallback(
                 };
             }
         } else {
+            #[cfg(all(unix, not(target_os = "macos")))]
+            fs::copy(from, to)
+                .and_then(|_| fsxattr::copy_xattrs(&from, &to))
+                .and_then(|_| fs::remove_file(from))?;
+            #[cfg(any(target_os = "macos", not(unix)))]
             fs::copy(from, to).and_then(|_| fs::remove_file(from))?;
         }
     }

src/uucore/Cargo.toml

@@ -54,6 +54,7 @@ sm3 = { workspace = true, optional = true }
 [target.'cfg(unix)'.dependencies]
 walkdir = { workspace = true, optional = true }
 nix = { workspace = true, features = ["fs", "uio", "zerocopy", "signal"] }
+xattr = { workspace = true, optional = true }
 
 [dev-dependencies]
 clap = { workspace = true }
@@ -77,6 +78,7 @@ encoding = ["data-encoding", "data-encoding-macro", "z85", "thiserror"]
 entries = ["libc"]
 fs = ["dunce", "libc", "winapi-util", "windows-sys"]
 fsext = ["libc", "time", "windows-sys"]
+fsxattr = ["xattr"]
 lines = []
 format = ["itertools"]
 mode = ["libc"]

src/uucore/src/lib/features.rs

@@ -46,6 +46,8 @@ pub mod pipes;
 #[cfg(all(unix, feature = "process"))]
 pub mod process;
 
+#[cfg(all(unix, not(target_os = "macos"), feature = "fsxattr"))]
+pub mod fsxattr;
 #[cfg(all(unix, not(target_os = "fuchsia"), feature = "signals"))]
 pub mod signals;
 #[cfg(all(

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

@@ -0,0 +1,154 @@
+// This file is part of the uutils coreutils package.
+//
+// For the full copyright and license information, please view the LICENSE
+// file that was distributed with this source code.
+
+// spell-checker:ignore getxattr
+
+//! Set of functions to manage xattr on files and dirs
+use std::collections::HashMap;
+use std::ffi::OsString;
+use std::path::Path;
+
+/// Copies extended attributes (xattrs) from one file or directory to another.
+///
+/// # Arguments
+///
+/// * `source` - A reference to the source path.
+/// * `dest` - A reference to the destination path.
+///
+/// # Returns
+///
+/// A result indicating success or failure.
+pub fn copy_xattrs<P: AsRef<Path>>(source: P, dest: P) -> std::io::Result<()> {
+    for attr_name in xattr::list(&source)? {
+        if let Some(value) = xattr::get(&source, &attr_name)? {
+            xattr::set(&dest, &attr_name, &value)?;
+        }
+    }
+    Ok(())
+}
+
+/// Retrieves the extended attributes (xattrs) of a given file or directory.
+///
+/// # Arguments
+///
+/// * `source` - A reference to the path of the file or directory.
+///
+/// # Returns
+///
+/// A result containing a HashMap of attributes names and values, or an error.
+pub fn retrieve_xattrs<P: AsRef<Path>>(source: P) -> std::io::Result<HashMap<OsString, Vec<u8>>> {
+    let mut attrs = HashMap::new();
+    for attr_name in xattr::list(&source)? {
+        if let Some(value) = xattr::get(&source, &attr_name)? {
+            attrs.insert(attr_name, value);
+        }
+    }
+    Ok(attrs)
+}
+
+/// Applies extended attributes (xattrs) to a given file or directory.
+///
+/// # Arguments
+///
+/// * `dest` - A reference to the path of the file or directory.
+/// * `xattrs` - A HashMap containing attribute names and their corresponding values.
+///
+/// # Returns
+///
+/// A result indicating success or failure.
+pub fn apply_xattrs<P: AsRef<Path>>(
+    dest: P,
+    xattrs: HashMap<OsString, Vec<u8>>,
+) -> std::io::Result<()> {
+    for (attr, value) in xattrs {
+        xattr::set(&dest, &attr, &value)?;
+    }
+    Ok(())
+}
+
+/// Checks if a file has an Access Control List (ACL) based on its extended attributes.
+///
+/// # Arguments
+///
+/// * `file` - A reference to the path of the file.
+///
+/// # Returns
+///
+/// `true` if the file has extended attributes (indicating an ACL), `false` otherwise.
+pub fn has_acl<P: AsRef<Path>>(file: P) -> bool {
+    // don't use exacl here, it is doing more getxattr call then needed
+    match xattr::list(file) {
+        Ok(acl) => {
+            // if we have extra attributes, we have an acl
+            acl.count() > 0
+        }
+        Err(_) => false,
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use std::fs::File;
+    use tempfile::tempdir;
+
+    #[test]
+    fn test_copy_xattrs() {
+        let temp_dir = tempdir().unwrap();
+        let source_path = temp_dir.path().join("source.txt");
+        let dest_path = temp_dir.path().join("dest.txt");
+
+        File::create(&source_path).unwrap();
+        File::create(&dest_path).unwrap();
+
+        let test_attr = "user.test";
+        let test_value = b"test value";
+        xattr::set(&source_path, test_attr, test_value).unwrap();
+
+        copy_xattrs(&source_path, &dest_path).unwrap();
+
+        let copied_value = xattr::get(&dest_path, test_attr).unwrap().unwrap();
+        assert_eq!(copied_value, test_value);
+    }
+
+    #[test]
+    fn test_apply_and_retrieve_xattrs() {
+        let temp_dir = tempdir().unwrap();
+        let file_path = temp_dir.path().join("test_file.txt");
+
+        File::create(&file_path).unwrap();
+
+        let mut test_xattrs = HashMap::new();
+        let test_attr = "user.test_attr";
+        let test_value = b"test value";
+        test_xattrs.insert(OsString::from(test_attr), test_value.to_vec());
+        apply_xattrs(&file_path, test_xattrs).unwrap();
+
+        let retrieved_xattrs = retrieve_xattrs(&file_path).unwrap();
+        assert!(retrieved_xattrs.contains_key(OsString::from(test_attr).as_os_str()));
+        assert_eq!(
+            retrieved_xattrs
+                .get(OsString::from(test_attr).as_os_str())
+                .unwrap(),
+            test_value
+        );
+    }
+
+    #[test]
+    fn test_file_has_acl() {
+        let temp_dir = tempdir().unwrap();
+        let file_path = temp_dir.path().join("test_file.txt");
+
+        File::create(&file_path).unwrap();
+
+        assert!(!has_acl(&file_path));
+
+        let test_attr = "user.test_acl";
+        let test_value = b"test value";
+        xattr::set(&file_path, test_attr, test_value).unwrap();
+
+        assert!(has_acl(&file_path));
+    }
+}

src/uucore/src/lib/lib.rs

@@ -43,8 +43,6 @@ pub use crate::features::encoding;
 pub use crate::features::format;
 #[cfg(feature = "fs")]
 pub use crate::features::fs;
-#[cfg(feature = "fsext")]
-pub use crate::features::fsext;
 #[cfg(feature = "lines")]
 pub use crate::features::lines;
 #[cfg(feature = "quoting-style")]
@@ -89,6 +87,12 @@ pub use crate::features::utmpx;
 #[cfg(all(windows, feature = "wide"))]
 pub use crate::features::wide;
 
+#[cfg(feature = "fsext")]
+pub use crate::features::fsext;
+
+#[cfg(all(unix, not(target_os = "macos"), feature = "fsxattr"))]
+pub use crate::features::fsxattr;
+
 //## core functions
 
 use std::ffi::OsString;

tests/by-util/test_cp.rs

@@ -57,7 +57,7 @@ static TEST_MOUNT_OTHER_FILESYSTEM_FILE: &str = "mount/DO_NOT_copy_me.txt";
 #[cfg(unix)]
 static TEST_NONEXISTENT_FILE: &str = "nonexistent_file.txt";
 #[cfg(all(unix, not(any(target_os = "android", target_os = "macos"))))]
-use xattr;
+use crate::common::util::compare_xattrs;
 
 /// Assert that mode, ownership, and permissions of two metadata objects match.
 #[cfg(all(not(windows), not(target_os = "freebsd")))]
@@ -3739,21 +3739,6 @@ fn test_cp_no_such() {
         .stderr_is("cp: 'no-such/' is not a directory\n");
 }
 
-#[cfg(all(unix, not(any(target_os = "android", target_os = "macos"))))]
-fn compare_xattrs<P: AsRef<Path>>(path1: P, path2: P) -> bool {
-    let get_sorted_xattrs = |path: P| {
-        xattr::list(path)
-            .map(|attrs| {
-                let mut attrs = attrs.collect::<Vec<_>>();
-                attrs.sort();
-                attrs
-            })
-            .unwrap_or_else(|_| Vec::new())
-    };
-
-    get_sorted_xattrs(path1) == get_sorted_xattrs(path2)
-}
-
 #[cfg(all(unix, not(any(target_os = "android", target_os = "macos"))))]
 #[test]
 fn test_acl_preserve() {

tests/by-util/test_mv.rs

@@ -1569,6 +1569,47 @@ fn test_mv_dir_into_path_slash() {
     assert!(at.dir_exists("f/b"));
 }
 
+#[cfg(all(unix, not(target_os = "macos")))]
+#[test]
+fn test_acl() {
+    use std::process::Command;
+
+    use crate::common::util::compare_xattrs;
+
+    let scene = TestScenario::new(util_name!());
+    let at = &scene.fixtures;
+    let path1 = "a";
+    let path2 = "b";
+    let file = "a/file";
+    let file_target = "b/file";
+    at.mkdir(path1);
+    at.mkdir(path2);
+    at.touch(file);
+
+    let path = at.plus_as_string(file);
+    // calling the command directly. xattr requires some dev packages to be installed
+    // and it adds a complex dependency just for a test
+    match Command::new("setfacl")
+        .args(["-m", "group::rwx", &path1])
+        .status()
+        .map(|status| status.code())
+    {
+        Ok(Some(0)) => {}
+        Ok(_) => {
+            println!("test skipped: setfacl failed");
+            return;
+        }
+        Err(e) => {
+            println!("test skipped: setfacl failed with {}", e);
+            return;
+        }
+    }
+
+    scene.ucmd().arg(&path).arg(path2).succeeds();
+
+    assert!(compare_xattrs(&file, &file_target));
+}
+
 // Todo:
 
 // $ at.touch a b

tests/common/util.rs

@@ -756,6 +756,26 @@ pub fn get_root_path() -> &'static str {
     }
 }
 
+/// Compares the extended attributes (xattrs) of two files or directories.
+///
+/// # Returns
+///
+/// `true` if both paths have the same set of extended attributes, `false` otherwise.
+#[cfg(all(unix, not(target_os = "macos")))]
+pub fn compare_xattrs<P: AsRef<std::path::Path>>(path1: P, path2: P) -> bool {
+    let get_sorted_xattrs = |path: P| {
+        xattr::list(path)
+            .map(|attrs| {
+                let mut attrs = attrs.collect::<Vec<_>>();
+                attrs.sort();
+                attrs
+            })
+            .unwrap_or_else(|_| Vec::new())
+    };
+
+    get_sorted_xattrs(path1) == get_sorted_xattrs(path2)
+}
+
 /// Object-oriented path struct that represents and operates on
 /// paths relative to the directory it was constructed for.
 #[derive(Clone)]
@@ -3375,4 +3395,26 @@ mod tests {
         );
         assert!(command.tmpd.is_some());
     }
+
+    #[cfg(all(unix, not(target_os = "macos")))]
+    #[test]
+    fn test_compare_xattrs() {
+        use tempfile::tempdir;
+
+        let temp_dir = tempdir().unwrap();
+        let file_path1 = temp_dir.path().join("test_file1.txt");
+        let file_path2 = temp_dir.path().join("test_file2.txt");
+
+        File::create(&file_path1).unwrap();
+        File::create(&file_path2).unwrap();
+
+        let test_attr = "user.test_attr";
+        let test_value = b"test value";
+        xattr::set(&file_path1, test_attr, test_value).unwrap();
+
+        assert!(!compare_xattrs(&file_path1, &file_path2));
+
+        xattr::set(&file_path2, test_attr, test_value).unwrap();
+        assert!(compare_xattrs(&file_path1, &file_path2));
+    }
 }