-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
Provide an API for std::process::Command
to set CreateProcess
flags on Windows
#37827
Comments
Seems reasonable to me! |
👍 |
I'm always in favor of adding more Windows specific extension traits to expose useful functionality that other platforms don't seem to have. |
I have a patch, it just took me a while to write a test for it. I'm trying to sort out the stability annotations for the new APIs--can I just use this issue as the tracking issue for the new feature? |
Sure! |
To the open question: I had first written the API as originally proposed: with a |
Add std::os::windows::process::CommandExt. Fixes #37827 This adds a CommandExt trait for Windows along with an implementation of it for std::process::Command with methods to set the process creation flags that are passed to CreateProcess.
std::process::Command
to set DETACHED_PROCESS
flag on Windowsstd::process::Command
to set CreateProcess
flags on Windows
This fixes the extra console windows that show up when the sccache server creates compiler processes on Windows, but it only works on nightly Rust currently, and requires building with `--features=unstable`, because it relies on the not-yet-stable `windows_process_extensions` feature (rust-lang/rust#37827). Also I added a bunch of extra documentation to the mock_command traits.
@rfcbot fcp merge With the one remaining open question I think it's safe to stabilize as this is still consistent with our approach to custom flags in other areas of platform-specific extensions as well. |
Team member @alexcrichton has proposed to merge this. The next step is review by the rest of the tagged teams: No concerns currently listed. Once these reviewers reach consensus, this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up! See this document for info about what commands tagged team members can give me. |
🔔 This is now entering its final comment period, as per the review above. 🔔 psst @alexcrichton, I wasn't able to add the |
This commit applies the stabilization/deprecations of the 1.16.0 release, as tracked by the rust-lang/rust issue tracker and the final-comment-period tag. The following APIs were stabilized: * `VecDeque::truncate` * `VecDeque::resize` * `String::insert_str` * `Duration::checked_{add,sub,div,mul}` * `str::replacen` * `SocketAddr::is_ipv{4,6}` * `IpAddr::is_ipv{4,6}` * `str::repeat` * `Vec::dedup_by` * `Vec::dedup_by_key` * `Result::unwrap_or_default` * `<*const T>::wrapping_offset` * `<*mut T>::wrapping_offset` * `CommandExt::creation_flags` (on Windows) * `File::set_permissions` * `String::split_off` The following APIs were deprecated * `EnumSet` - replaced with other ecosystem abstractions, long since unstable Closes rust-lang#27788 Closes rust-lang#35553 Closes rust-lang#35774 Closes rust-lang#36436 Closes rust-lang#36949 Closes rust-lang#37079 Closes rust-lang#37087 Closes rust-lang#37516 Closes rust-lang#37827 Closes rust-lang#37916 Closes rust-lang#37966 Closes rust-lang#38080
std: Stabilize APIs for the 1.16.0 release This commit applies the stabilization/deprecations of the 1.16.0 release, as tracked by the rust-lang/rust issue tracker and the final-comment-period tag. The following APIs were stabilized: * `VecDeque::truncate` * `VecDeque::resize` * `String::insert_str` * `Duration::checked_{add,sub,div,mul}` * `str::replacen` * `SocketAddr::is_ipv{4,6}` * `IpAddr::is_ipv{4,6}` * `str::repeat` * `Vec::dedup_by` * `Vec::dedup_by_key` * `Result::unwrap_or_default` * `<*const T>::wrapping_offset` * `<*mut T>::wrapping_offset` * `CommandExt::creation_flags` (on Windows) * `File::set_permissions` * `String::split_off` The following APIs were deprecated * `EnumSet` - replaced with other ecosystem abstractions, long since unstable Closes rust-lang#27788 Closes rust-lang#35553 Closes rust-lang#35774 Closes rust-lang#36436 Closes rust-lang#36949 Closes rust-lang#37079 Closes rust-lang#37087 Closes rust-lang#37516 Closes rust-lang#37827 Closes rust-lang#37916 Closes rust-lang#37966 Closes rust-lang#38080
Updated report
Now a tracking issue for
CommandExt::creation_flags
on Windows.Open questions:
Also, don't forget to add to the prelude when stabilizing!
Original report
There's a currently-unused codepath in the Windows implementation of
std::process::Command::spawn
that can setDETATCHED_PROCESS
:rust/src/libstd/sys/windows/process.rs
Line 162 in b1363a7
It looks like historically there was a method to set this but it got removed. Unfortunately there's currently no way to set this, and no other way to set the
CreateProcess
flags, so for a project I'm working on I basically had to reimplementspawn
for Windows:https://github.com/luser/sccache2/blob/89f4b44d01084846893a64137d965ee844fe2d72/src/commands.rs#L103
I think we need some way to set this, even if it's Windows-specific. There are a bunch of other flags that can be specified, so maybe a way to set arbitrary flags would be useful?
https://msdn.microsoft.com/en-us/library/windows/desktop/ms684863(v=vs.85).aspx
I think this would fit nicely onto a Windows equivalent of the
std::os::unix::process::CommandExt
trait, maybe something like:The text was updated successfully, but these errors were encountered: