feat: add &mut Command to post_spawn()

[TheLostLambda]

Builds on #23 (so closes #22) and fixes the stack overflow in that PR on Windows. Then adds &mut Command to post_spawn().

If @passcod is happy with the code, then it'll just need some documentation and tests, I reckon!

[TheLostLambda]

It seems like all of the remaining changes were quite simple after all of your hard work! I'll test them with my LogFile wrapper and show a before and after of how my code would be simplified by this PR!

Obviously I'm also happy to split this into two PRs if you think that's more reasonable :)

[TheLostLambda]

Looks like there is still a bug with start_kill being overridden by default — I'll investigate!

[TheLostLambda]

Okay, I correctly fixed that stack overflow now, and everything is working!

For a final comparison, here is my code (leaving out imports and consts for length) before this PR:

#[derive(Debug)]
struct LogFile {
    path: PathBuf,
    thread: Option<JoinHandle<()>>,
}

impl LogFile {
    fn new(path: impl Into<PathBuf>) -> Self {
        Self {
            path: path.into(),
            thread: None,
        }
    }
}

impl StdCommandWrapper for LogFile {
    fn pre_spawn(&mut self, command: &mut Command, _core: &StdCommandWrap) -> io::Result<()> {
        let mut logfile = File::create(&self.path)?;
        let (mut rx, tx) = pipe()?;

        self.thread = Some(thread::spawn(move || {
            io::copy(&mut rx, &mut logfile).unwrap();
        }));

        command.stdout(tx.try_clone()?).stderr(tx);
        Ok(())
    }

    fn wrap_child(
        &mut self,
        child: Box<dyn StdChildWrapper>,
        _core: &StdCommandWrap,
    ) -> io::Result<Box<dyn StdChildWrapper>> {
        let wrapped_child = LogFileChild {
            inner: child,
            thread: mem::take(&mut self.thread),
        };
        Ok(Box::new(wrapped_child))
    }
}

#[derive(Debug)]
struct LogFileChild {
    inner: Box<dyn StdChildWrapper>,
    thread: Option<JoinHandle<()>>,
}

impl StdChildWrapper for LogFileChild {
    fn inner(&self) -> &Child {
        self.inner.inner()
    }

    fn inner_mut(&mut self) -> &mut Child {
        self.inner.inner_mut()
    }

    fn into_inner(self: Box<Self>) -> Child {
        self.inner.into_inner()
    }

    fn wait(&mut self) -> io::Result<ExitStatus> {
        let exit_status = self.inner.wait();

        if let Some(thread) = mem::take(&mut self.thread) {
            thread.join().expect("logfile thread panicked");
        }

        exit_status
    }

    // NOTE: This is a hacky work-around needed before this PR
    fn start_kill(&mut self) -> io::Result<()> {
        self.inner.start_kill()
    }
}

fn main() -> Result<(), Box<dyn Error>> {
    let mut command = StdCommandWrap::with_new(PROGRAM, |command| {
        command.args(ARGS);
    });
    command
        .wrap(JobObject)
        .wrap(CreationFlags(CREATE_NO_WINDOW | CREATE_SUSPENDED))
        .wrap(LogFile::new(LOGFILE_PATH));

    let mut child = command.spawn()?;

    // NOTE: This is a hacky work-around needed before this PR
    command
        .command_mut()
        .stdout(Stdio::inherit())
        .stderr(Stdio::inherit());

    sleep(Duration::from_secs(10));

    child.kill()?;

    Ok(())
}

And here is the same code after this PR:

#[derive(Debug)]
struct LogFile {
    path: PathBuf,
    thread: Option<JoinHandle<()>>,
}

impl LogFile {
    fn new(path: impl Into<PathBuf>) -> Self {
        Self {
            path: path.into(),
            thread: None,
        }
    }
}

impl StdCommandWrapper for LogFile {
    fn pre_spawn(&mut self, command: &mut Command, _core: &StdCommandWrap) -> io::Result<()> {
        let mut logfile = File::create(&self.path)?;
        let (mut rx, tx) = pipe()?;

        self.thread = Some(thread::spawn(move || {
            io::copy(&mut rx, &mut logfile).unwrap();
        }));

        command.stdout(tx.try_clone()?).stderr(tx);
        Ok(())
    }

    fn post_spawn(
        &mut self,
        command: &mut Command,
        _child: &mut Child,
        _core: &StdCommandWrap,
    ) -> io::Result<()> {
        command.stdout(Stdio::inherit()).stderr(Stdio::inherit());

        Ok(())
    }

    fn wrap_child(
        &mut self,
        child: Box<dyn StdChildWrapper>,
        _core: &StdCommandWrap,
    ) -> io::Result<Box<dyn StdChildWrapper>> {
        let wrapped_child = LogFileChild {
            inner: child,
            thread: mem::take(&mut self.thread),
        };
        Ok(Box::new(wrapped_child))
    }
}

#[derive(Debug)]
struct LogFileChild {
    inner: Box<dyn StdChildWrapper>,
    thread: Option<JoinHandle<()>>,
}

impl StdChildWrapper for LogFileChild {
    fn inner(&self) -> &dyn StdChildWrapper {
        &*self.inner
    }

    fn inner_mut(&mut self) -> &mut dyn StdChildWrapper {
        &mut *self.inner
    }

    fn into_inner(self: Box<Self>) -> Box<dyn StdChildWrapper> {
        self.inner
    }

    fn wait(&mut self) -> io::Result<ExitStatus> {
        let exit_status = self.inner.wait();

        if let Some(thread) = mem::take(&mut self.thread) {
            thread.join().expect("logfile thread panicked");
        }

        exit_status
    }
}

fn main() -> Result<(), Box<dyn Error>> {
    let mut command = StdCommandWrap::with_new(PROGRAM, |command| {
        command.args(ARGS);
    });
    command
        .wrap(JobObject)
        .wrap(CreationFlags(CREATE_NO_WINDOW | CREATE_SUSPENDED))
        .wrap(LogFile::new(LOGFILE_PATH));

    let mut child = command.spawn()?;

    sleep(Duration::from_secs(10));

    child.kill()?;

    Ok(())
}

Looking at it on GitHub, that's actually a lot of text, so here is the diff between those two (showing how this PR let me simplify my code):

[passcod]

I prefer stacking PRs so I've done that to make it easier to review

[passcod]

can you explain why it's needed / what it's for? Ah, I see from the other PR #23 (comment)

[passcod]

Okay with tests and documentation I'll gladly merge this.

[TheLostLambda]

Hitting three birds with one stone here! I've added doc-tests that cover this new functionality as used in my Logfile wrapper. That means I also fleshed out the documentation for creating your own wrappers!

[TheLostLambda]

Hopefully ready for merging and unblocks everything else! Sorry for the delay here and let me know if there is anything more you'd like me to do!

[passcod]

awesome!