watchexec / command-group

Deprecated: use process-wrap. || Extension to Command to spawn in a process group
https://docs.rs/command-group
Other
37 stars 10 forks source link

Support kill_on_drop(false) on Windows #17

Closed arlyon closed 1 year ago

arlyon commented 1 year ago

Hi! I am interested in using this lib to start cross-platform daemons on win / unix. As I understand it, in this case, we should be able to avoid setting the flag here:

https://github.com/watchexec/command-group/blob/main/src/winres.rs#L89

I assume that this is not implemented due to kill_on_drop being private?

Seems to be a similar problem to #15

arlyon commented 1 year ago

I would like to suggest as an alternative to both of these that the library handles the setting of these parameters on its own builder. I'm a little bit hesitant because it could lead to some awkward footguns, if, for example, someone sets kill_on_drop before converting it to a group builder... however, my thought would be:

let mut group = tokio::process::Command::new(path)
    .arg("daemon")
    .stderr(Stdio::null())
    .stdout(Stdio::null())
    .group() // converts it to a group builder
    .kill_on_drop(false)
    .spawn()?;

Happy to open a PR, though I realize this is a major breaking change...

passcod commented 1 year ago

Not necessarily a breaking change, if both group() and group_spawn() are supported. I'm keen, I think that would then make it elegant to support e.g. setting process flags for #15

arlyon commented 1 year ago

Thanks for the quick reply, I'll get a PR up soon :)