8000 Add `CommandExt` for working with `std::process::Command` output streams by Malax · Pull Request #535 · heroku/libcnb.rs · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Add CommandExt for working with std::process::Command output streams #535

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 4 commits into from
Dec 15, 2022
Merged

Add CommandExt for working with std::process::Command output streams #535

merged 4 commits into from
Dec 15, 2022

Conversation

Malax
Copy link
Member
@Malax Malax commented Dec 7, 2022 •
edited
Loading

While working on the Gradle buildpack, I had the need to prefix gradlew output so the user can clearly distinguish between gradlew and buildpack output. While implementing a solution, @dzuelke mentioned that he has the same need and also some additional ones. So I went the extra mile, made my solution generic and added docs so that it is easy to use. Implementing this is less trivial than it might sound so I thought it's a great fit for libherokubuildpack (or whatever it will be called in the future)

It all revolves around a new trait: CommandExt which extends std::process:Command with (currently) two helpers that allow writing the unbuffered output of stdout and stderr to a std::io::Writer.

This is the main hook to add more complex logic like teeing output to two different locations, prefixing lines and live-modifying streams using standard String functions. These helpers are contained in the new write module which has the aforementioned helpers. These can be used without CommentExt and are standalone. Their only interface is std::io::Write.

Here is an example how this can be used:

use libherokubuildpack::command::CommandExt;
use libherokubuildpack::write::tee;
use std::fs;
use std::process::Command;

let logfile = fs::File::open("log.txt").unwrap();

let exit_status = Command::new("date")
    .spawn_and_write_streams(tee(std::io::stdout(), &logfile), std::io::stderr())
    .and_then(|mut child| child.wait())
    .unwrap();

The exposed API has proper rust docstrings. For detailed info and more usage examples refer to those.

Closes GUS-W-12188139

@Malax Malax added enhancement New feature or request libherokubuildpack labels Dec 7, 2022
@Malax Malax force-pushed the malax/command branch 11 times, most recently from ce52b7f to c4be703 Compare December 8, 2022 15:24
@Malax Malax changed the title Add helpers to work with Command output Add CommandExt for working with std::process::Command output streams Dec 8, 2022
@Malax Malax force-pushed the malax/command branch 2 times, most recently from 605177d to 4e483c1 Compare December 8, 2022 15:44
@Malax Malax marked this pull request as ready for review December 8, 2022 15:52
@Malax Malax requested a review from a team as a code owner December 8, 2022 15:52
schneems
schneems reviewed Dec 12, 2022
View reviewed changes
Copy link
Contributor
@schneems schneems left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still working on the review. Wanted to leave these questions/notes for you. I'll pick up the write.rs and finish up later.

Overall looks good. I like having some of this general purpose goodness in shared libraries.

schneems
schneems approved these changes Dec 13, 2022
View reviewed changes
Copy link
Contributor
@schneems schneems left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added some additional comments.

  • One question about whether we need Clone or why we might not want it.
  • Another comment related to naming. I would like to add capture to output_and_write_streams or otherwise indicate what it's doing as I feel the existing naming of spawn or output isn't very descriptive. I would prefer it but don't want to block on it if you feel strongly otherwise.

edmorley
edmorley reviewed Dec 15, 2022
View reviewed changes
Copy link
Member
@edmorley edmorley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for adding this!

@Malax Malax merged commit 2c9c642 into main Dec 15, 2022
@Malax Malax deleted the malax/command branch December 15, 2022 11:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@schneems schneems schneems approved these changes

@edmorley edmorley edmorley approved these changes

Assignees
No one assigned
Labels
enhancement New feature or request libherokubuildpack
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Malax @schneems @edmorley
0