Skip to content

Improve File::set_times error handling #101675

New issue

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

#

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

Already on GitHub? # to your account

Jump to bottom
Merged
merged 2 commits into from
Oct 1, 2022
Merged

Improve File::set_times error handling #101675

merged 2 commits into from
Oct 1, 2022

Conversation

beetrees
Copy link
Contributor

@beetrees beetrees commented Sep 11, 2022 •
edited
Loading

Makes File::set_times return an error if the SystemTime cannot fit into the required type instead of panicking in FileTimes::set_{accessed,modified}. Also makes File::set_times return an error on Windows if either of the passed times are 0xFFFF_FFFF_FFFF_FFFF, as the documentation for SetFileTime states that this will prevent operations on the file handle from updating the corresponding file time instead of setting the corresponding file time to that value.

Tracking issue: #98245

@rustbot
Copy link
Collaborator

rustbot commented Sep 11, 2022

Hey! It looks like you've submitted a new PR for the library teams!

If this PR contains changes to any rust-lang/rust public library APIs then please comment with @rustbot label +T-libs-api -T-libs to tag it appropriately. If this PR contains changes to any unstable APIs please edit the PR description to add a link to the relevant API Change Proposal or create one if you haven't already. If you're unsure where your change falls no worries, just leave it as is and the reviewer will take a look and make a decision to forward on if necessary.

Examples of T-libs-api changes:

  • Stabilizing library features
  • Introducing insta-stable changes such as new implementations of existing stable traits on existing stable types
  • Introducing new or changing existing unstable library APIs (excluding permanently unstable features / features without a tracking issue)
  • Changing public documentation in ways that create new stability guarantees
  • Changing observable runtime behavior of library APIs

@rustbot rustbot added the T-libs Relevant to the library team, which will review and decide on the PR/issue. label Sep 11, 2022
@rust-highfive
Copy link
Contributor

r? @Mark-Simulacrum

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 11, 2022
joshtriplett
joshtriplett reviewed Sep 12, 2022
View reviewed changes
@beetrees
Copy link
Contributor Author

@joshtriplett #102368 has now been merged. Is there anything else in this PR that needs addressing?

@joshtriplett
Copy link
Member

Looks good. Thank you!

@bors r+

@bors
Copy link
Collaborator

bors commented Sep 30, 2022

📌 Commit b8ae84c2f0e7f43521fb20bf385bb0aad4b9d509 has been approved by joshtriplett

It is now in the queue for this repository.

@bors bors 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 Sep 30, 2022
@bors
Copy link
Collaborator

bors commented Oct 1, 2022

⌛ Testing commit b8ae84c2f0e7f43521fb20bf385bb0aad4b9d509 with merge 51446c90ae1a022044c794d72326091c011bf446...

@bors
Copy link
Collaborator

bors commented Oct 1, 2022

💔 Test failed - checks-actions

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Oct 1, 2022
@rust-log-analyzer

This comment has been minimized.

# to view
@beetrees beetrees force-pushed the set-times-no-panic branch from b8ae84c to c66860a Compare October 1, 2022 02:24
@beetrees
Copy link
Contributor Author

beetrees commented Oct 1, 2022

@joshtriplett I've fixed the unused import that caused the build to fail.

@joshtriplett
Copy link
Member

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Oct 1, 2022

📌 Commit c66860a has been approved by joshtriplett

It is now in the queue for this repository.

@bors bors 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 Oct 1, 2022
@matthiaskrgr matthiaskrgr mentioned this pull request Oct 1, 2022
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Oct 1, 2022
@bors
Auto merge of rust-lang#102545 - matthiaskrgr:rollup-13i3tc3, r=matth…
56a35bc 
…iaskrgr

Rollup of 3 pull requests

Successful merges:

 - rust-lang#101675 (Improve `File::set_times` error handling)
 - rust-lang#102500 (Remove `expr_parentheses_needed` from `ParseSess`)
 - rust-lang#102533 (rustdoc: remove unused CSS selector `a.source`)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 21fc218 into rust-lang:master Oct 1, 2022
@rustbot rustbot added this to the 1.66.0 milestone Oct 1, 2022
@beetrees beetrees deleted the set-times-no-panic branch March 30, 2023 14:36
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@joshtriplett joshtriplett joshtriplett left review comments

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.66.0
Development

Successfully merging this pull request may close these issues.
7 participants
@beetrees @rustbot @rust-highfive @joshtriplett @bors @rust-log-analyzer @Mark-Simulacrum