Skip to content
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.

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

Move FdTable to a common location and split off Unix behavior #4045

Merged
merged 3 commits into from
Dec 1, 2024

Conversation

CraftSpider
Copy link
Contributor

I'm open to instead keeping all the functions on the base trait, but this seemed like an easy way to keep them clearly only intended to be used in the unix shims.

Preparation for Windows FS support, see #4043

@cgettys-microsoft
Copy link
Contributor

@sivadeilra - here's one of the PRs related to adding Miri support for Windows FS APIs, RE: #3482

@RalfJung
Copy link
Member

I'm open to instead keeping all the functions on the base trait, but this seemed like an easy way to keep them clearly only intended to be used in the unix shims.

What are you referring to here?

@bors
Copy link
Contributor

bors commented Nov 26, 2024

☔ The latest upstream changes (presumably #4016) made this pull request unmergeable. Please resolve the merge conflicts.

@CraftSpider
Copy link
Contributor Author

This is now 3 commits - the first only moves the FdMap, the seconds splits out Unix functionality, the third makes the fn metadata change used by the Windows PR

@RalfJung
Copy link
Member

@CraftSpider Thanks!
This seems to have conflicts with master now, so CI cannot run. Could you resolve those conflicts?

@rustbot author

(write @rustbot ready when this is ready for review)

@rustbot rustbot added the S-waiting-on-author Status: Waiting for the PR author to address review comments label Nov 29, 2024
@CraftSpider
Copy link
Contributor Author

@rustbot ready

@rustbot rustbot added S-waiting-on-review Status: Waiting for a review to complete and removed S-waiting-on-author Status: Waiting for the PR author to address review comments labels Dec 1, 2024
src/shims/files.rs Outdated Show resolved Hide resolved
src/shims/files.rs Outdated Show resolved Hide resolved
src/shims/files.rs Outdated Show resolved Hide resolved
@RalfJung
Copy link
Member

RalfJung commented Dec 1, 2024

@rustbot author

@rustbot rustbot added S-waiting-on-author Status: Waiting for the PR author to address review comments and removed S-waiting-on-review Status: Waiting for a review to complete labels Dec 1, 2024
@CraftSpider
Copy link
Contributor Author

@rustbot ready

@rustbot rustbot added S-waiting-on-review Status: Waiting for a review to complete and removed S-waiting-on-author Status: Waiting for the PR author to address review comments labels Dec 1, 2024
@RalfJung
Copy link
Member

RalfJung commented Dec 1, 2024

Looks good, thanks! Please squash the last 2 commits in the previous ones (but definitely leave the first commit separate).

@RalfJung RalfJung added S-waiting-on-author Status: Waiting for the PR author to address review comments and removed S-waiting-on-review Status: Waiting for a review to complete labels Dec 1, 2024
@CraftSpider
Copy link
Contributor Author

@rustbot ready

@rustbot rustbot added S-waiting-on-review Status: Waiting for a review to complete and removed S-waiting-on-author Status: Waiting for the PR author to address review comments labels Dec 1, 2024
@RalfJung RalfJung enabled auto-merge December 1, 2024 21:52
@RalfJung RalfJung added this pull request to the merge queue Dec 1, 2024
Merged via the queue into rust-lang:master with commit b27f1d6 Dec 1, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Waiting for a review to complete
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants