Skip to content

Windows: fix Dir::remove and Dir::rename following symlinks - #164559

Merged
rust-bors[bot] merged 2 commits into
rust-lang:mainfrom
RalfJung:dir-remove-rename-symlinks
Nov 10, 2026
Merged

rust-bors[bot] merged 2 commits into
rust-lang:mainfrom
RalfJung:dir-remove-rename-symlinks

Conversation

@RalfJung

@RalfJung RalfJung commented Oct 19, 2026 •

Copy link
Copy Markdown
Member

View all comments

Windows fs::Dir used to follow symlinks for operations that should instead act on the symlink itself. This fixes that.
Fixes #164249

try-job: test-x86_64-msvc*
Cc @ChrisDenton

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Oct 19, 2026
@RalfJung

Copy link
Copy Markdown
Member Author

@bors try

@rust-bors

This comment has been minimized.

rust-bors Bot pushed a commit that referenced this pull request Oct 19, 2026
ensure Dir::remove and Dir::rename do not follow symlinks

try-job: test-x86_64-msvc*
@RalfJung
RalfJung force-pushed the dir-remove-rename-symlinks branch from 19f0348 to 3cbd1f6 Compare October 19, 2026 07:47
@rust-log-analyzer

This comment has been minimized.

@rust-bors

rust-bors Bot commented Oct 19, 2026

Copy link
Copy Markdown
Contributor

💔 Test for cad40d1 failed: CI. Failed job:

@RalfJung

Copy link
Copy Markdown
Member Author

rename also follows symlinks so that'll also need to be fixed.

@RalfJung
RalfJung force-pushed the dir-remove-rename-symlinks branch from 3cbd1f6 to 085caae Compare October 19, 2026 10:54
@RalfJung

Copy link
Copy Markdown
Member Author

@bors try

@rust-bors

This comment has been minimized.

rust-bors Bot pushed a commit that referenced this pull request Oct 19, 2026
ensure Dir::remove and Dir::rename do not follow symlinks

try-job: test-x86_64-msvc*
@RalfJung RalfJung changed the title ensure Dir::remove and Dir::rename do not follow symlinks Windows: fix Dir::remove and Dir::rename following symlinks Oct 19, 2026
@rust-bors

rust-bors Bot commented Oct 19, 2026

Copy link
Copy Markdown
Contributor

☀️ Try build successful (CI)
Build commit: 5c70812 (5c70812ef7d94883981d8ea671e5cebcf84e1e29)
Base parent: 588c7e9 (588c7e92f931498b1552ee64cad77fb0344cabfa)

@RalfJung
RalfJung marked this pull request as ready for review October 19, 2026 14:13
@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 19, 2026
@rustbot rustbot removed the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Oct 19, 2026
@rustbot

rustbot commented Oct 19, 2026

Copy link
Copy Markdown
Collaborator

r? @Mark-Simulacrum

rustbot has assigned @Mark-Simulacrum.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

  • Owners of files modified in this PR: @ChrisDenton, libs
  • @ChrisDenton, libs expanded to 13 candidates

Comment thread library/std/src/fs/tests.rs Resolved
@RalfJung
RalfJung force-pushed the dir-remove-rename-symlinks branch from 085caae to 84800e1 Compare October 20, 2026 06:10
@RalfJung

Copy link
Copy Markdown
Member Author

r? @ChrisDenton

@RalfJung

RalfJung commented Nov 7, 2026

Copy link
Copy Markdown
Member Author

@ChrisDenton this is a fairly small PR that only touches unstable APIs, so hopefully should not be too hard to review. It's now been waiting for 3 weeks. Happy to assign to another reviewer if there is someone else on the team with sufficient Windows knowledge. :)

let tmpdir = tmpdir();
let mut f = check!(File::create(tmpdir.join("foo.txt")));
check!(f.write(b"bar"));
check!(f.flush());

@ChrisDenton ChrisDenton Nov 10, 2026 •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why was this removed? Or why was it there before?

View changes since the review

@RalfJung RalfJung Nov 10, 2026 •

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed it because it seemed unnecessary and I was touching this test anyway. There are a couple of flush in this file. The oldest one was added by you in c6f7aa0; #150679 likely just copied that then.

EDIT: I undid this part of the diff. These flush should IMO have comments as they look quite odd, but that is orthogonal to this PR.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Specifically File::flush is even documented to be a NOP.

Since a File structure doesn’t contain any buffers, this function is currently a no-op on Unix and Windows. Note that this may change in the future.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might have been to ensure the metadata is updated before we reopen the file.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

flush is not sync_all though.
I made this a separate PR: #165739.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I was speculating as to the intent, not the effect.

@RalfJung
RalfJung force-pushed the dir-remove-rename-symlinks branch from 84800e1 to 811ebf7 Compare November 10, 2026 07:47
@rustbot rustbot added the A-filesystem Area: `std::fs` label Nov 10, 2026
@ChrisDenton

