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

process: Impl {ChildStd*}::into_owned_{fd, handle} #5899

Merged
merged 3 commits into from Aug 9, 2023

Conversation

NobodyXu
Copy link
Contributor

Fixed #4403 and fixed #5333

Motivation

tokio::process::ChildStd* does not implement IntoRawFd/IntoRawHandle or any similar method to obtain an owned fd/handle of the child stdin/stdout/stderr.

It makes sense since converting it to a raw fd/handle is fallible operation.

Solution

Implement into_owned_fd and into_owned_handle for these ChildStd*, which returns io::Result<Owned*> since it is fallible.

I chose to return OwnedFd/OwnedHandle instead of RawFd/RawHandle since the msrv has just bumped to 1.63 so we can now use these types and it is much more ergonomic than returning raw ones.

@NobodyXu NobodyXu force-pushed the feat/into-owned-fd-and-handle branch from bf150ff to 491cdd6 Compare July 30, 2023 12:46
@Darksonn Darksonn added A-tokio Area: The main tokio crate M-process Module: tokio/process labels Jul 31, 2023
tokio/src/process/mod.rs Outdated Show resolved Hide resolved
@NobodyXu NobodyXu force-pushed the feat/into-owned-fd-and-handle branch from 491cdd6 to 35cb474 Compare July 31, 2023 08:10
@NobodyXu NobodyXu requested a review from Darksonn July 31, 2023 08:10
@NobodyXu NobodyXu changed the title Impl {ChildStd*}::into_owned_{fd, handle} process: Impl {ChildStd*}::into_owned_{fd, handle} Aug 1, 2023
@NobodyXu NobodyXu force-pushed the feat/into-owned-fd-and-handle branch 2 times, most recently from 90f463f to d87412d Compare August 4, 2023 05:17
@NobodyXu
Copy link
Contributor Author

NobodyXu commented Aug 4, 2023

CI failure is due to #5888

A re-run is required.

tokio/src/process/mod.rs Outdated Show resolved Hide resolved
@NobodyXu NobodyXu requested a review from Darksonn August 4, 2023 11:37
@NobodyXu NobodyXu force-pushed the feat/into-owned-fd-and-handle branch 4 times, most recently from 38f1ee1 to ba89641 Compare August 4, 2023 11:54
@NobodyXu
Copy link
Contributor Author

NobodyXu commented Aug 4, 2023

@Darksonn I've verified that the docs.rs build includes both windows and unix with features doc:

image image

@NobodyXu

This comment was marked as outdated.

@NobodyXu NobodyXu force-pushed the feat/into-owned-fd-and-handle branch 3 times, most recently from ca14846 to ae064a9 Compare August 6, 2023 14:14
NobodyXu and others added 3 commits August 9, 2023 10:37
Fixed tokio-rs#4403 and fixed tokio-rs#5333

Signed-off-by: Jiahao XU <Jiahao_XU@outlook.com>
Co-authored-by: Alice Ryhl <aliceryhl@google.com>
Signed-off-by: Jiahao XU <Jiahao_XU@outlook.com>
@NobodyXu NobodyXu force-pushed the feat/into-owned-fd-and-handle branch from ae064a9 to 4137b6e Compare August 9, 2023 00:37
Copy link
Contributor

@Darksonn Darksonn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@Darksonn Darksonn merged commit 0a631f8 into tokio-rs:master Aug 9, 2023
72 checks passed
@NobodyXu NobodyXu deleted the feat/into-owned-fd-and-handle branch August 9, 2023 20:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-tokio Area: The main tokio crate M-process Module: tokio/process
Projects
None yet
2 participants