WIP

Author: nikolalukovic

src/uu/cp/src/copydir.rs

@@ -166,6 +166,7 @@ impl<'a> Context<'a> {
 ///     }
 /// ];
 /// ```
+#[derive(Debug)]
 struct Entry {
     /// The absolute path to file or directory to copy.
     source_absolute: PathBuf,
@@ -178,6 +179,9 @@ struct Entry {
 
     /// Whether the destination is a file.
     target_is_file: bool,
+
+    /// Whether we created the destination dir
+    target_is_created: bool,
 }
 
 impl Entry {
@@ -231,6 +235,7 @@ impl Entry {
             source_relative,
             local_to_target,
             target_is_file,
+            target_is_created: false,
         })
     }
 }
@@ -239,7 +244,7 @@ impl Entry {
 /// Copy a single entry during a directory traversal.
 fn copy_direntry(
     progress_bar: Option<&ProgressBar>,
-    entry: &Entry,
+    entry: &mut Entry,
     entry_is_symlink: bool,
     entry_is_dir_no_follow: bool,
     options: &Options,
@@ -270,6 +275,7 @@ fn copy_direntry(
                 options,
                 Some(&entry.source_absolute),
             )?;
+            entry.target_is_created = true;
             if options.verbose {
                 println!(
                     "{}",
@@ -421,7 +427,7 @@ pub(crate) fn copy_directory(
     let mut last_iter: Option<DirEntry> = None;
 
     // Keep track of all directories we've created that need permission fixes
-    let mut dirs_needing_permissions: Vec<(PathBuf, PathBuf)> = Vec::new();
+    let mut dirs_needing_permissions: Vec<(PathBuf, PathBuf, bool)> = Vec::new();
 
     // Traverse the contents of the directory, copying each one.
     for direntry_result in WalkDir::new(root)
@@ -440,11 +446,11 @@ pub(crate) fn copy_directory(
                         }
                         Err(_) => (direntry_type.is_symlink(), direntry_type.is_dir()),
                     };
-                let entry = Entry::new(&context, direntry_path, options.no_target_dir)?;
+                let mut entry = Entry::new(&context, direntry_path, options.no_target_dir)?;
 
                 copy_direntry(
                     progress_bar,
-                    &entry,
+                    &mut entry,
                     entry_is_symlink,
                     entry_is_dir_no_follow,
                     options,
@@ -470,8 +476,11 @@ pub(crate) fn copy_directory(
                     entry_is_dir_no_follow || (options.dereference && direntry_path.is_dir());
                 if is_dir_for_permissions {
                     // Add this directory to our list for permission fixing later
-                    dirs_needing_permissions
-                        .push((entry.source_absolute.clone(), entry.local_to_target.clone()));
+                    dirs_needing_permissions.push((
+                        entry.source_absolute.clone(),
+                        entry.local_to_target.clone(),
+                        entry.target_is_created,
+                    ));
 
                     // If true, last_iter is not a parent of this iter.
                     // The means we just exited a directory.
@@ -504,6 +513,7 @@ pub(crate) fn copy_directory(
                                 &entry.source_absolute,
                                 &entry.local_to_target,
                                 options,
+                                entry.target_is_created,
                             )?;
                         }
                     }
@@ -519,8 +529,8 @@ pub(crate) fn copy_directory(
 
     // Fix permissions for all directories we created
     // This ensures that even sibling directories get their permissions fixed
-    for (source_path, dest_path) in dirs_needing_permissions {
-        copy_attributes(&source_path, &dest_path, options)?;
+    for (source_path, dest_path, is_dir_created) in dirs_needing_permissions {
+        copy_attributes(&source_path, &dest_path, options, is_dir_created)?;
     }
 
     // Also fix permissions for parent directories,
@@ -529,7 +539,7 @@ pub(crate) fn copy_directory(
         let dest = target.join(root.file_name().unwrap());
         for (x, y) in aligned_ancestors(root, dest.as_path()) {
             if let Ok(src) = canonicalize(x, MissingHandling::Normal, ResolveMode::Physical) {
-                copy_attributes(&src, y, options)?;
+                copy_attributes(&src, y, options, false)?;
             }
         }
     }

src/uu/cp/src/cp.rs

@@ -194,7 +194,7 @@ pub enum SparseMode {
 }
 
 /// The expected file type of copy target
-#[derive(Copy, Clone)]
+#[derive(Copy, Clone, Debug)]
 pub enum TargetType {
     Directory,
     File,
@@ -1506,7 +1506,7 @@ fn copy_source(
         if options.parents {
             for (x, y) in aligned_ancestors(source, dest.as_path()) {
                 if let Ok(src) = canonicalize(x, MissingHandling::Normal, ResolveMode::Physical) {
-                    copy_attributes(&src, y, options)?;
+                    copy_attributes(&src, y, options, false)?;
                 }
             }
         }
@@ -1655,11 +1655,22 @@ fn copy_extended_attrs(source: &Path, dest: &Path) -> CopyResult<()> {
 }
 
 /// Copy the specified attributes from one path to another.
-pub(crate) fn copy_attributes(source: &Path, dest: &Path, options: &Options) -> CopyResult<()> {
+pub(crate) fn copy_attributes(
+    source: &Path,
+    dest: &Path,
+    options: &Options,
+    is_dir_created: bool,
+) -> CopyResult<()> {
     let context = &*format!("{} -> {}", source.quote(), dest.quote());
     let source_metadata =
         fs::symlink_metadata(source).map_err(|e| CpError::IoErrContext(e, context.to_owned()))?;
 
+    let mode = if dest.is_dir() && is_dir_created {
+        Preserve::Yes { required: false }
+    } else {
+        options.attributes.mode
+    };
+
     // Ownership must be changed first to avoid interfering with mode change.
     #[cfg(unix)]
     handle_preserve(&options.attributes.ownership, || -> CopyResult<()> {
@@ -1697,22 +1708,22 @@ pub(crate) fn copy_attributes(source: &Path, dest: &Path, options: &Options) ->
         Ok(())
     })?;
 
-    handle_preserve(&options.attributes.mode, || -> CopyResult<()> {
+    handle_preserve(&mode, || -> CopyResult<()> {
         // The `chmod()` system call that underlies the
         // `fs::set_permissions()` call is unable to change the
         // permissions of a symbolic link. In that case, we just
         // do nothing, since every symbolic link has the same
         // permissions.
         if !dest.is_symlink() {
-            // let dest_permissions = calculate_dest_permissions(
-            //     fs::metadata(dest).ok().as_ref(),
-            //     dest,
-            //     &source_metadata,
-            //     options,
-            //     context,
-            // )?;
-            let dest_permissions = source_metadata.permissions();
-            fs::set_permissions(dest, dest_permissions)
+            let mut perms = source_metadata.permissions();
+            if is_dir_created {
+                let mode = handle_no_preserve_mode(options, perms.mode());
+                use uucore::mode::get_umask;
+                let mode = mode & !get_umask();
+                perms.set_mode(mode);
+            }
+
+            fs::set_permissions(dest, perms)
                 .map_err(|e| CpError::IoErrContext(e, context.to_owned()))?;
             // FIXME: Implement this for windows as well
             #[cfg(feature = "feat_acl")]
@@ -2516,7 +2527,7 @@ fn copy_file(
     if options.dereference(source_in_command_line) {
         if let Ok(src) = canonicalize(source, MissingHandling::Normal, ResolveMode::Physical) {
             if src.exists() {
-                copy_attributes(&src, dest, options)?;
+                copy_attributes(&src, dest, options, false)?;
             }
         }
     } else if source_is_stream && !source.exists() {
@@ -2525,7 +2536,7 @@ fn copy_file(
         // attributes. However, this is already handled in the stream
         // copy function (see `copy_stream` under platform/linux.rs).
     } else {
-        copy_attributes(source, dest, options)?;
+        copy_attributes(source, dest, options, false)?;
     }
 
     #[cfg(feature = "selinux")]
@@ -2701,7 +2712,7 @@ fn copy_link(
         delete_path(dest, options)?;
     }
     symlink_file(&link, dest, symlinked_files)?;
-    copy_attributes(source, dest, options)
+    copy_attributes(source, dest, options, false)
 }
 
 /// Generate an error message if `target` is not the correct `target_type`

src/uucore/src/lib/features/fs.rs

@@ -542,7 +542,6 @@ pub fn display_permissions_unix(mode: mode_t, display_file_type: bool) -> String
 /// std::fs::create_dir("foo/."); fails in pure Rust
 pub fn dir_strip_dot_for_creation(path: &Path) -> PathBuf {
     let path_str = path.to_string_lossy();
-
     if path_str.ends_with("/.") || path_str.ends_with("/./") {
         // Do a simple dance to strip the "/."
         Path::new(&path).components().collect()

tests/by-util/test_cp.rs

@@ -25,7 +25,7 @@ use std::os::unix::fs::{FileTypeExt, MetadataExt};
 use std::os::windows::fs::symlink_file;
 #[cfg(not(windows))]
 use std::path::Path;
-#[cfg(target_os = "linux")]
+
 use std::path::PathBuf;
 
 #[cfg(any(target_os = "linux", target_os = "android"))]