Closed dimo414 closed 1 year ago
Moving this out of the PR description before merging:
I'm positing this as a PR to solicit feedback before merging, so please feel free to weigh in on any parts of this change you think could be improved. Some areas for potential feedback:
- The signatures and ergonomics of the affected and new methods
- In particular the
impl Write+Send
trait bound (which prevents us from usingStdoutLock
/StderrLock
because they are notSend
)- The use of scoped threads to avoid the
'static
trait bound- Error handling and
Result
propagation
- Notably, currently the
.join()
calls are.expect()
-ed because they don't seem to be compatible with?
- The
DisregardBrokenPipe
behavior, and whether it should be moved into the library core
This PR reimplements
Bkt::execute_subprocess()
and its callers to support streaming the subprocess' output while it's running, rather than blocking silently until the subprocess finishes.As suggested in https://stackoverflow.com/q/66060139 the child process' streams are processed in background threads responsible for both persisting and streaming the output to the caller's stdout/stderr.
Scoped threads are used to support streaming to references such as
&mut Vec<u8>
and to loosen the type requirements of the stream (namely to not require that they are 'static).CLI:
Library:
Bkt::retrieve_streaming
andBkt::refresh_streaming
methods which accept out/err sinks. The subprocess' output is written to these streams in addition to being cached and available in the returnedInvocation
.Bkt::retrieve
/Bkt::refresh
behave as previously, i.e. they do not support streaming.Fixes #31