Skip to content

Fix set_times_nofollow for directory on windows #152083

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
rust-bors merged 1 commit into from
Feb 4, 2026
Merged

Fix set_times_nofollow for directory on windows #152083

rust-bors merged 1 commit into from
Feb 4, 2026
+53 −2

Conversation

@chenyukang
Copy link
Member

@chenyukang chenyukang commented Feb 4, 2026

Fix issue from:
#147455 (comment)

old code opts.write(true) on Windows requests GENERIC_WRITE access, replace with opts.access_mode(c::FILE_WRITE_ATTRIBUTES) to get minimal permission.

r? @joshtriplett

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Feb 4, 2026
@rustbot
Copy link
Collaborator

rustbot commented Feb 4, 2026

joshtriplett is not on the review rotation at the moment.
They may take a while to respond.

@chenyukang chenyukang mentioned this pull request Feb 4, 2026
Open
3 tasks
@chenyukang
Copy link
Member Author

chenyukang commented Feb 4, 2026

@bors try jobs=x86_64-msvc-1

@rust-bors

This comment has been minimized.

Sign in to view
rust-bors bot pushed a commit that referenced this pull request Feb 4, 2026
@bors
Auto merge of #152083 - chenyukang:yukang-fix-set-times-nofollow-on-w…
49572b4 
…indows, r=<try>

Fix set_times_nofollow for directory on windows


try-job: x86_64-msvc-1
@rust-bors
Copy link
Contributor

rust-bors bot commented Feb 4, 2026

☀️ Try build successful (CI)
Build commit: 49572b4 (49572b4ab0f6dedcbdec096a2879e0caf5905d17, parent: 0c40f5be0c8eb04d206e928c5d8c133a9142143a)

@ChrisDenton
Copy link
Member

ChrisDenton commented Feb 4, 2026 •
edited
Loading

lgtm, thanks!

@bors r+ rollup

@rust-bors
Copy link
Contributor

rust-bors bot commented Feb 4, 2026

📌 Commit 8e62b1d has been approved by ChrisDenton

It is now in the queue for this repository.

@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 Feb 4, 2026
JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request Feb 4, 2026
@JonathanBrouwer
Rollup merge of rust-lang#152083 - chenyukang:yukang-fix-set-times-no…
80e4531 
…follow-on-windows, r=ChrisDenton

Fix set_times_nofollow for directory on windows

Fix issue from:
rust-lang#147455 (comment)

old code `opts.write(true)` on Windows requests `GENERIC_WRITE` access, replace with `opts.access_mode(c::FILE_WRITE_ATTRIBUTES)` to get minimal permission.

r? @joshtriplett
@JonathanBrouwer JonathanBrouwer mentioned this pull request Feb 4, 2026
Closed
rust-bors bot pushed a commit that referenced this pull request Feb 4, 2026
@bors
Auto merge of #152099 - JonathanBrouwer:rollup-FYyOGkz, r=JonathanBro…
46c8c2f 
…uwer

Rollup of 11 pull requests

Successful merges:

 - #150605 (skip codegen for intrinsics with big fallback bodies if backend does not need them)
 - #150992 (link modifier `export-symbols`: export all global symbols from selected uptream c static libraries)
 - #151534 (target: fix destabilising target-spec-json)
 - #152088 (rustbook/README.md: add missing `)`)
 - #151526 (Fix autodiff codegen tests)
 - #151810 (citool: report debuginfo test statistics)
 - #152065 (Convert to inline diagnostics in `rustc_ty_utils`)
 - #152068 (Convert to inline diagnostics in `rustc_resolve`)
 - #152070 (Convert to inline diagnostics in `rustc_pattern_analysis`)
 - #152072 (Convert to inline diagnostics in `rustc_monomorphize`)
 - #152083 (Fix set_times_nofollow for directory on windows)

Failed merges:

 - #152069 (Convert to inline diagnostics in `rustc_privacy`)
JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request Feb 4, 2026
@JonathanBrouwer
Rollup merge of rust-lang#152083 - chenyukang:yukang-fix-set-times-no…
4939825 
…follow-on-windows, r=ChrisDenton

Fix set_times_nofollow for directory on windows

Fix issue from:
rust-lang#147455 (comment)

old code `opts.write(true)` on Windows requests `GENERIC_WRITE` access, replace with `opts.access_mode(c::FILE_WRITE_ATTRIBUTES)` to get minimal permission.