Copy link
Copy Markdown
Member

@bors r+ rollup

@rust-bors

rust-bors Bot commented Nov 10, 2026

Copy link
Copy Markdown
Contributor

📌 Commit 811ebf7 has been tentatively approved by ChrisDenton

It will be put into the queue for this repository once PR CI succeeds.

@rust-bors rust-bors Bot added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 10, 2026
JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request Nov 10, 2026
…s, r=ChrisDenton

Windows: fix Dir::remove and Dir::rename following symlinks

Windows `fs::Dir` used to follow symlinks for operations that should instead act on the symlink itself. This fixes that.
Fixes rust-lang#164249

try-job: test-x86_64-msvc*
Cc @ChrisDenton
rust-bors Bot pushed a commit that referenced this pull request Nov 10, 2026
…uwer

Rollup of 16 pull requests

Successful merges:

 - #165575 (Upgrade bundled musl to 1.2.6)
 - #163992 (add `core::arch::{x86, x86_64}::x87_f80`)
 - #165117 (std: use skip_unsupported_miri_tests to control when tests should be skipped in Miri due to missing shims)
 - #165458 (fix incorrect byte count suffix on MSVC)
 - #165487 (Migrate built-in macros `pattern_type` & `view_type` to macros 2.0 that expand to internal syntax)
 - #165490 (Mark the compile unit for `-Zdebuginfo-for-profiling`)
 - #165586 (Remove `backend_optimization_level`.)
 - #165646 (fix NumScalableVectors::from_field_count)
 - #165655 (Unignore passing GCC backend tests)
 - #165740 (miri subtree update)
 - #164559 (Windows: fix Dir::remove and Dir::rename following symlinks)
 - #165682 (interpret: ensure newly created unions have uninit in their padding)
 - #165693 (Move `check_attr` to attribute parsing)
 - #165715 (windows: check for null before wide encoding)
 - #165717 (mpsc_sync::stream_send_recv_stress: wait for all threads to finish)
 - #165733 (docs(num): remove needless whitespaces)

Failed merges:

 - #165727 (Normalize "beta release channel" in `const-supertrait-unstable` test)
 - #153261 (Partially stabilize `ptr_alignment_type` as `alignment_type`)
 - #164218 (Tweak private re-export errors to point at correct span and have more context)
 - #165709 (remove 'x miri' smoke test from PR CI)
JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request Nov 10, 2026
…s, r=ChrisDenton

Windows: fix Dir::remove and Dir::rename following symlinks

Windows `fs::Dir` used to follow symlinks for operations that should instead act on the symlink itself. This fixes that.
Fixes rust-lang#164249

try-job: test-x86_64-msvc*
Cc @ChrisDenton
rust-bors Bot pushed a commit that referenced this pull request Nov 10, 2026
…uwer

Rollup of 16 pull requests

Successful merges:

 - #165575 (Upgrade bundled musl to 1.2.6)
 - #163992 (add `core::arch::{x86, x86_64}::x87_f80`)
 - #165117 (std: use skip_unsupported_miri_tests to control when tests should be skipped in Miri due to missing shims)
 - #165458 (fix incorrect byte count suffix on MSVC)
 - #165490 (Mark the compile unit for `-Zdebuginfo-for-profiling`)
 - #165586 (Remove `backend_optimization_level`.)
 - #165646 (fix NumScalableVectors::from_field_count)
 - #165655 (Unignore passing GCC backend tests)
 - #165740 (miri subtree update)
 - #153261 (Partially stabilize `ptr_alignment_type` as `alignment_type`)
 - #164559 (Windows: fix Dir::remove and Dir::rename following symlinks)
 - #165682 (interpret: ensure newly created unions have uninit in their padding)
 - #165693 (Move `check_attr` to attribute parsing)
 - #165715 (windows: check for null before wide encoding)
 - #165717 (mpsc_sync::stream_send_recv_stress: wait for all threads to finish)
 - #165733 (docs(num): remove needless whitespaces)

Failed merges:

 - #165727 (Normalize "beta release channel" in `const-supertrait-unstable` test)
 - #164218 (Tweak private re-export errors to point at correct span and have more context)
 - #165709 (remove 'x miri' smoke test from PR CI)
JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request Nov 10, 2026
…s, r=ChrisDenton

Windows: fix Dir::remove and Dir::rename following symlinks

Windows `fs::Dir` used to follow symlinks for operations that should instead act on the symlink itself. This fixes that.
Fixes rust-lang#164249

try-job: test-x86_64-msvc*
Cc @ChrisDenton
JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request Nov 10, 2026
…s, r=ChrisDenton

Windows: fix Dir::remove and Dir::rename following symlinks

Windows `fs::Dir` used to follow symlinks for operations that should instead act on the symlink itself. This fixes that.
Fixes rust-lang#164249

