miri icon indicating copy to clipboard operation
miri copied to clipboard

Support getting file metadata on Windows

Open CraftSpider opened this issue 1 year ago • 16 comments

Main changes are the first two commits - one refactors handles a bit to make their usage more consistent and adds the file variant, the other actually adds the metadata shims.

This also changes miri to never return invalid handle errors, instead always treating them as an error. This behavior matches that of code running under the MSVC debugger - invalid handles are treated as exceptions in debug contexts.

Split off #4043

CraftSpider avatar Dec 02 '24 03:12 CraftSpider

Before we get too much further on this PR, I'd like to point out https://github.com/rust-lang/miri/pull/4043/commits/296d806930304c03f2cba91cc8a2fed4a2f38d16 (which I just pushed) on the big PR. It's the changes needed to FileDescription methods to make them platform-agnostic enough for Windows use-cases. If they look fine, we can continue, but if you think it's a bad design then the FileDescription trait will need entirely split in two, which will change code even in this PR.

CraftSpider avatar Dec 06 '24 04:12 CraftSpider

Reducing the places where we have to use global state like errno seems like an improvement. But please use Result<_, IoError> (with Miri's IoError type).

RalfJung avatar Dec 06 '24 06:12 RalfJung

@rustbot ready

CraftSpider avatar Dec 06 '24 21:12 CraftSpider

Okay I finally had a look at the core new file in this PR. :) @rustbot author

RalfJung avatar Dec 07 '24 08:12 RalfJung

@rustbot ready

CraftSpider avatar Dec 09 '24 21:12 CraftSpider

:umbrella: The latest upstream changes (presumably #4068) made this pull request unmergeable. Please resolve the merge conflicts.

bors avatar Dec 12 '24 08:12 bors

@rustbot author

RalfJung avatar Dec 12 '24 16:12 RalfJung

:umbrella: The latest upstream changes (possibly 887b4984e2ec4690afb6aa2c3f31408b570033ca) made this pull request unmergeable. Please resolve the merge conflicts.

rustbot avatar Dec 29 '24 09:12 rustbot

:umbrella: The latest upstream changes (possibly aaa8151e524d1e0c87137ba64a260330e67ed0b0) made this pull request unmergeable. Please resolve the merge conflicts.

rustbot avatar Jan 30 '25 17:01 rustbot

Amazing timing, me. I'm on vacation for the next week, but will rebase again after

CraftSpider avatar Feb 06 '25 02:02 CraftSpider

Oh, was this waiting for review? Sorry, that wasn't clear to me. Please write @rustbot review once the PR is ready again.

RalfJung avatar Feb 06 '25 08:02 RalfJung

No problem, I forgot to do that a while back, then didn't want to do it until I rebased, and somewhere along the way a test started failing... So I'll fix everything that fell out from a month of going stale, then mark it ready.

CraftSpider avatar Feb 06 '25 22:02 CraftSpider

@rustbot review

CraftSpider avatar Feb 12 '25 07:02 CraftSpider

@RalfJung any update on this? It's fine if you're busy or such, just don't want it to get too stale again hopefully

CraftSpider avatar Feb 24 '25 20:02 CraftSpider

Sorry for the delay. It's in my queue, I have not lost track. :)

RalfJung avatar Feb 24 '25 20:02 RalfJung

Thanks, sorry for the unnecessary ping then.

CraftSpider avatar Feb 24 '25 21:02 CraftSpider

@rustbot author

RalfJung avatar Mar 20 '25 17:03 RalfJung

@rustbot review

CraftSpider avatar Apr 02 '25 02:04 CraftSpider

:umbrella: The latest upstream changes (possibly 7df496c4c37acdc832c4296a47b783a4882960a2) made this pull request unmergeable. Please resolve the merge conflicts.

rustbot avatar Apr 04 '25 16:04 rustbot

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

rustbot avatar Apr 07 '25 15:04 rustbot

Thanks for the patience and help with getting this over the line! After this merges, I'll work to flesh out the shims more, make it so you can actually do useful things with files targeting Windows.

@rustbot ready

CraftSpider avatar Apr 07 '25 22:04 CraftSpider

Thank you for your patience with the sometimes slow round-trip times. :)

This looks great, thanks! Please squash the commits, then we can land this. Please use the --keep-base flag when squashing so that the force-push diff is easier to review. Then write @rustbot ready after you pushed the squashed PR.

@rustbot author

RalfJung avatar Apr 08 '25 06:04 RalfJung

@rustbot review

CraftSpider avatar Apr 09 '25 04:04 CraftSpider