Add std::os::windows::process::CommandExt. Fixes #37827 #38098
Conversation
…d add_creation_flags methods. Fixes rust-lang#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.
|
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @alexcrichton (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. Please see the contribution instructions for more information. |
|
This didn't seem to auto-link, but this fixes #37827 |
|
I wound up using the Windows debugger API in the test I wrote for this--I spent quite some time trying to find something simpler that I could test the behavior of that was keyed off of process creation flags, but it turns out to be kind of hard. |
There was a problem hiding this comment.
Looks great to me, nice test! (complexity seems fine)
Also we'll want to keep #37827 open until we stabilize, so it's ok to not auto-close.
| /// These will always be ORed with `CREATE_UNICODE_ENVIRONMENT`. | ||
| /// [1]: https://msdn.microsoft.com/en-us/library/windows/desktop/ms684863(v=vs.85).aspx | ||
| #[unstable(feature = "windows_process_extensions", issue = "37827")] | ||
| fn add_creation_flags(&mut self, flags: u32) -> &mut process::Command; |
There was a problem hiding this comment.
In terms of naming and functionality I think we'll want to follow the same pattern as
OpenOptions for specifying Windows-specific features. In that sense perhaps we can call this creation_flag and avoid the set/add variants and have just the equivalent of set?
There was a problem hiding this comment.
I had originally written it that way, but I worried about flexibility--it'd mean you couldn't attempt to set the creation flags in more than one place, since the last call would override. Perhaps I'm overthinking it? I suppose it's unlikely you'd be able to set creation flags from a library function in a useful manner.
There was a problem hiding this comment.
I renamed it to creation_flags (with an s) because the parameter in the CreateProcess signature is named dwCreationFlags. (There seem to be a few _flags methods in OpenOptions as well.)
There was a problem hiding this comment.
Ah yeah plural sounds good to me, and it's a good point about working with libraries. I'll add this as a concern to the tracking issue
|
|
||
| extern "system" { | ||
| fn WaitForDebugEvent(lpDebugEvent: *mut DEBUG_EVENT, dwMilliseconds: DWORD) -> BOOL; | ||
| fn ContinueDebugEvent(dwProcessId: DWORD, dwThreadId: DWORD, dwContinueStatus: DWORD) -> BOOL; |
There was a problem hiding this comment.
I think tidy is complaining about this line being over 100 chars
|
@bors: r+ |
|
📌 Commit e6975e9 has been approved by |
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.
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.