r? @joshtriplett
@JonathanBrouwer JonathanBrouwer mentioned this pull request Feb 4, 2026
Closed
JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request Feb 4, 2026
@JonathanBrouwer
Rollup merge of rust-lang#152083 - chenyukang:yukang-fix-set-times-no…
f47d856 
…follow-on-windows, r=ChrisDenton

Fix set_times_nofollow for directory on windows

Fix issue from:
rust-lang#147455 (comment)

old code `opts.write(true)` on Windows requests `GENERIC_WRITE` access, replace with `opts.access_mode(c::FILE_WRITE_ATTRIBUTES)` to get minimal permission.

r? @joshtriplett
This was referenced Feb 4, 2026
Closed
Merged
rust-bors bot pushed a commit that referenced this pull request Feb 4, 2026
@bors
Auto merge of #152104 - JonathanBrouwer:rollup-N93TdHm, r=JonathanBro…
8bccf12 
…uwer

Rollup of 12 pull requests

Successful merges:

 - #150992 (link modifier `export-symbols`: export all global symbols from selected uptream c static libraries)
 - #151534 (target: fix destabilising target-spec-json)
 - #152088 (rustbook/README.md: add missing `)`)
 - #151526 (Fix autodiff codegen tests)
 - #151810 (citool: report debuginfo test statistics)
 - #151952 (Revert doc attribute parsing errors to future warnings)
 - #152065 (Convert to inline diagnostics in `rustc_ty_utils`)
 - #152066 (Convert to inline diagnostics in `rustc_session`)
 - #152069 (Convert to inline diagnostics in `rustc_privacy`)
 - #152072 (Convert to inline diagnostics in `rustc_monomorphize`)
 - #152083 (Fix set_times_nofollow for directory on windows)
 - #152102 (Convert to inline diagnostics in all codegen backends)

Failed merges:

 - #152068 (Convert to inline diagnostics in `rustc_resolve`)
 - #152070 (Convert to inline diagnostics in `rustc_pattern_analysis`)
@rust-bors rust-bors bot merged commit 3184d55 into rust-lang:main Feb 4, 2026
12 checks passed
@rustbot rustbot added this to the 1.95.0 milestone Feb 4, 2026
rust-timer added a commit that referenced this pull request Feb 4, 2026
@rust-timer
Unrolled build for #152083
fb5135a 
Rollup merge of #152083 - chenyukang:yukang-fix-set-times-nofollow-on-windows, r=ChrisDenton

Fix set_times_nofollow for directory on windows

Fix issue from:
#147455 (comment)

old code `opts.write(true)` on Windows requests `GENERIC_WRITE` access, replace with `opts.access_mode(c::FILE_WRITE_ATTRIBUTES)` to get minimal permission.

r? @joshtriplett
@dishmaker
Copy link

dishmaker commented Feb 6, 2026

Sorry. The bug was my mistake. set_times actually works with directories on nightly 57d2fb136 2026-02-01 (before the fix).

What I did was:

  • times.set_created(created); instead of
  • times = times.set_created(created);, which returns Self to be assigned later.

Maybe #[must_use] would prevent such bugs with FileTimes::set_created.

@chenyukang
Copy link
Member Author

chenyukang commented Feb 6, 2026 •
edited
Loading

Sorry. The bug was my mistake. set_times actually works with directories on nightly 57d2fb136 2026-02-01 (before the fix).

What I did was:

  • times.set_created(created); instead of
  • times = times.set_created(created);, which returns Self to be assigned later.

Maybe #[must_use] would prevent such bugs with FileTimes::set_created.

could you please confirm the new nightly also work?
if there was no issues, we can keep the change.
there is a unit test, I assume it works as before.

@dishmaker
Copy link

dishmaker commented Feb 6, 2026

New nightly works too :) db3e99bba 2026-02-04

@ChrisDenton
Copy link
Member

ChrisDenton commented Feb 6, 2026

The change looks good to me regardless. Using the minimum necessary permissions is going to work in more situations and it's documented that FILE_WRITE_ATTRIBUTES is all that's needed. https://learn.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-setfiletime

@chenyukang
Copy link
Member Author

chenyukang commented Feb 6, 2026

yeah, let's keep it anyway.

@dishmaker dishmaker mentioned this pull request Feb 6, 2026
Open
@github-actions github-actions bot mentioned this pull request Feb 10, 2026
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@joshtriplett joshtriplett

Labels

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

Milestone

1.95.0

Development

Successfully merging this pull request may close these issues.

5 participants

@chenyukang @rustbot @ChrisDenton @dishmaker @joshtriplett