try-job: test-x86_64-msvc*
Cc @ChrisDenton
rust-bors Bot pushed a commit that referenced this pull request Nov 10, 2026
…uwer

Rollup of 24 pull requests

Successful merges:

 - #165709 (remove 'x miri' smoke test from PR CI)
 - #164217 (Prepare to make `rustc_trivial_field_reads` usable in user code)
 - #165569 (`Move` trait feature gate)
 - #165575 (Upgrade bundled musl to 1.2.6)
 - #163241 (Add enclosing item context when pointing at `FnMut` bound)
 - #163992 (add `core::arch::{x86, x86_64}::x87_f80`)
 - #165047 (Change the span building of paths from first-to-last to chained)
 - #165458 (fix incorrect byte count suffix on MSVC)
 - #165490 (Mark the compile unit for `-Zdebuginfo-for-profiling`)
 - #165586 (Remove `backend_optimization_level`.)
 - #165646 (fix NumScalableVectors::from_field_count)
 - #165655 (Unignore passing GCC backend tests)
 - #165740 (miri subtree update)
 - #165753 (avoid Terminator:None in MIR inliner)
 - #153261 (Partially stabilize `ptr_alignment_type` as `alignment_type`)
 - #164559 (Windows: fix Dir::remove and Dir::rename following symlinks)
 - #165562 (stop using gce in a test)
 - #165681 (Add safety requirements for Pattern and Searcher)
 - #165682 (interpret: ensure newly created unions have uninit in their padding)
 - #165693 (Move `check_attr` to attribute parsing)
 - #165715 (windows: check for null before wide encoding)
 - #165717 (mpsc_sync::stream_send_recv_stress: wait for all threads to finish)
 - #165733 (docs(num): remove needless whitespaces)
 - #165739 (remove NOP File::flush from fs tests)

Failed merges:

 - #165727 (Normalize "beta release channel" in `const-supertrait-unstable` test)
 - #164218 (Tweak private re-export errors to point at correct span and have more context)
rust-bors Bot pushed a commit that referenced this pull request Nov 10, 2026
…uwer

Rollup of 24 pull requests

Successful merges:

 - #165709 (remove 'x miri' smoke test from PR CI)
 - #164217 (Prepare to make `rustc_trivial_field_reads` usable in user code)
 - #165569 (`Move` trait feature gate)
 - #165575 (Upgrade bundled musl to 1.2.6)
 - #163241 (Add enclosing item context when pointing at `FnMut` bound)
 - #163992 (add `core::arch::{x86, x86_64}::x87_f80`)
 - #165047 (Change the span building of paths from first-to-last to chained)
 - #165458 (fix incorrect byte count suffix on MSVC)
 - #165490 (Mark the compile unit for `-Zdebuginfo-for-profiling`)
 - #165586 (Remove `backend_optimization_level`.)
 - #165646 (fix NumScalableVectors::from_field_count)
 - #165655 (Unignore passing GCC backend tests)
 - #165740 (miri subtree update)
 - #165753 (avoid Terminator:None in MIR inliner)
 - #153261 (Partially stabilize `ptr_alignment_type` as `alignment_type`)
 - #164559 (Windows: fix Dir::remove and Dir::rename following symlinks)
 - #165562 (stop using gce in a test)
 - #165681 (Add safety requirements for Pattern and Searcher)
 - #165682 (interpret: ensure newly created unions have uninit in their padding)
 - #165693 (Move `check_attr` to attribute parsing)
 - #165715 (windows: check for null before wide encoding)
 - #165717 (mpsc_sync::stream_send_recv_stress: wait for all threads to finish)
 - #165733 (docs(num): remove needless whitespaces)
 - #165739 (remove NOP File::flush from fs tests)

Failed merges:

 - #165727 (Normalize "beta release channel" in `const-supertrait-unstable` test)
 - #164218 (Tweak private re-export errors to point at correct span and have more context)
@rust-bors
rust-bors Bot merged commit 8a8e3bf into rust-lang:main Nov 10, 2026
14 checks passed
@rustbot rustbot added this to the 1.102.0 milestone Nov 10, 2026
rust-bors Bot pushed a commit that referenced this pull request Nov 10, 2026
Rollup merge of #164559 - RalfJung:dir-remove-rename-symlinks, r=ChrisDenton

Windows: fix Dir::remove and Dir::rename following symlinks

Windows `fs::Dir` used to follow symlinks for operations that should instead act on the symlink itself. This fixes that.
Fixes #164249

try-job: test-x86_64-msvc*
Cc @ChrisDenton
@RalfJung
RalfJung deleted the dir-remove-rename-symlinks branch November 10, 2026 20:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-filesystem Area: `std::fs` S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs Relevant to the library team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fs::Dir::remove_file and remove_dir remove the target of a symlink on Windows

6 participants