Skip to content

Commit 05fd0ff

Browse files
joshuamegnauth54joshuamegnauth54
committed
host_env: renameat support for os.rename
Python's `os.rename` supports renaming a path relative to a directory descriptor - essentially, `renameat`. The syscall and its Rustix wrapper does all of the work, so this patch mainly updates the Python signature to forward to the implementation. As of this patch, `os.rename` is still incorrect for Windows because it always replaces the destination file. I added a small note for future contributors (or myself, since I might tackle it) as a reminder of what's broken.
1 parent dbfd69e commit 05fd0ff

2 files changed

Lines changed: 58 additions & 8 deletions

File tree

‎crates/host_env/src/os.rs‎

Lines changed: 32 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@ use core::ffi::CStr;
1010
use core::str::Utf8Error;
1111
#[cfg(windows)]
1212
use core::time::Duration;
13+
#[cfg(unix)]
14+
use rustix::fd::AsFd;
1315
use std::{
1416
env,
1517
ffi::{OsStr, OsString},
@@ -266,14 +268,42 @@ pub fn copy_file_range(
266268
rustix::fs::copy_file_range(src, offset_src, dst, offset_dst, count)
267269
}
268270

271+
#[cfg(windows)]
269272
pub fn rename(
270273
from: impl AsRef<std::path::Path>,
274+
from_fd: Option<crt_fd::Borrowed<'_>>,
271275
to: impl AsRef<std::path::Path>,
276+
to_fd: Option<crt_fd::Borrowed<'_>>,
272277
) -> io::Result<()> {
273-
std::fs::rename(from, to)
278+
if from_fd.is_none() && to_fd.is_none() {
279+
// TODO: Rust's implementation always overwrites the file so ensure consistency between
280+
// operating systems. We need to use windows-sys directly to distinguish between
281+
// os.rename and os.replace.
282+
std::fs::rename(from, to)
283+
} else {
284+
core::hint::cold_path();
285+
Err(io::Error::new(
286+
io::ErrorKind::Unsupported,
287+
"renameat is not available on this platform",
288+
))
289+
}
274290
}
275291

276-
#[cfg(windows)]
292+
#[cfg(unix)]
293+
pub fn rename(
294+
from: impl AsRef<std::path::Path>,
295+
from_fd: Option<crt_fd::Borrowed<'_>>,
296+
to: impl AsRef<std::path::Path>,
297+
to_fd: Option<crt_fd::Borrowed<'_>>,
298+
) -> io::Result<()> {
299+
let from = from.as_ref();
300+
let from_fd = from_fd.as_ref().map_or(rustix::fs::CWD, AsFd::as_fd);
301+
let to = to.as_ref();
302+
let to_fd = to_fd.as_ref().map_or(rustix::fs::CWD, AsFd::as_fd);
303+
rustix::fs::renameat(from_fd, from, to_fd, to).map_err(Into::into)
304+
}
305+
306+
#[cfg(not(unix))]
277307
pub fn seek_fd(
278308
fd: crt_fd::Borrowed<'_>,
279309
position: crt_fd::Offset,

‎crates/vm/src/stdlib/os.rs‎

Lines changed: 26 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -189,6 +189,7 @@ pub(super) mod _os {
189189
const UTIME_DIR_FD: bool = cfg!(not(any(windows, target_os = "redox")));
190190
pub(crate) const SYMLINK_DIR_FD: bool = cfg!(not(any(windows, target_os = "redox")));
191191
pub(crate) const UNLINK_DIR_FD: bool = cfg!(not(any(windows, target_os = "redox")));
192+
const RENAME_DIR_FD: bool = cfg!(unix);
192193
const RMDIR_DIR_FD: bool = cfg!(not(any(windows, target_os = "redox")));
193194
const SCANDIR_FD: bool = cfg!(all(unix, not(target_os = "redox")));
194195

@@ -1377,19 +1378,38 @@ pub(super) mod _os {
13771378
FsPath::try_from_path_like(path, false, vm)
13781379
}
13791380

1381+
#[derive(FromArgs)]
1382+
struct RenameArgs<'fd> {
1383+
#[pyarg(positional)]
1384+
src: PyObjectRef,
1385+
#[pyarg(positional)]
1386+
dst: PyObjectRef,
1387+
#[pyarg(any, default)]
1388+
src_dir_fd: OptionalArg<crt_fd::Borrowed<'fd>>,
1389+
#[pyarg(any, default)]
1390+
dst_dir_fd: OptionalArg<crt_fd::Borrowed<'fd>>,
1391+
}
1392+
1393+
#[cfg(not(target_arch = "wasm32"))]
13801394
#[pyfunction]
13811395
#[pyfunction(name = "replace")]
1382-
fn rename(src: PyObjectRef, dst: PyObjectRef, vm: &VirtualMachine) -> PyResult<()> {
1396+
fn rename(args: RenameArgs<'_>, vm: &VirtualMachine) -> PyResult<()> {
13831397
let src = PathConverter::new()
13841398
.function("rename")
13851399
.argument("src")
1386-
.try_path(src, vm)?;
1400+
.try_path(args.src, vm)?;
13871401
let dst = PathConverter::new()
13881402
.function("rename")
13891403
.argument("dst")
1390-
.try_path(dst, vm)?;
1404+
.try_path(args.dst, vm)?;
13911405

1392-
crate::host_env::os::rename(&src.path, &dst.path).map_err(|err| {
1406+
crate::host_env::os::rename(
1407+
&src,
1408+
args.src_dir_fd.into_option(),
1409+
&dst,
1410+
args.dst_dir_fd.into_option(),
1411+
)
1412+
.map_err(|err| {
13931413
let builder = err.to_os_error_builder(vm);
13941414
let builder = builder.filename(src.filename(vm));
13951415
let builder = builder.filename2(dst.filename(vm));
@@ -1932,8 +1952,8 @@ pub(super) mod _os {
19321952
SupportFunc::new("readlink", Some(false), None, Some(false)),
19331953
SupportFunc::new("remove", Some(false), Some(UNLINK_DIR_FD), Some(false)),
19341954
SupportFunc::new("unlink", Some(false), Some(UNLINK_DIR_FD), Some(false)),
1935-
SupportFunc::new("rename", Some(false), None, Some(false)),
1936-
SupportFunc::new("replace", Some(false), None, Some(false)), // TODO: Fix replace
1955+
SupportFunc::new("rename", Some(false), Some(RENAME_DIR_FD), Some(false)),
1956+
SupportFunc::new("replace", Some(false), Some(RENAME_DIR_FD), Some(false)), // TODO: Fix replace
19371957
SupportFunc::new("rmdir", Some(false), Some(RMDIR_DIR_FD), Some(false)),
19381958
SupportFunc::new("scandir", Some(SCANDIR_FD), Some(false), Some(false)),
19391959
SupportFunc::new("stat", Some(true), Some(STAT_DIR_FD), Some(true)),

0 commit comments

Comments
 (0)