std: add fs::rename_noreplace

[valentynkit]

View all comments

Tracking issue: #161427
Accepted ACP: rust-lang/libs-team#131

Summary

Add fs::rename_noreplace. Unlike fs::rename, the destination is never overwritten, instead returns ErrorKind::AlreadyExists if to already exists.

Platform-specific APIs used:

• renameat2 with RENAME_NOREPLACE flag on Linux and Android
• renamex_np with RENAME_EXCL on Apple
• MoveFileExW without the MOVEFILE_REPLACE_EXISTING flag on Windows. Extracted functionality into rename_inner to avoid duplication for rename and rename_noreplace
• On other Unix platforms and on WASI, and on Linux, Android and Apple when the kernel or the filesystem doesn't support it, link followed by unlink is used instead.
• Platforms with neither return Unsupported.

Linux and Android case, has different error handling than Apple because the platforms report these conditions differently. On Linux the syscall may not exist at all (ENOSYS), and EINVAL covers "filesystem doesn't support the flag" and flag misuse (incorrect combinations of flags, as example). On Apple renamex_np is called directly, and Darwin use ENOTSUP for not supported case, and keeps EINVAL only for misuse combinations.

EINVAL An invalid flag was specified in flags.
EINVAL Both RENAME_NOREPLACE and RENAME_EXCHANGE were specified in flags.
EINVAL The filesystem does not support one of the flags in flags.
renameat2

Open questions

1. ENOSYS on Linux and Android could be cached, unlike EINVAL case which on the same machine, when using different filesystem may return different result. In this PR I deliberately avoided adding caching to avoid premature optimization, but could add it if you think it worth it.
2. The link + unlink fallback can't move directories: link returns EPERM. Should it report Unsupported there instead?

r? libs

[rustbot]

Reminder, once the PR becomes ready for a review, use @rustbot ready.

[joshtriplett]

Added one suggestion to improve the documentation comment.

With that suggestion applied, r=me.

[joshtriplett]

@bors delegate+

[rust-bors]

✌️ @valentynkit, you can now approve this pull request!

If @joshtriplett told you to "r=me" after making some further change, then please make that change and post @bors r=joshtriplett.

View changes since this delegation.

[rustbot]

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

[valentynkit]

@rustbot ready
CI failed because #162270 removed the MOVE_FILE_FLAGS alias. Rebased and switched the Windows code to use plain u32.

Since this change wasn't part of the requested suggestion, I'd rather not approve it myself using the delegation, even so it's straightforward change. Would you mind taking a quick look?

[Mark-Simulacrum]

@bors r=joshtriplett,Mark-Simulacrum

[rust-bors]

📌 Commit 7b7ed95 has been approved by joshtriplett,Mark-Simulacrum

It is now in the queue for this repository.

[jhpratt]

@bors r- #163729 (comment)

failures:

---- fs::tests::test_rename_noreplace_directory stdout ----

thread 'fs::tests::test_rename_noreplace_directory' (10893) panicked at library/std/src/fs/tests.rs:2739:54:
called `Result::unwrap()` on an `Err` value: Os { code: 1, kind: PermissionDenied, message: "Operation not permitted" }
---- fs::tests::test_rename_noreplace_directory stdout end ----

failures:
    fs::tests::test_rename_noreplace_directory

test result: FAILED. 429 passed; 1 failed; 28 ignored; 0 measured; 0 filtered out; finished in 302.65s

[rust-bors]

This pull request was unapproved.

This PR was contained in a rollup (#163729), which was unapproved.

View changes since this unapproval

[valentynkit]

failures:

---- fs::tests::test_rename_noreplace_directory stdout ----
thread 'fs::tests::test_rename_noreplace_directory' (10893) panicked at library/std/src/fs/tests.rs:2739:54:
called ``Result::unwrap()`` on an ``Err`` value: Os { code: 1, kind: PermissionDenied, message: "Operation not permitted" }

Sorry, my bad for not catching this during implementation.
After going through it, it looks like the problem in the test itself test_rename_noreplace_directory assumed that gated platforms will always use the native rename no replace, but older version like Linux before 3.15 fall back to implementation that use link/unlink, which doesn't support directory. This limitation is documented in implementation and this PR body, but I didn't handle the fallback path in the test.

Test was updated to support fallback approach.

[valentynkit]

@bors try jobs=test-arm-android
Running the failed job, looks like it's only included during the rollup.

[rust-bors]

☀️ Try build successful (CI)
Build commit: 94dce72 (94dce72406d558b25c6b2761ebac5fc850af9e18)
Base parent: a639ea0 (a639ea0890c0977b1c45596fd6b5f9a70a1d67da)

[valentynkit]

@rustbot ready