docs: show how to stringify the output of Command

[beyarkay]

This documentation-only change clarifies how the user can get the string representation of the output of a command.

The stdout and stderr members of std::process::Output from std::process::Command return a Vec, which is not always what the user wants.

For simple cases like printing stderr iff the std::process::ExitStatus is non-zero, it's useful to get the string representation of this Vec<u8>. This can be done via String::from_utf8_lossy, but it's not clear that this is possible from the documentation without first searching the internet for an answer.

Link to playground: https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=8189f65dc4b354a643311af2cea5b230

[rustbot]

Hey! It looks like you've submitted a new PR for the library teams!

If this PR contains changes to any rust-lang/rust public library APIs then please comment with @rustbot label +T-libs-api -T-libs to tag it appropriately. If this PR contains changes to any unstable APIs please edit the PR description to add a link to the relevant API Change Proposal or create one if you haven't already. If you're unsure where your change falls no worries, just leave it as is and the reviewer will take a look and make a decision to forward on if necessary.

Examples of T-libs-api changes:

• Stabilizing library features
• Introducing insta-stable changes such as new implementations of existing stable traits on existing stable types
• Introducing new or changing existing unstable library APIs (excluding permanently unstable features / features without a tracking issue)
• Changing public documentation in ways that create new stability guarantees
• Changing observable runtime behavior of library APIs

[rust-highfive]

r? @thomcc

(rust-highfive has picked a reviewer for you, use r? to override)

[joshtriplett]

Rather than calling lossy string conversions, how about adding an example to the documentation showing how to print the error output to stderr without converting it? That just requires a call to write_all.

[beyarkay]

Happy to remove the lossy conversion, although I initially favoured using the assert_eq since it'll be run as a unit test (I believe?). I've re-written the example below to remove the lossy conversion but keep the assert.

I haven't used stdout much before. Just to make sure I understand you correctly, would the example look something like this (link to playground):

#![allow(unused)]
use std::io::{self, Stdout, Write};
use std::process::Command;

fn main() -> io::Result<()> {
    let output = if cfg!(target_os = "windows") {
        Command::new("cmd")
            .args(["/C", "echo hello"])
            .output()
            .expect("failed to execute process")
    } else {
        Command::new("sh")
            .arg("-c")
            .arg("echo hello")
            .output()
            .expect("failed to execute process")
    };

    // Note that `output.stdout` is a slice of bytes
    assert_eq!(&output.stdout, b"hello\n");

    // It can be written to stdout as a string either via stdout.write_all:
    let mut stdout = io::stdout().lock();
    stdout.write_all(&output.stdout)?;

    // Or via a lossy conversion:
    assert_eq!(String::from_utf8_lossy(&output.stdout), "hello\n");

    Ok(())
}

Possibly excluding the assert_eq, comments, or println!s, depending on what you think?

Thanks for the help!

[beyarkay]

@rustbot label +S-waiting-on-review -S-waiting-on-author

[thomcc]

Rather than calling lossy string conversions, how about adding an example to the documentation showing how to print the error output to stderr without converting it? That just requires a call to write_all.

IIRC that fails on windows if it's not UTF-8, FWIW. Although I guess you probably wouldn't get UTF-8 input there (although you could from a program with a different codepage, presumably).

Anyway, I've done or seen the lossy converstion dance here a jillion times and can't think of a case where I've seen the direct logging to stderr, so I think this is fine as is.

@bors r+ rollup

[bors]

📌 Commit 988a7e7 has been approved by thomcc

It is now in the queue for this repository.

[beyarkay]

Thanks!

IIRC that fails on windows if it's not UTF-8, FWIW

Didn't know that, good to know 🤓

[Dylan-DPC]

fails in rollup

@bors r-

[matthiaskrgr]

also failed earlier here: #99439 (comment)

[beyarkay]

So based on rust-log-analyzer:

stderr:
thread 'main' panicked at 'assertion failed: `(left == right)`
  left: `"hello\r\n"`,
 right: `"hello\n"`', src\process.rs:19:1

It seems like the failure was at std/process:19 which wasn't modified in my commit, although I do include a similar assertion.

Happy to modify my changes, but I'm not sure if they were the culprit?

[beyarkay]

Sorry, still getting used to the rust PR system. Is there anything I need to do to make this commit merge-able?

Thanks!

[thomcc]

It failed in CI when merging, you need to make sure it passes the tests that failed.

[JohnCSimon]

@beyarkay
Triage: what is the status of this PR?

FYI: when a PR is ready for review, send a message containing
@rustbot ready to switch to S-waiting-on-review so the PR is in the reviewer's backlog.

[beyarkay]

Okay I've hopefully fixed the issue causing the assertions to sometimes fail. My knowledge of OS-specific line endings isn't great, but it looks like on some builds the assertion would fail because I used \n but it was expecting \r\n.

Side note, I got confused by this line (reproduced below):

failures:

---- src\process.rs - process::Command (line 461) stdout ----
Test executable failed (exit code: 101).

stderr:
thread 'main' panicked at 'assertion failed: `(left == right)`
  left: `"hello\r\n"`,
 right: `"hello\n"`', src\process.rs:19:1                           <--- This line
stack backtrace:

Because it reads to me like the error was on line 19 (which made me think it wasn't something I'd caused) but on closer reading it's referring to line 461 which was the issue I'd caused.

I also see that line 19 is the last line of the first example code block in process.rs. Any chance a friendly rustacian could explain why the output refers to line 19?

@rustbot label +S-waiting-on-review -S-waiting-on-author

[rustbot]

Error: The "Ready" shortcut only works on pull requests.

Please file an issue on GitHub at triagebot if there's a problem with this bot, or reach out on #t-infra on Zulip.

[thomcc]

This looks fine, but you have a merge commit in the history, which we don't allow (See https://rustc-dev-guide.rust-lang.org/git.html#no-merge-policy). Can you clean up the git history?

[JohnCSimon]

triage:
@beyarkay ☝️

[JohnCSimon]

@beyarkay
Ping from triage: I'm closing this due to inactivity, Please reopen when you are ready to continue with this.
Note: if you do please open the PR BEFORE you push to it, else you won't be able to reopen - this is a quirk of github.
Thanks for your contribution.

@rustbot label: +S-inactive