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 fd into std::sys #139092

Merged
merged 2 commits into from
Apr 5, 2025
Merged

Move fd into std::sys #139092

merged 2 commits into from
Apr 5, 2025

Conversation

thaliaarchi
Copy link
Contributor

@thaliaarchi thaliaarchi commented Mar 29, 2025

Move platform definitions of fd into std::sys, as part of #117276.

Unlike other modules directly under std::sys, this is only available on some platforms and I have not provided a fallback abstraction for unsupported platforms. That is similar to how std::os::fd is gated to only supported platforms.

Also, fix the unsafe_op_in_unsafe_fn lint, which was allowed for the Unix fd impl. Since macro expansions from std::sys::pal::unix::weak trigger this lint, fix it there too.

cc @joboet, @ChrisDenton

try-job: x86_64-gnu-aux

@rustbot
Copy link
Collaborator

rustbot commented Mar 29, 2025

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

@rustbot rustbot added O-hermit Operating System: Hermit O-SGX Target: SGX O-unix Operating system: Unix-like O-wasi Operating system: Wasi, Webassembly System Interface 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 Mar 29, 2025
@joboet
Copy link
Member

joboet commented Mar 29, 2025

It'd be best left to another PR, but I think we could even move the Windows Handle code in this module and rename FileDesc to something like Object.

@joboet
Copy link
Member

joboet commented Mar 29, 2025

Great, thank you!
r=me once #138988 has merged.

@bors delegate+

@bors
Copy link
Collaborator

bors commented Mar 29, 2025

✌️ @thaliaarchi, you can now approve this pull request!

If @joboet told you to "r=me" after making some further change, please make that change, then do @bors r=@joboet

@bors
Copy link
Collaborator

bors commented Mar 29, 2025

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

@thaliaarchi
Copy link
Contributor Author

@bors r=@joboet

@bors
Copy link
Collaborator

bors commented Mar 29, 2025

📌 Commit 41d6fbf has been approved by joboet

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 Mar 29, 2025
jhpratt added a commit to jhpratt/rust that referenced this pull request Mar 30, 2025
Move `fd` into `std::sys`

Move platform definitions of `fd` into `std::sys`, as part of rust-lang#117276.

Unlike other modules directly under `std::sys`, this is only available on some platforms and I have not provided a fallback abstraction for unsupported platforms. That is similar to how `std::os::fd` is gated to only supported platforms.

Also, fix the `unsafe_op_in_unsafe_fn` lint, which was allowed for the Unix fd impl. Since macro expansions from `std::sys::pal::unix::weak` trigger this lint, fix it there too.

cc `@joboet,` `@ChrisDenton`
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 30, 2025
Rollup of 5 pull requests

Successful merges:

 - rust-lang#137836 (Set `target_vendor = "openwrt"` on `mips64-openwrt-linux-musl`)
 - rust-lang#138206 ([AIX] Ignore linting on repr(C) structs with repr(packed) or repr(align(n)))
 - rust-lang#139044 (bootstrap: Avoid cloning `change-id` list)
 - rust-lang#139092 (Move `fd` into `std::sys`)
 - rust-lang#139111 (Properly document FakeReads)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors
Copy link
Collaborator

bors commented Mar 30, 2025

⌛ Testing commit 41d6fbf with merge b15da48...

bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 30, 2025
Move `fd` into `std::sys`

Move platform definitions of `fd` into `std::sys`, as part of rust-lang#117276.

Unlike other modules directly under `std::sys`, this is only available on some platforms and I have not provided a fallback abstraction for unsupported platforms. That is similar to how `std::os::fd` is gated to only supported platforms.

Also, fix the `unsafe_op_in_unsafe_fn` lint, which was allowed for the Unix fd impl. Since macro expansions from `std::sys::pal::unix::weak` trigger this lint, fix it there too.

cc `@joboet,` `@ChrisDenton`
@rust-log-analyzer

This comment has been minimized.

@bors
Copy link
Collaborator

bors commented Mar 30, 2025

💔 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 Mar 30, 2025
@thaliaarchi
Copy link
Contributor Author

