Skip to content
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.

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

Output hygiene: write to stderr by default #5049

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions src/bin/cargo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -161,26 +161,26 @@ fn execute(flags: Flags, config: &mut Config) -> CliResult {

if flags.flag_version {
let version = cargo::version();
println!("{}", version);
eprintln!("{}", version);
if flags.flag_verbose > 0 {
println!("release: {}.{}.{}",
eprintln!("release: {}.{}.{}",
version.major,
version.minor,
version.patch);
if let Some(ref cfg) = version.cfg_info {
if let Some(ref ci) = cfg.commit_info {
println!("commit-hash: {}", ci.commit_hash);
println!("commit-date: {}", ci.commit_date);
eprintln!("commit-hash: {}", ci.commit_hash);
eprintln!("commit-date: {}", ci.commit_date);
}
}
}
return Ok(());
}

if flags.flag_list {
println!("Installed Commands:");
eprintln!("Installed Commands:");
for command in list_commands(config) {
println!(" {}", command);
eprintln!(" {}", command);
}
return Ok(());
}
Expand Down
2 changes: 1 addition & 1 deletion src/bin/login.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ pub fn execute(options: Options, config: &mut Config) -> CliResult {
options.flag_host.clone().unwrap_or(config.api.unwrap())
}
};
println!("please visit {}me and paste the API Token below", host);
eprintln!("please visit {}me and paste the API Token below", host);
let mut line = String::new();
let input = io::stdin();
input.lock().read_line(&mut line).chain_err(|| {
Expand Down
2 changes: 1 addition & 1 deletion src/bin/pkgid.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ pub fn execute(options: Options, config: &mut Config) -> CliResult {
};
let spec = spec.as_ref().map(|s| &s[..]);
let spec = ops::pkgid(&ws, spec)?;
println!("{}", spec);
eprintln!("{}", spec);
Ok(())
}

2 changes: 1 addition & 1 deletion src/bin/version.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ Options:
pub fn execute(_: Options, _: &mut Config) -> CliResult {
debug!("executing; cmd=cargo-version; args={:?}", env::args().collect::<Vec<_>>());

println!("{}", cargo::version());
eprintln!("{}", cargo::version());

Ok(())
}
2 changes: 1 addition & 1 deletion src/cargo/core/manifest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -355,7 +355,7 @@ impl Manifest {
pub fn print_teapot(&self, config: &Config) {
if let Some(teapot) = self.im_a_teapot {
if config.cli_unstable().print_im_a_teapot {
println!("im-a-teapot = {}", teapot);
eprintln!("im-a-teapot = {}", teapot);
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/cargo/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ pub fn exit_with_error(err: CliError, shell: &mut Shell) -> ! {
} else if fatal {
drop(shell.error(&error))
} else {
println!("{}", error);
eprintln!("{}", error);
}

if !handle_cause(&error, shell) || hide {
Expand Down
4 changes: 2 additions & 2 deletions src/cargo/ops/cargo_install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -558,9 +558,9 @@ pub fn install_list(dst: Option<&str>, config: &Config) -> CargoResult<()> {
let dst = metadata(config, &dst)?;
let list = read_crate_list(&dst)?;
for (k, v) in list.v1.iter() {
println!("{}:", k);
eprintln!("{}:", k);
for bin in v {
println!(" {}", bin);
eprintln!(" {}", bin);
}
}
Ok(())
Expand Down
2 changes: 1 addition & 1 deletion src/cargo/ops/cargo_new.rs
Original file line number Diff line number Diff line change
Expand Up @@ -493,7 +493,7 @@ authors = [{}]
let default_file_content : &[u8] = if i.bin {
b"\
fn main() {
println!(\"Hello, world!\");
eprintln!(\"Hello, world!\");
}
"
} else {
Expand Down
2 changes: 1 addition & 1 deletion src/cargo/ops/cargo_package.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ pub fn package(ws: &Workspace,
}).collect();
list.sort();
for file in list.iter() {
println!("{}", file.display());
eprintln!("{}", file.display());
Copy link
Member

Choose a reason for hiding this comment

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

This is machine-consumable, and is the only way to get the list of files that would be packaged AFAIK.

Copy link
Author

@sanmai-NL sanmai-NL Feb 17, 2018

Choose a reason for hiding this comment

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

I concur. However, this PR isn’t so much about distinguishing machine-readable output from human-readable output across Cargo. It is rather about distinguishing standardized, exclusively machine-readable output from ‘other’ output. So can’t we make principled decision about what ‘machine-readable’ means within Cargo? If that means making all such output JSON, then let’s do that rather than making local exceptions left and right.

Copy link
Member

Choose a reason for hiding this comment

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

The change as-is will break code that calls this command and expects to see something on stdout.

}
return Ok(None)
}
Expand Down
3 changes: 1 addition & 2 deletions src/cargo/ops/cargo_rustc/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ use std::collections::{HashMap, HashSet};
use std::env;
use std::ffi::{OsStr, OsString};
use std::fs;
use std::io::{self, Write};
use std::path::{self, PathBuf};
use std::sync::Arc;

Expand Down Expand Up @@ -415,7 +414,7 @@ fn rustc<'a, 'cfg>(cx: &mut Context<'a, 'cfg>,
});
} else {
// Forward non-JSON to stderr
writeln!(io::stderr(), "{}", line)?;
eprintln!("{}", line);
}
Ok(())
}
Expand Down
14 changes: 7 additions & 7 deletions src/cargo/ops/registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -432,12 +432,12 @@ pub fn modify_owners(config: &Config, opts: &OwnersOptions) -> CargoResult<()> {
format!("failed to list owners of crate {}", name)
})?;
for owner in owners.iter() {
print!("{}", owner.login);
eprint!("{}", owner.login);
match (owner.name.as_ref(), owner.email.as_ref()) {
(Some(name), Some(email)) => println!(" ({} <{}>)", name, email),
(Some(name), Some(email)) => eprintln!(" ({} <{}>)", name, email),
(Some(s), None) |
(None, Some(s)) => println!(" ({})", s),
(None, None) => println!(),
(None, Some(s)) => eprintln!(" ({})", s),
(None, None) => eprintln!(),
}
}
}
Expand Down Expand Up @@ -529,15 +529,15 @@ pub fn search(query: &str,
}
None => name
};
println!("{}", line);
eprintln!("{}", line);
}

let search_max_limit = 100;
if total_crates > u32::from(limit) && limit < search_max_limit {
println!("... and {} crates more (use --limit N to see more)",
eprintln!("... and {} crates more (use --limit N to see more)",
total_crates - u32::from(limit));
} else if total_crates > u32::from(limit) && limit >= search_max_limit {
println!("... and {} crates more (go to http://crates.io/search?q={} to see more)",
eprintln!("... and {} crates more (go to http://crates.io/search?q={} to see more)",
total_crates - u32::from(limit),
percent_encode(query.as_bytes(), QUERY_ENCODE_SET));
}
Expand Down
2 changes: 1 addition & 1 deletion src/cargo/util/profile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ impl Drop for Profiler {
let mut last = 0;
for (i, &(l, time, ref msg)) in msgs.iter().enumerate() {
if l != lvl { continue }
println!("{} {:6}ms - {}",
eprintln!("{} {:6}ms - {}",
repeat(" ").take(lvl + 1).collect::<String>(),
time, msg);

Expand Down
2 changes: 1 addition & 1 deletion src/doc/src/getting-started/first-steps.md
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ Here’s what’s in `src/main.rs`:

```rust
fn main() {
println!("Hello, world!");
eprintln!("Hello, world!");
}
```

Expand Down
2 changes: 1 addition & 1 deletion src/doc/src/guide/creating-a-new-project.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ Here’s what’s in `src/main.rs`:

```rust
fn main() {
println!("Hello, world!");
eprintln!("Hello, world!");
}
```

Expand Down
2 changes: 1 addition & 1 deletion src/doc/src/guide/dependencies.md
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ use regex::Regex;

fn main() {
let re = Regex::new(r"^\d{4}-\d{2}-\d{2}$").unwrap();
println!("Did our date match? {}", re.is_match("2014-01-01"));
eprintln!("Did our date match? {}", re.is_match("2014-01-01"));
}
```

Expand Down
2 changes: 1 addition & 1 deletion src/doc/src/reference/build-scripts.md
Original file line number Diff line number Diff line change
Expand Up @@ -285,7 +285,7 @@ Next, let’s peek at the library itself:
include!(concat!(env!("OUT_DIR"), "/hello.rs"));

fn main() {
println!("{}", message());
eprintln!("{}", message());
}
```

Expand Down
8 changes: 4 additions & 4 deletions tests/build-script.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2561,7 +2561,7 @@ fn switch_features_rerun() {
"#)
.file("src/main.rs", r#"
fn main() {
println!(include_str!(concat!(env!("OUT_DIR"), "/output")));
eprintln!(include_str!(concat!(env!("OUT_DIR"), "/output")));
}
"#)
.file("build.rs", r#"
Expand All @@ -2585,11 +2585,11 @@ fn switch_features_rerun() {
.build();

assert_that(p.cargo("run").arg("-v").arg("--features=foo"),
execs().with_status(0).with_stdout("foo\n"));
execs().with_status(0).with_stderr_contains("foo\n"));
assert_that(p.cargo("run").arg("-v"),
execs().with_status(0).with_stdout("bar\n"));
execs().with_status(0).with_stderr_contains("bar\n"));
assert_that(p.cargo("run").arg("-v").arg("--features=foo"),
execs().with_status(0).with_stdout("foo\n"));
execs().with_status(0).with_stderr_contains("foo\n"));
}

#[test]
Expand Down
6 changes: 1 addition & 5 deletions tests/cargo-features.rs
Original file line number Diff line number Diff line change
Expand Up @@ -244,11 +244,7 @@ error: unknown `-Z` flag specified: arg
.masquerade_as_nightly_cargo()
.arg("-Zprint-im-a-teapot"),
execs().with_status(0)
.with_stdout("im-a-teapot = true\n")
.with_stderr("\
[COMPILING] a [..]
[FINISHED] [..]
"));
.with_stderr_contains("im-a-teapot = true\n"));
}

#[test]
Expand Down
4 changes: 2 additions & 2 deletions tests/cargo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ fn list_command_looks_at_path() {
let output = pr.arg("-v").arg("--list")
.env("PATH", &path);
let output = output.exec_with_output().unwrap();
let output = str::from_utf8(&output.stdout).unwrap();
let output = str::from_utf8(&output.stderr).unwrap();
assert!(output.contains("\n 1\n"), "missing 1: {}", output);
}

Expand All @@ -94,7 +94,7 @@ fn list_command_resolves_symlinks() {
let output = pr.arg("-v").arg("--list")
.env("PATH", &path);
let output = output.exec_with_output().unwrap();
let output = str::from_utf8(&output.stdout).unwrap();
let output = str::from_utf8(&output.stderr).unwrap();
assert!(output.contains("\n 2\n"), "missing 2: {}", output);
}

Expand Down
2 changes: 1 addition & 1 deletion tests/cargotest/support/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -760,7 +760,7 @@ impl ham::Matcher<ProcessBuilder> for Execs {

impl<'a> ham::Matcher<&'a mut ProcessBuilder> for Execs {
fn matches(&self, process: &'a mut ProcessBuilder) -> ham::MatchResult {
println!("running {}", process);
eprintln!("running {}", process);
let res = process.exec_with_output();

match res {
Expand Down
6 changes: 3 additions & 3 deletions tests/cross-compile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -436,9 +436,9 @@ fn cross_tests() {
[COMPILING] foo v0.0.0 ({foo})
[FINISHED] dev [unoptimized + debuginfo] target(s) in [..]
[RUNNING] target[/]{triple}[/]debug[/]deps[/]foo-[..][EXE]
[RUNNING] target[/]{triple}[/]debug[/]deps[/]bar-[..][EXE]", foo = p.url(), triple = target))
.with_stdout_contains("test test_foo ... ok")
.with_stdout_contains("test test ... ok"));
test test_foo ... ok
[RUNNING] target[/]{triple}[/]debug[/]deps[/]bar-[..][EXE]
test test ... ok", foo = p.url(), triple = target)));
}

#[test]
Expand Down
4 changes: 2 additions & 2 deletions tests/features.rs
Original file line number Diff line number Diff line change
Expand Up @@ -377,7 +377,7 @@ fn no_feature_doesnt_build() {
#[cfg(feature = "bar")]
extern crate bar;
#[cfg(feature = "bar")]
fn main() { bar::bar(); println!("bar") }
fn main() { bar::bar(); eprintln!("bar") }
#[cfg(not(feature = "bar"))]
fn main() {}
"#)
Expand Down Expand Up @@ -405,7 +405,7 @@ fn no_feature_doesnt_build() {
[FINISHED] dev [unoptimized + debuginfo] target(s) in [..]
", dir = p.url())));
assert_that(p.process(&p.bin("foo")),
execs().with_status(0).with_stdout("bar\n"));
execs().with_status(0).with_stderr("bar\n"));
}

#[test]
Expand Down
18 changes: 9 additions & 9 deletions tests/install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -462,7 +462,7 @@ warning: be sure to add `[..]` to your PATH to be able to run the installed bina
home = cargo_home().display())));

assert_that(cargo_process("install").arg("--list"),
execs().with_status(0).with_stdout("\
execs().with_status(0).with_stderr("\
foo v0.2.0 ([..]):
foo[..]
"));
Expand Down Expand Up @@ -507,7 +507,7 @@ warning: be sure to add `[..]` to your PATH to be able to run the installed bina
home = cargo_home().display())));

assert_that(cargo_process("install").arg("--list"),
execs().with_status(0).with_stdout("\
execs().with_status(0).with_stderr("\
foo v0.1.0 ([..]):
foo-bin1[..]
foo v0.2.0 ([..]):
Expand Down Expand Up @@ -558,7 +558,7 @@ warning: be sure to add `[..]` to your PATH to be able to run the installed bina
home = cargo_home().display())));

assert_that(cargo_process("install").arg("--list"),
execs().with_status(0).with_stdout("\
execs().with_status(0).with_stderr("\
foo v0.1.0 ([..]):
foo-bin1[..]
foo v0.2.0 ([..]):
Expand Down Expand Up @@ -631,7 +631,7 @@ fn list() {
assert_that(cargo_process("install").arg("foo"),
execs().with_status(0));
assert_that(cargo_process("install").arg("--list"),
execs().with_status(0).with_stdout("\
execs().with_status(0).with_stderr("\
bar v0.2.1:
bar[..]
foo v0.0.1:
Expand All @@ -645,7 +645,7 @@ fn list_error() {
assert_that(cargo_process("install").arg("foo"),
execs().with_status(0));
assert_that(cargo_process("install").arg("--list"),
execs().with_status(0).with_stdout("\
execs().with_status(0).with_stderr("\
foo v0.0.1:
foo[..]
"));
Expand Down Expand Up @@ -732,16 +732,16 @@ fn subcommand_works_out_of_the_box() {
Package::new("cargo-foo", "1.0.0")
.file("src/main.rs", r#"
fn main() {
println!("bar");
eprintln!("bar");
}
"#)
.publish();
assert_that(cargo_process("install").arg("cargo-foo"),
execs().with_status(0));
assert_that(cargo_process("foo"),
execs().with_status(0).with_stdout("bar\n"));
execs().with_status(0).with_stderr("bar\n"));
assert_that(cargo_process("--list"),
execs().with_status(0).with_stdout_contains(" foo\n"));
execs().with_status(0).with_stderr_contains(" foo\n"));
}

#[test]
Expand Down Expand Up @@ -799,7 +799,7 @@ fn reports_unsuccessful_subcommand_result() {
assert_that(cargo_process("install").arg("cargo-fail"),
execs().with_status(0));
assert_that(cargo_process("--list"),
execs().with_status(0).with_stdout_contains(" fail\n"));
execs().with_status(0).with_stderr_contains(" fail\n"));
assert_that(cargo_process("fail"),
execs().with_status(101).with_stderr_contains("\
thread '[..]' panicked at 'explicit panic', [..]
Expand Down
Loading