Skip to content

Commit

Permalink
mv: temp_fs test fix
Browse files Browse the repository at this point in the history
  • Loading branch information
@matrixhead
matrixhead committed Oct 9, 2024
1 parent ef3a5eb commit 59f85e3
Show file tree
Hide file tree
Showing 2 changed files with 82 additions and 51 deletions.
61 changes: 29 additions & 32 deletions src/uu/mv/src/mv.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ mod error;

use clap::builder::ValueParser;
use clap::{crate_version, error::ErrorKind, Arg, ArgAction, ArgMatches, Command};
use filetime::{set_file_times, set_symlink_file_times};
use filetime::set_symlink_file_times;
use indicatif::{MultiProgress, ProgressBar, ProgressStyle};
use std::collections::HashSet;
use std::env;
Expand Down Expand Up @@ -923,7 +923,7 @@ fn move_dir(
let mut last_rem_err_depth: Option<usize> = None;
while let Some((src_path, file_type, dest_path, src_md, depth)) = moved_entries.pop() {
if let Some(src_metadata) = src_md {
copy_metadata(&src_path, &dest_path, &src_metadata).ok();
copy_metadata(&src_path, &dest_path, &src_metadata);
}
if matches!(last_rem_err_depth,Some(lred)if lred > depth) {
// This means current dir entry is parent directory of a child
Expand Down Expand Up @@ -1008,47 +1008,44 @@ fn copy_file(
}
}

// For GNU compatibility, this function does not report any errors that occur within it.
#[allow(unused_variables)]
fn copy_metadata(src: &Path, dest: &Path, src_metadata: &fs::Metadata) -> io::Result<()> {
// Get metadata of the dest file
let dest_metadata = fs::symlink_metadata(dest)?;
fn copy_metadata(src: &Path, dest: &Path, src_metadata: &fs::Metadata) {
// Copy file permissions
let permissions = src_metadata.permissions();
fs::set_permissions(dest, permissions)?;

// Copy xattrs
#[cfg(all(unix, not(any(target_os = "macos", target_os = "redox"))))]
fsxattr::copy_xattrs(src, dest)?;
fs::set_permissions(dest, permissions).ok();

// Copy ownership (if on Unix-like system)
#[cfg(unix)]
{
let uid = MetadataExt::uid(src_metadata);
let gid = MetadataExt::gid(src_metadata);
wrap_chown(
dest,
&dest_metadata,
Some(uid),
Some(gid),
false,
Verbosity {
groups_only: false,
level: VerbosityLevel::Silent,
},
)
.map_err(|err| io::Error::new(io::ErrorKind::Other, err))?;
if let Ok(dest_md) = fs::symlink_metadata(dest).as_ref() {
wrap_chown(
dest,
dest_md,
Some(uid),
Some(gid),
false,
Verbosity {
groups_only: false,
level: VerbosityLevel::Silent,
},
)
.ok();
}
}

// Copy the modified and accessed timestamps
let modified_time = src_metadata.modified()?;
let accessed_time = src_metadata.accessed()?;
if dest_metadata.is_symlink() {
set_symlink_file_times(dest, accessed_time.into(), modified_time.into())?;
} else {
set_file_times(dest, accessed_time.into(), modified_time.into())?;
let modified_time = src_metadata.modified();
let accessed_time = src_metadata.accessed();
if let (Ok(modified_time), Ok(accessed_time)) = (modified_time, accessed_time) {
set_symlink_file_times(dest, accessed_time.into(), modified_time.into()).ok();
}

Ok(())
// Copy xattrs.
#[cfg(all(unix, not(any(target_os = "macos", target_os = "redox"))))]
fsxattr::copy_xattrs(src, dest).ok();
}

#[cfg(test)]
Expand Down Expand Up @@ -1260,7 +1257,7 @@ mod tests {
let src_md = fs::metadata(&src_path).unwrap();

// Call the function under test
copy_metadata(&src_path, &dest_path, &src_md).unwrap();
copy_metadata(&src_path, &dest_path, &src_md);

// Verify that the permissions were copied
let dest_permissions = fs::metadata(&dest_path).unwrap().permissions();
Expand Down Expand Up @@ -1289,7 +1286,7 @@ mod tests {
.expect("couldn't get accessed time for src file");

//Try to copy metadata
copy_metadata(&src_path, &dest_path, &src_metadata).unwrap();
copy_metadata(&src_path, &dest_path, &src_metadata);

// Get file times for the dest file
let dest_metadata = fs::metadata(&dest_path).expect("couldn't get metadata for dest file");
Expand Down Expand Up @@ -1326,7 +1323,7 @@ mod tests {
apply_xattrs(&src_path, test_xattrs).expect("couldn't apply xattr to the destination file");

//Try to copy metadata
copy_metadata(&src_path, &dest_path, &src_metadata).expect("couldn't copy metadata");
copy_metadata(&src_path, &dest_path, &src_metadata);

// Verify that the xattrs were copied
let retrieved_xattrs = retrieve_xattrs(&dest_path).unwrap();
Expand Down
72 changes: 53 additions & 19 deletions tests/by-util/test_mv.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1629,7 +1629,8 @@ mod inter_partition_copying {
use super::*;
use crate::common::util::TestScenario;
use std::ffi::OsString;
use std::fs::{read_to_string, set_permissions, write};
use std::fs::{read_to_string, set_permissions, write, File};
use std::io;
use std::os::unix::fs::{symlink, FileTypeExt, MetadataExt, PermissionsExt};
use tempfile::TempDir;
use uucore::display::Quotable;
Expand Down Expand Up @@ -1947,6 +1948,20 @@ mod inter_partition_copying {
// create a folder in another partition.
let other_fs_tempdir =
TempDir::new_in("/dev/shm/").expect("Unable to create temp directory");

// https://github.com/Stebalien/xattr/issues/24#issuecomment-1279682009
let mut dest_fs_xattr_support = true;
let tempfile_path = other_fs_tempdir.path().join("temp_file");
let tf = File::create(&tempfile_path).expect("couldn't create tempfile");
if let Err(err) = tf.set_xattr(test_attr, test_value) {
if err.kind() == io::ErrorKind::Unsupported {
dest_fs_xattr_support = false;

Check warning on line 1958 in tests/by-util/test_mv.rs

View check run for this annotation

Codecov / codecov/patch

tests/by-util/test_mv.rs#L1958 

Added line #L1958 was not covered by tests
}
}
if !dest_fs_xattr_support {
println!("no fs xattr support");

Check warning on line 1962 in tests/by-util/test_mv.rs

View check run for this annotation

Codecov / codecov/patch

tests/by-util/test_mv.rs#L1962 

Added line #L1962 was not covered by tests
}

// make sure to wait for a second so that when the dest file is created, it
// would have a different filetime so that the only way the dest file
// would have a same timestamp is by copying the timestamp of src file.
Expand Down Expand Up @@ -1979,15 +1994,18 @@ mod inter_partition_copying {
accessed_time
);

// Verify that the xattrs were copied
let retrieved_xattrs = retrieve_xattrs(other_fs_tempdir.path().join("dir/file")).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())
.expect("couldn't find xattr with name user.test_attr"),
test_value
);
if dest_fs_xattr_support {
// Verify that the xattrs were copied
let retrieved_xattrs =
retrieve_xattrs(other_fs_tempdir.path().join("dir/file")).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())
.expect("couldn't find xattr with name user.test_attr"),
test_value
);
}
}

// this test would fail if progress bar flag is set to true because we need
Expand Down Expand Up @@ -2023,6 +2041,20 @@ mod inter_partition_copying {
// make sure to wait for a second so that when the dest file is created, it
// would have a different filetime so that the only way the dest file
// would have a same timestamp is by copying the timestamp of src file.

// https://github.com/Stebalien/xattr/issues/24#issuecomment-1279682009
let mut dest_fs_xattr_support = true;
let tempfile_path = other_fs_tempdir.path().join("temp_file");
let tf = File::create(&tempfile_path).expect("couldn't create tempfile");
if let Err(err) = tf.set_xattr(test_attr, test_value) {
if err.kind() == io::ErrorKind::Unsupported {
dest_fs_xattr_support = false;

Check warning on line 2051 in tests/by-util/test_mv.rs

View check run for this annotation

Codecov / codecov/patch

tests/by-util/test_mv.rs#L2051 

Added line #L2051 was not covered by tests
}
}
if !dest_fs_xattr_support {
println!("no fs xattr support");

Check warning on line 2055 in tests/by-util/test_mv.rs

View check run for this annotation

Codecov / codecov/patch

tests/by-util/test_mv.rs#L2055 

Added line #L2055 was not covered by tests
}

sleep(Duration::from_secs(1));
// mv to other partition
scene
Expand Down Expand Up @@ -2052,15 +2084,17 @@ mod inter_partition_copying {
accessed_time
);

// Verify that the xattrs were copied
let retrieved_xattrs = retrieve_xattrs(other_fs_tempdir.path().join("dir")).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())
.expect("couldn't find xattr with name user.test_attr"),
test_value
);
if dest_fs_xattr_support {
// Verify that the xattrs were copied
let retrieved_xattrs = retrieve_xattrs(other_fs_tempdir.path().join("dir")).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())
.expect("couldn't find xattr with name user.test_attr"),
test_value
);
}
}

#[test]
Expand Down

0 comments on commit 59f85e3

Please sign in to comment.