host_env: os.replace for Windows (#8212)

`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

@@ -56,6 +56,8 @@ pub mod posix;
 #[cfg(windows)]
 #[path = "posix_windows.rs"]
 pub mod posix;
+#[cfg(any(unix, target_os = "wasi"))]
+pub mod posix_unix_like;
 #[cfg(unix)]
 pub mod pwd;
 #[cfg(unix)]

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<std::path::Path>,
-    from_fd: Option<crt_fd::Borrowed<'_>>,
-    to: impl AsRef<std::path::Path>,
-    to_fd: Option<crt_fd::Borrowed<'_>>,
-) -> 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<std::path::Path>,
-    from_fd: Option<crt_fd::Borrowed<'_>>,
-    to: impl AsRef<std::path::Path>,
-    to_fd: Option<crt_fd::Borrowed<'_>>,
-) -> 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<'_>,

crates/host_env/src/posix.rs

@@ -10,7 +10,7 @@ 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_like::*;
 
 pub use libc::{c_char, pid_t};
 
@@ -178,16 +178,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<crt_fd::Borrowed<'_>>,
-    path: &impl AsRef<Path>,
-    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 {

crates/host_env/src/posix_unix_like.rs

@@ -0,0 +1,47 @@
+//! Common POSIX implementations across Unix-likes.
+
+use std::{io, path::Path};
+
+use rustix::{fd::AsFd, fs};
+
+pub use rustix::fs::RawMode;
+
+use crate::crt_fd;
+
+/// https://pubs.opengroup.org/onlinepubs/9799919799/functions/mkdir.html
+pub fn make_dir(
+    dir_fd: Option<crt_fd::Borrowed<'_>>,
+    path: impl AsRef<Path>,
+    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<Path>,
+    from_fd: Option<crt_fd::Borrowed<'_>>,
+    to: impl AsRef<Path>,
+    to_fd: Option<crt_fd::Borrowed<'_>>,
+) -> 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<Path>,
+    from_fd: Option<crt_fd::Borrowed<'_>>,
+    to: impl AsRef<Path>,
+    to_fd: Option<crt_fd::Borrowed<'_>>,
+) -> io::Result<()> {
+    rename(from, from_fd, to, to_fd)
+}

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_like::*;
 
-pub fn make_dir(
-    dir_fd: Option<crt_fd::Borrowed<'_>>,
-    path: &impl AsRef<Path>,
-    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()?;

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<crt_fd::Borrowed<'_>>,
-    path: &impl AsRef<Path>,
-    _mode: mode_t,
+    path: impl AsRef<Path>,
+    _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<Path>,
+    #[cfg_attr(not(debug_assertions), expect(unused_variables))] from_fd: Option<
+        crt_fd::Borrowed<'_>,
+    >,
+    to: impl AsRef<Path>,
+    #[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<Path>,
+    #[cfg_attr(not(debug_assertions), expect(unused_variables))] from_fd: Option<
+        crt_fd::Borrowed<'_>,
+    >,
+    to: impl AsRef<Path>,
+    #[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<Path>,
+    to: impl AsRef<Path>,
+    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())
+    }
+}

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<P: AsRef<Path>>(
     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) };
 
-// 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]);
+pub trait DirFdKeyword: Clone + Copy + Eq + PartialEq {
+    const NAME: &'static str;
+}
 
-impl<const AVAILABLE: usize> Default for DirFd<'_, AVAILABLE> {
+#[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(Clone, Copy, Eq, PartialEq)]
+pub struct DirFd<'fd, const AVAILABLE: usize, KW: DirFdKeyword = DefaultDirFd>(
+    pub(crate) [crt_fd::Borrowed<'fd>; AVAILABLE],
+    PhantomData<KW>,
+);
+
+impl<const AVAILABLE: usize, KW: DirFdKeyword> 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<crt_fd::Borrowed<'fd>> {
         let [fd] = self.0;
@@ -76,9 +96,9 @@ impl<'fd> DirFd<'fd, 1> {
     }
 }
 
-impl<const AVAILABLE: usize> FromArgs for DirFd<'_, AVAILABLE> {
+impl<const AVAILABLE: usize, KW: DirFdKeyword> FromArgs for DirFd<'_, AVAILABLE, KW> {
     fn from_args(vm: &VirtualMachine, args: &mut FuncArgs) -> Result<Self, ArgumentError> {
-        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<const AVAILABLE: usize> 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,6 +200,8 @@ pub(super) mod _os {
         types::{Destructor, IterNext, Iterable, PyStructSequence, Representable, SelfIter},
         vm::VirtualMachine,
     };
+    #[cfg(not(windows))]
+    use core::marker::PhantomData;
     use core::time::Duration;
     use crossbeam_utils::atomic::AtomicCell;
     use rustpython_common::wtf8::Wtf8Buf;
@@ -195,7 +217,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 +369,7 @@ pub(super) mod _os {
     #[pyfunction]
     fn mkdir(
         path: OsPath,
-        mode: OptionalArg<mode_t>,
+        mode: OptionalArg<RawMode>,
         #[cfg_attr(not(any(unix, target_os = "wasi")), expect(unused_variables))] dir_fd: DirFd<
             '_,
             { MKDIR_DIR_FD as usize },
@@ -625,7 +647,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 +1404,15 @@ pub(super) mod _os {
         src: PyObjectRef,
         #[pyarg(positional)]
         dst: PyObjectRef,
-        #[pyarg(any, default)]
-        src_dir_fd: OptionalArg<crt_fd::Borrowed<'fd>>,
-        #[pyarg(any, default)]
-        dst_dir_fd: OptionalArg<crt_fd::Borrowed<'fd>>,
+        #[pyarg(flatten)]
+        #[cfg_attr(not(any(unix, target_os = "wasi")), expect(dead_code))]
+        src_dir_fd: DirFd<'fd, { RENAME_DIR_FD as usize }, SrcDirFd>,
+        #[pyarg(flatten)]
+        #[cfg_attr(not(any(unix, target_os = "wasi")), expect(dead_code))]
+        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 +1423,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));