Skip to content

Commit 8fbe48e

Browse files
committed
mv: Make mv command fallback to copy only if the src and dst are on different device
1 parent ee0d178 commit 8fbe48e

File tree

5 files changed

+139
-1
lines changed

5 files changed

+139
-1
lines changed

.vscode/cspell.dictionaries/workspace.wordlist.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,7 @@ vmsplice
136136

137137
# * vars/libc
138138
COMFOLLOW
139+
EXDEV
139140
FILENO
140141
FTSENT
141142
HOSTSIZE

Cargo.lock

Lines changed: 2 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/uu/mv/Cargo.toml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,12 @@ uucore = { workspace = true, features = [
2828
] }
2929
thiserror = { workspace = true }
3030

31+
[target.'cfg(windows)'.dependencies]
32+
windows-sys = { workspace = true, features = ["Win32_Foundation", "Win32_Security", "Win32_Storage_FileSystem"] }
33+
34+
[target.'cfg(unix)'.dependencies]
35+
libc = { workspace = true }
36+
3137
[[bin]]
3238
name = "mv"
3339
path = "src/main.rs"

src/uu/mv/src/mv.rs

Lines changed: 64 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -657,7 +657,22 @@ fn rename_with_fallback(
657657
to: &Path,
658658
multi_progress: Option<&MultiProgress>,
659659
) -> io::Result<()> {
660-
if fs::rename(from, to).is_err() {
660+
if let Err(err) = fs::rename(from, to) {
661+
#[cfg(windows)]
662+
const EXDEV: i32 = windows_sys::Win32::Foundation::ERROR_NOT_SAME_DEVICE as _;
663+
#[cfg(unix)]
664+
const EXDEV: i32 = libc::EXDEV as _;
665+
666+
// We will only copy if:
667+
// 1. Files are on different devices (EXDEV error)
668+
// 2. On Windows, if the target file exists and source file is opened by another process
669+
// (MoveFileExW fails with "Access Denied" even if the source file has FILE_SHARE_DELETE permission)
670+
let should_fallback = matches!(err.raw_os_error(), Some(EXDEV))
671+
|| (from.is_file() && can_delete_file(from).unwrap_or(false));
672+
if !should_fallback {
673+
return Err(err);
674+
}
675+
661676
// Get metadata without following symlinks
662677
let metadata = from.symlink_metadata()?;
663678
let file_type = metadata.file_type();
@@ -792,3 +807,51 @@ fn is_empty_dir(path: &Path) -> bool {
792807
Err(_e) => false,
793808
}
794809
}
810+
811+
/// Checks if a file can be deleted by attempting to open it with delete permissions.
812+
#[cfg(windows)]
813+
fn can_delete_file(path: &Path) -> Result<bool, io::Error> {
814+
use std::{
815+
os::windows::ffi::OsStrExt as _,
816+
ptr::{null, null_mut},
817+
};
818+
819+
use windows_sys::Win32::{
820+
Foundation::{CloseHandle, INVALID_HANDLE_VALUE},
821+
Storage::FileSystem::{
822+
CreateFileW, DELETE, FILE_ATTRIBUTE_NORMAL, FILE_SHARE_DELETE, FILE_SHARE_READ,
823+
FILE_SHARE_WRITE, OPEN_EXISTING,
824+
},
825+
};
826+
827+
let wide_path = path
828+
.as_os_str()
829+
.encode_wide()
830+
.chain([0])
831+
.collect::<Vec<u16>>();
832+
833+
let handle = unsafe {
834+
CreateFileW(
835+
wide_path.as_ptr(),
836+
DELETE,
837+
FILE_SHARE_DELETE | FILE_SHARE_READ | FILE_SHARE_WRITE,
838+
null(),
839+
OPEN_EXISTING,
840+
FILE_ATTRIBUTE_NORMAL,
841+
null_mut(),
842+
)
843+
};
844+
845+
if handle == INVALID_HANDLE_VALUE {
846+
return Err(io::Error::last_os_error());
847+
}
848+
849+
unsafe { CloseHandle(handle) };
850+
851+
Ok(true)
852+
}
853+
854+
#[cfg(not(windows))]
855+
fn can_delete_file(_: &Path) -> Result<bool, io::Error> {
856+
Ok(true)
857+
}

tests/by-util/test_mv.rs

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,27 @@ fn test_mv_rename_file() {
4949
assert!(at.file_exists(file2));
5050
}
5151

52+
#[test]
53+
fn test_mv_with_source_file_opened_and_target_file_exists() {
54+
let (at, mut ucmd) = at_and_ucmd!();
55+
56+
let src = "source_file_opened";
57+
let dst = "target_file_exists";
58+
59+
let f = at.make_file(src);
60+
61+
at.touch(dst);
62+
63+
ucmd.arg(dst)
64+
.arg(src)
65+
.arg("--update=all")
66+
.succeeds()
67+
.no_stderr()
68+
.no_stdout();
69+
70+
drop(f);
71+
}
72+
5273
#[test]
5374
fn test_mv_move_file_into_dir() {
5475
let (at, mut ucmd) = at_and_ucmd!();
@@ -1670,6 +1691,51 @@ fn test_acl() {
16701691
assert!(compare_xattrs(&file, &file_target));
16711692
}
16721693

1694+
#[test]
1695+
#[cfg(windows)]
1696+
fn test_move_should_not_fallback_to_copy() {
1697+
use std::os::windows::fs::OpenOptionsExt;
1698+
1699+
let (at, mut ucmd) = at_and_ucmd!();
1700+
1701+
let locked_file = "a_file_is_locked";
1702+
let locked_file_path = at.plus(locked_file);
1703+
let file = std::fs::OpenOptions::new()
1704+
.create(true)
1705+
.write(true)
1706+
.share_mode(
1707+
uucore::windows_sys::Win32::Storage::FileSystem::FILE_SHARE_READ
1708+
| uucore::windows_sys::Win32::Storage::FileSystem::FILE_SHARE_WRITE,
1709+
)
1710+
.open(locked_file_path);
1711+
1712+
let target_file = "target_file";
1713+
ucmd.arg(locked_file).arg(target_file).fails();
1714+
1715+
assert!(at.file_exists(locked_file));
1716+
assert!(!at.file_exists(target_file));
1717+
1718+
drop(file);
1719+
}
1720+
1721+
#[test]
1722+
#[cfg(unix)]
1723+
fn test_move_should_not_fallback_to_copy() {
1724+
let (at, mut ucmd) = at_and_ucmd!();
1725+
1726+
let readonly_dir = "readonly_dir";
1727+
let locked_file = "readonly_dir/a_file_is_locked";
1728+
at.mkdir(readonly_dir);
1729+
at.touch(locked_file);
1730+
at.set_mode(readonly_dir, 0o555);
1731+
1732+
let target_file = "target_file";
1733+
ucmd.arg(locked_file).arg(target_file).fails();
1734+
1735+
assert!(at.file_exists(locked_file));
1736+
assert!(!at.file_exists(target_file));
1737+
}
1738+
16731739
// Todo:
16741740

16751741
// $ at.touch a b

0 commit comments

Comments
 (0)