From 99a6517203bc9fa908088fab1befcb5c3c9eb81e Mon Sep 17 00:00:00 2001 From: Josh Megnauth Date: Sun, 5 Jul 2026 21:25:37 -0400 Subject: [PATCH] host_env: os.replace for Windows `rename` and `replace` are the same for Unixes but different for Windows in Python. POSIX's `rename` atomically replaces its target; there is no `replace` in POSIX. `renameat2` with `RENAME_NOREPLACE` atomically checks if a file exists and renames if it doesn't, but Python's `rename` and `replace` predate `renameat2`. For Windows, `rename` acts like the `RENAME_NOREPLACE` flag while `replace` functions like `rename`. I implemented both using the Windows API. Both implementations follow CPython's code by using `MoveFileExW`. There are modern Windows APIs that may be worth using in the future. Rust's standard library prefers the modern APIs but falls back to `MoveFileExW` for deprecated Windows versions. --- crates/host_env/src/lib.rs | 2 + crates/host_env/src/os.rs | 37 -------- crates/host_env/src/posix.rs | 13 +-- crates/host_env/src/posix_unix_wasi.rs | 45 ++++++++++ crates/host_env/src/posix_wasi.rs | 11 +-- crates/host_env/src/posix_windows.rs | 83 ++++++++++++++++- crates/vm/src/stdlib/os.rs | 118 ++++++++++++++++++------- 7 files changed, 217 insertions(+), 92 deletions(-) create mode 100644 crates/host_env/src/posix_unix_wasi.rs diff --git a/crates/host_env/src/lib.rs b/crates/host_env/src/lib.rs index 15f816ea8f4..14c07ab5f5c 100644 --- a/crates/host_env/src/lib.rs +++ b/crates/host_env/src/lib.rs @@ -55,6 +55,8 @@ pub mod posix; #[cfg(windows)] #[path = "posix_windows.rs"] pub mod posix; +#[cfg(any(unix, target_os = "wasi"))] +pub mod posix_unix_wasi; #[cfg(unix)] pub mod pwd; #[cfg(unix)] diff --git a/crates/host_env/src/os.rs b/crates/host_env/src/os.rs index bd2a5acb906..7d11d7bbec8 100644 --- a/crates/host_env/src/os.rs +++ b/crates/host_env/src/os.rs @@ -9,8 +9,6 @@ use core::ffi::CStr; use core::str::Utf8Error; #[cfg(windows)] use core::time::Duration; -#[cfg(unix)] -use rustix::fd::AsFd; use std::{ env, ffi::{OsStr, OsString}, @@ -267,41 +265,6 @@ pub fn copy_file_range( rustix::fs::copy_file_range(src, offset_src, dst, offset_dst, count) } -#[cfg(not(unix))] -pub fn rename( - from: impl AsRef, - from_fd: Option>, - to: impl AsRef, - to_fd: Option>, -) -> io::Result<()> { - if from_fd.is_none() && to_fd.is_none() { - // TODO: Rust's implementation always overwrites the file so ensure consistency between - // operating systems. We need to use windows-sys directly to distinguish between - // os.rename and os.replace. - std::fs::rename(from, to) - } else { - core::hint::cold_path(); - Err(io::Error::new( - io::ErrorKind::Unsupported, - "renameat is not available on this platform", - )) - } -} - -#[cfg(unix)] -pub fn rename( - from: impl AsRef, - from_fd: Option>, - to: impl AsRef, - to_fd: Option>, -) -> io::Result<()> { - let from = from.as_ref(); - let from_fd = from_fd.as_ref().map_or(rustix::fs::CWD, AsFd::as_fd); - let to = to.as_ref(); - let to_fd = to_fd.as_ref().map_or(rustix::fs::CWD, AsFd::as_fd); - rustix::fs::renameat(from_fd, from, to_fd, to).map_err(Into::into) -} - #[cfg(windows)] pub fn seek_fd( fd: crt_fd::Borrowed<'_>, diff --git a/crates/host_env/src/posix.rs b/crates/host_env/src/posix.rs index 1239d3526c4..8b9beb9e5aa 100644 --- a/crates/host_env/src/posix.rs +++ b/crates/host_env/src/posix.rs @@ -10,7 +10,8 @@ use std::os::fd::FromRawFd; use std::os::fd::{AsFd, AsRawFd, BorrowedFd, IntoRawFd, OwnedFd}; use std::path::Path; -use crate::crt_fd; +pub use super::posix_unix_wasi::*; +pub use rustix::fs::RawMode; pub struct UnameInfo { pub sysname: String, @@ -176,16 +177,6 @@ pub fn fcopyfile(in_fd: i32, out_fd: i32, flags: u32) -> std::io::Result<()> { } } -#[cfg(any(unix, target_os = "wasi"))] -pub fn make_dir( - dir_fd: Option>, - path: &impl AsRef, - mode: libc::mode_t, -) -> std::io::Result<()> { - let dir_fd = dir_fd.as_ref().map_or(rustix::fs::CWD, AsFd::as_fd); - rustix::fs::mkdirat(dir_fd, path.as_ref(), mode.into()).map_err(Into::into) -} - #[cfg(unix)] pub fn link_paths(src: &CStr, dst: &CStr, follow_symlinks: bool) -> std::io::Result<()> { let flags = if follow_symlinks { diff --git a/crates/host_env/src/posix_unix_wasi.rs b/crates/host_env/src/posix_unix_wasi.rs new file mode 100644 index 00000000000..98698798b54 --- /dev/null +++ b/crates/host_env/src/posix_unix_wasi.rs @@ -0,0 +1,45 @@ +//! Common POSIX implementations across Unix and WASI. + +use std::{io, path::Path}; + +use rustix::{fd::AsFd, fs}; + +use crate::crt_fd; + +/// https://pubs.opengroup.org/onlinepubs/9799919799/functions/mkdir.html +pub fn make_dir( + dir_fd: Option>, + path: &impl AsRef, + mode: fs::RawMode, +) -> io::Result<()> { + let dir_fd = dir_fd.as_ref().map_or(fs::CWD, AsFd::as_fd); + fs::mkdirat(dir_fd, path.as_ref(), mode.into()).map_err(Into::into) +} + +/// https://pubs.opengroup.org/onlinepubs/9799919799/functions/rename.html +pub fn rename( + from: impl AsRef, + from_fd: Option>, + to: impl AsRef, + to_fd: Option>, +) -> io::Result<()> { + let from = from.as_ref(); + let from_fd = from_fd.as_ref().map_or(fs::CWD, AsFd::as_fd); + let to = to.as_ref(); + let to_fd = to_fd.as_ref().map_or(fs::CWD, AsFd::as_fd); + fs::renameat(from_fd, from, to_fd, to).map_err(Into::into) +} + +/// https://docs.python.org/3/library/os.html#os.replace +/// +/// Atomically replace `to` with `from`. +/// POSIX's rename already atomically replaces targets, so this function just forwards to [`rename`]. +#[inline] +pub fn replace( + from: impl AsRef, + from_fd: Option>, + to: impl AsRef, + to_fd: Option>, +) -> io::Result<()> { + rename(from, from_fd, to, to_fd) +} diff --git a/crates/host_env/src/posix_wasi.rs b/crates/host_env/src/posix_wasi.rs index f1883fccd58..458e0c85d4d 100644 --- a/crates/host_env/src/posix_wasi.rs +++ b/crates/host_env/src/posix_wasi.rs @@ -3,16 +3,9 @@ use core::{ffi::CStr, time::Duration}; use rustix::fd::AsFd; use std::{ffi::OsStr, io, path::Path}; -use crate::{crt_fd, os::CheckLibcResult}; +pub use super::posix_unix_wasi::*; -pub fn make_dir( - dir_fd: Option>, - path: &impl AsRef, - mode: libc::mode_t, -) -> std::io::Result<()> { - let dir_fd = dir_fd.as_ref().map_or(rustix::fs::CWD, AsFd::as_fd); - rustix::fs::mkdirat(dir_fd, path.as_ref(), mode.into()).map_err(Into::into) -} +use crate::{crt_fd, os::CheckLibcResult}; pub fn remove_dir_at(dir_fd: i32, path: &CStr) -> io::Result<()> { unsafe { libc::unlinkat(dir_fd, path.as_ptr(), libc::AT_REMOVEDIR) }.check_libc_neg()?; diff --git a/crates/host_env/src/posix_windows.rs b/crates/host_env/src/posix_windows.rs index ac9e2cfa808..ac92ba66ac7 100644 --- a/crates/host_env/src/posix_windows.rs +++ b/crates/host_env/src/posix_windows.rs @@ -4,19 +4,96 @@ //! these syscalls, but they can be emulated with a mix of the Windows API and the Rust standard //! library, the latter of which calls the former. +use core::hint::cold_path; use std::{fs, io, path::Path}; +use widestring::WideCString; +use windows_sys::Win32::{ + Foundation::FALSE, + Storage::FileSystem::{MOVE_FILE_FLAGS, MOVEFILE_REPLACE_EXISTING, MoveFileExW}, +}; + use crate::crt_fd; -#[expect(non_camel_case_types)] -pub type mode_t = u32; +pub type RawMode = u32; pub fn make_dir( dir_fd: Option>, path: &impl AsRef, - _mode: mode_t, + _mode: RawMode, ) -> io::Result<()> { debug_assert!(dir_fd.is_none()); // TODO: On Windows, Python has an override if the mode is 0o700 fs::create_dir(path) } + +/// https://pubs.opengroup.org/onlinepubs/9799919799/functions/rename.html +#[inline] +pub fn rename( + from: impl AsRef, + #[cfg_attr(not(debug_assertions), expect(unused_variables))] from_fd: Option< + crt_fd::Borrowed<'_>, + >, + to: impl AsRef, + #[cfg_attr(not(debug_assertions), expect(unused_variables))] to_fd: Option< + crt_fd::Borrowed<'_>, + >, +) -> io::Result<()> { + debug_assert!(from_fd.is_none()); + debug_assert!(to_fd.is_none()); + + rename_impl(from, to, 0) +} + +/// https://docs.python.org/3/library/os.html#os.replace +/// +/// Atomically replace `to` with `from`. +#[inline] +pub fn replace( + from: impl AsRef, + #[cfg_attr(not(debug_assertions), expect(unused_variables))] from_fd: Option< + crt_fd::Borrowed<'_>, + >, + to: impl AsRef, + #[cfg_attr(not(debug_assertions), expect(unused_variables))] to_fd: Option< + crt_fd::Borrowed<'_>, + >, +) -> io::Result<()> { + debug_assert!(from_fd.is_none()); + debug_assert!(to_fd.is_none()); + + rename_impl(from, to, MOVEFILE_REPLACE_EXISTING) +} + +fn rename_impl( + from: impl AsRef, + to: impl AsRef, + flags: MOVE_FILE_FLAGS, +) -> io::Result<()> { + let from = WideCString::from_os_str(from.as_ref()) + .map_err(io::Error::other)? + .into_vec_with_nul(); + let to = WideCString::from_os_str(to.as_ref()) + .map_err(io::Error::other)? + .into_vec_with_nul(); + + // SAFETY: + // * from and to are NUL terminated wide strings + let success = unsafe { + // Rust's [`std::fs::rename`] is more complicated than CPython's. Rust attempts to use modern APIs + // where available, such as `FileRenameInfoEx`, which better map to POSIX. CPython simply + // calls MoveFileExW so we'll do that for parity. However, it may be better to use the new + // APIs and fall back if possible, especially if they're faster. + // + // Unlike POSIX's rename, MoveFileExW does not automatically move between volumes. + // This is expected behavior in CPython. + MoveFileExW(from.as_ptr(), to.as_ptr(), flags) + }; + + if success != FALSE { + Ok(()) + } else { + cold_path(); + Err(io::Error::last_os_error()) + } +} diff --git a/crates/vm/src/stdlib/os.rs b/crates/vm/src/stdlib/os.rs index 94dd19f79d6..267ee69be20 100644 --- a/crates/vm/src/stdlib/os.rs +++ b/crates/vm/src/stdlib/os.rs @@ -6,16 +6,11 @@ use crate::{ builtins::{PyModule, PySet}, convert::{IntoPyException, ToPyException, ToPyObject}, function::{ArgumentError, FromArgs, FuncArgs}, - host_env::crt_fd, + host_env::{crt_fd, posix::RawMode}, }; +use core::marker::PhantomData; use std::{io, path::Path}; -#[cfg(not(windows))] -use libc::mode_t; - -#[cfg(windows)] -use crate::host_env::posix::mode_t; - pub(crate) fn fs_metadata>( path: P, follow_symlink: bool, @@ -45,19 +40,44 @@ cfg_select! { const DEFAULT_DIR_FD: crt_fd::Borrowed<'static> = unsafe { crt_fd::Borrowed::borrow_raw(AT_FDCWD) }; +pub trait DirFdKeyword: Clone + Copy + Eq + PartialEq { + const NAME: &'static str; +} + +#[derive(Clone, Copy, Eq, PartialEq)] +pub struct DefaultDirFd; +impl DirFdKeyword for DefaultDirFd { + const NAME: &'static str = "dir_fd"; +} + +#[derive(Clone, Copy, Eq, PartialEq)] +pub struct SrcDirFd; +impl DirFdKeyword for SrcDirFd { + const NAME: &'static str = "src_dir_fd"; +} + +#[derive(Clone, Copy, Eq, PartialEq)] +pub struct DstDirFd; +impl DirFdKeyword for DstDirFd { + const NAME: &'static str = "dst_dir_fd"; +} + // XXX: AVAILABLE should be a bool, but we can't yet have it as a bool and just cast it to usize -#[derive(Copy, Clone, PartialEq, Eq)] -pub struct DirFd<'fd, const AVAILABLE: usize>(pub(crate) [crt_fd::Borrowed<'fd>; AVAILABLE]); +#[derive(Clone, Copy, Eq, PartialEq)] +pub struct DirFd<'fd, const AVAILABLE: usize, KW: DirFdKeyword = DefaultDirFd>( + pub(crate) [crt_fd::Borrowed<'fd>; AVAILABLE], + PhantomData, +); -impl Default for DirFd<'_, AVAILABLE> { +impl Default for DirFd<'_, AVAILABLE, KW> { fn default() -> Self { - Self([DEFAULT_DIR_FD; AVAILABLE]) + Self([DEFAULT_DIR_FD; AVAILABLE], PhantomData) } } // not used on all platforms #[allow(unused)] -impl<'fd> DirFd<'fd, 1> { +impl<'fd, KW: DirFdKeyword> DirFd<'fd, 1, KW> { #[inline(always)] pub(crate) fn get_opt(self) -> Option> { let [fd] = self.0; @@ -76,9 +96,9 @@ impl<'fd> DirFd<'fd, 1> { } } -impl FromArgs for DirFd<'_, AVAILABLE> { +impl FromArgs for DirFd<'_, AVAILABLE, KW> { fn from_args(vm: &VirtualMachine, args: &mut FuncArgs) -> Result { - let fd = match args.take_keyword("dir_fd") { + let fd = match args.take_keyword(KW::NAME) { Some(o) if vm.is_none(&o) => Ok(DEFAULT_DIR_FD), None => Ok(DEFAULT_DIR_FD), Some(o) => { @@ -99,7 +119,7 @@ impl FromArgs for DirFd<'_, AVAILABLE> { .into()); } let fd = fd.map_err(|e| e.to_pyexception(vm))?; - Ok(Self([fd; AVAILABLE])) + Ok(Self([fd; AVAILABLE], PhantomData)) } } @@ -160,7 +180,7 @@ impl ToPyObject for crt_fd::Borrowed<'_> { #[pymodule(sub)] pub(super) mod _os { - use super::{DirFd, FollowSymlinks, SupportFunc, mode_t}; + use super::{DirFd, DstDirFd, FollowSymlinks, RawMode, SrcDirFd, SupportFunc}; use crate::host_env::fileutils::StatStruct; #[cfg(any(unix, windows))] use crate::utils::ToCString; @@ -180,7 +200,7 @@ pub(super) mod _os { types::{Destructor, IterNext, Iterable, PyStructSequence, Representable, SelfIter}, vm::VirtualMachine, }; - use core::time::Duration; + use core::{marker::PhantomData, time::Duration}; use crossbeam_utils::atomic::AtomicCell; use rustpython_common::wtf8::Wtf8Buf; #[cfg(windows)] @@ -195,7 +215,7 @@ pub(super) mod _os { const UTIME_DIR_FD: bool = cfg!(not(any(windows, target_os = "redox"))); pub(crate) const SYMLINK_DIR_FD: bool = cfg!(not(any(windows, target_os = "redox"))); pub(crate) const UNLINK_DIR_FD: bool = cfg!(not(any(windows, target_os = "redox"))); - const RENAME_DIR_FD: bool = cfg!(unix); + const RENAME_DIR_FD: bool = cfg!(any(unix, target_os = "wasi")); const RMDIR_DIR_FD: bool = cfg!(not(any(windows, target_os = "redox"))); const SCANDIR_FD: bool = cfg!(all(unix, not(target_os = "redox"))); @@ -347,7 +367,7 @@ pub(super) mod _os { #[pyfunction] fn mkdir( path: OsPath, - mode: OptionalArg, + mode: OptionalArg, #[cfg_attr(not(any(unix, target_os = "wasi")), expect(unused_variables))] dir_fd: DirFd< '_, { MKDIR_DIR_FD as usize }, @@ -625,7 +645,7 @@ pub(super) mod _os { // Safety: the fd came from os.open() and is borrowed for // the lifetime of this DirEntry reference. let borrowed = unsafe { crt_fd::Borrowed::borrow_raw(raw_fd) }; - return DirFd([borrowed; STAT_DIR_FD as usize]); + return DirFd([borrowed; STAT_DIR_FD as usize], PhantomData); } DirFd::default() } @@ -1382,14 +1402,15 @@ pub(super) mod _os { src: PyObjectRef, #[pyarg(positional)] dst: PyObjectRef, - #[pyarg(any, default)] - src_dir_fd: OptionalArg>, - #[pyarg(any, default)] - dst_dir_fd: OptionalArg>, + #[pyarg(flatten)] + #[cfg_attr(not(any(unix, target_os = "wasi")), expect(unused_variables))] + src_dir_fd: DirFd<'fd, { RENAME_DIR_FD as usize }, SrcDirFd>, + #[pyarg(flatten)] + #[cfg_attr(not(any(unix, target_os = "wasi")), expect(unused_variables))] + dst_dir_fd: DirFd<'fd, { RENAME_DIR_FD as usize }, DstDirFd>, } #[pyfunction] - #[pyfunction(name = "replace")] fn rename(args: RenameArgs<'_>, vm: &VirtualMachine) -> PyResult<()> { let src = PathConverter::new() .function("rename") @@ -1400,13 +1421,46 @@ pub(super) mod _os { .argument("dst") .try_path(args.dst, vm)?; - crate::host_env::os::rename( - &src, - args.src_dir_fd.into_option(), - &dst, - args.dst_dir_fd.into_option(), - ) - .map_err(|err| { + #[cfg(any(unix, target_os = "wasi"))] + let src_dir_fd = args.src_dir_fd.get_opt(); + #[cfg(not(any(unix, target_os = "wasi")))] + let src_dir_fd = None; + + #[cfg(any(unix, target_os = "wasi"))] + let dst_dir_fd = args.dst_dir_fd.get_opt(); + #[cfg(not(any(unix, target_os = "wasi")))] + let dst_dir_fd = None; + + crate::host_env::posix::rename(&src, src_dir_fd, &dst, dst_dir_fd).map_err(|err| { + let builder = err.to_os_error_builder(vm); + let builder = builder.filename(src.filename(vm)); + let builder = builder.filename2(dst.filename(vm)); + builder.build(vm).upcast() + }) + } + + #[pyfunction] + fn replace(args: RenameArgs<'_>, vm: &VirtualMachine) -> PyResult<()> { + let src = PathConverter::new() + .function("replace") + .argument("src") + .try_path(args.src, vm)?; + let dst = PathConverter::new() + .function("replace") + .argument("dst") + .try_path(args.dst, vm)?; + + #[cfg(any(unix, target_os = "wasi"))] + let src_dir_fd = args.src_dir_fd.get_opt(); + #[cfg(not(any(unix, target_os = "wasi")))] + let src_dir_fd = None; + + #[cfg(any(unix, target_os = "wasi"))] + let dst_dir_fd = args.dst_dir_fd.get_opt(); + #[cfg(not(any(unix, target_os = "wasi")))] + let dst_dir_fd = None; + + crate::host_env::posix::replace(&src, src_dir_fd, &dst, dst_dir_fd).map_err(|err| { let builder = err.to_os_error_builder(vm); let builder = builder.filename(src.filename(vm)); let builder = builder.filename2(dst.filename(vm));