The fix for the CI error is not obvious to me. Miri indeed does not implement readv or writev, but I would have expected an error from that to have been noticed much sooner than now. I didn’t modify any miri gates. Perhaps Miri hardcodes some pal paths?

@jieyouxu
Copy link
Member

cc @RalfJung

@RalfJung
Copy link
Member

Yeah, we skip the pal tests here:

$(Q)MIRIFLAGS="-Zmiri-disable-isolation" \
$(BOOTSTRAP) miri --stage 2 library/std \
$(BOOTSTRAP_ARGS) \
--no-doc -- \
--skip fs:: --skip net:: --skip process:: --skip sys::pal::
$(Q)MIRIFLAGS="-Zmiri-disable-isolation" \
$(BOOTSTRAP) miri --stage 2 library/std \
$(BOOTSTRAP_ARGS) \
--doc -- \
--skip fs:: --skip net:: --skip process:: --skip sys::pal::

So we'll need to come up with a new pattern for this. We already list fs::, maybe just add fd::?

@rustbot rustbot added the T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) label Apr 5, 2025
@thaliaarchi
Copy link
Contributor Author

Thanks for the fix, Ralf. I've pushed an update for that and added the failed job as a try-job. Can we do a try run?

@Zalathar
Copy link
Contributor

Zalathar commented Apr 5, 2025

@bors try

bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 5, 2025
Move `fd` into `std::sys`

Move platform definitions of `fd` into `std::sys`, as part of rust-lang#117276.

Unlike other modules directly under `std::sys`, this is only available on some platforms and I have not provided a fallback abstraction for unsupported platforms. That is similar to how `std::os::fd` is gated to only supported platforms.

Also, fix the `unsafe_op_in_unsafe_fn` lint, which was allowed for the Unix fd impl. Since macro expansions from `std::sys::pal::unix::weak` trigger this lint, fix it there too.

cc `@joboet,` `@ChrisDenton`

try-job: x86_64-gnu-aux
@bors
Copy link
Collaborator

bors commented Apr 5, 2025

⌛ Trying commit 3ab22fa with merge 45c5651...

@bors
Copy link
Collaborator

bors commented Apr 5, 2025

☀️ Try build successful - checks-actions
Build commit: 45c5651 (45c56513e043b1584b826d846c962eaf3f70c3e9)

@thaliaarchi
Copy link
Contributor Author

@bors r=joboet

@bors
Copy link
Collaborator

bors commented Apr 5, 2025

📌 Commit 3ab22fa has been approved by joboet

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 Apr 5, 2025
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 5, 2025
…iaskrgr

Rollup of 4 pull requests

Successful merges:

 - rust-lang#138368 (KCFI: Add KCFI arity indicator support)
 - rust-lang#138381 (Implement `SliceIndex` for `ByteStr`)
 - rust-lang#139092 (Move `fd` into `std::sys`)
 - rust-lang#139398 (Change notifications for Exploit Mitigations PG)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit a64ccf4 into rust-lang:master Apr 5, 2025
7 checks passed
@rustbot rustbot added this to the 1.88.0 milestone Apr 5, 2025
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Apr 5, 2025
Rollup merge of rust-lang#139092 - thaliaarchi:move-fd-pal, r=joboet

Move `fd` into `std::sys`

Move platform definitions of `fd` into `std::sys`, as part of rust-lang#117276.

Unlike other modules directly under `std::sys`, this is only available on some platforms and I have not provided a fallback abstraction for unsupported platforms. That is similar to how `std::os::fd` is gated to only supported platforms.

Also, fix the `unsafe_op_in_unsafe_fn` lint, which was allowed for the Unix fd impl. Since macro expansions from `std::sys::pal::unix::weak` trigger this lint, fix it there too.

cc `@joboet,` `@ChrisDenton`

try-job: x86_64-gnu-aux
@thaliaarchi thaliaarchi deleted the move-fd-pal branch April 5, 2025 19:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
O-hermit Operating System: Hermit O-SGX Target: SGX O-unix Operating system: Unix-like O-wasi Operating system: Wasi, Webassembly System Interface S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) 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.

9 participants