Skip to content

build_helper: try less confusing method names #63196

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

Merged
merged 3 commits into from
Aug 3, 2019
Merged

build_helper: try less confusing method names #63196

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
79a1e7a
build_helper: rename run -> run_verbose (seems unused)
RalfJung Aug 1, 2019
0d8c97b
build_helper: rename (try_)run_silent -> (try_)run
RalfJung Aug 1, 2019
30f61de
replace what I think is an erroneous try_run_quiet by try_run
RalfJung Aug 1, 2019
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
6 changes: 3 additions & 3 deletions src/bootstrap/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ use std::os::unix::fs::symlink as symlink_file;
use std::os::windows::fs::symlink_file;

use build_helper::{
mtime, output, run_silent, run_suppressed, t, try_run_silent, try_run_suppressed,
mtime, output, run, run_suppressed, t, try_run, try_run_suppressed,
};
use filetime::FileTime;

Expand Down Expand Up @@ -682,7 +682,7 @@ impl Build {
fn run(&self, cmd: &mut Command) {
if self.config.dry_run { return; }
self.verbose(&format!("running: {:?}", cmd));
run_silent(cmd)
run(cmd)
}

/// Runs a command, printing out nice contextual information if it fails.
Expand All @@ -698,7 +698,7 @@ impl Build {
fn try_run(&self, cmd: &mut Command) -> bool {
if self.config.dry_run { return true; }
self.verbose(&format!("running: {:?}", cmd));
try_run_silent(cmd)
try_run(cmd)
}

/// Runs a command, printing out nice contextual information if it fails.
Expand Down
2 changes: 1 addition & 1 deletion src/bootstrap/test.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1527,7 +1527,7 @@ impl Step for RustcGuide {
fn run(self, builder: &Builder<'_>) {
let src = builder.src.join("src/doc/rustc-guide");
let mut rustbook_cmd = builder.tool_cmd(Tool::Rustbook);
try_run_quiet(builder, rustbook_cmd.arg("linkcheck").arg(&src));
try_run(builder, rustbook_cmd.arg("linkcheck").arg(&src));
Copy link
Member Author

@RalfJung RalfJung Aug 1, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

This change is based on this thread: I think we actually want to see the output here.

The only other user of try_run_quiet uses logic of the kind

    if builder.config.verbose_tests {
        try_run(builder, &mut cmd)
    } else {
        try_run_quiet(builder, &mut cmd)
    }

Most tools just use try_run.

}
}

Expand Down
11 changes: 6 additions & 5 deletions src/build_helper/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,18 +45,19 @@ pub fn restore_library_path() {
}
}

pub fn run(cmd: &mut Command) {
/// Run the command, printing what we are running.
pub fn run_verbose(cmd: &mut Command) {
println!("running: {:?}", cmd);
run_silent(cmd);
run(cmd);
}

pub fn run_silent(cmd: &mut Command) {
if !try_run_silent(cmd) {
pub fn run(cmd: &mut Command) {
if !try_run(cmd) {
std::process::exit(1);
}
}

pub fn try_run_silent(cmd: &mut Command) -> bool {
pub fn try_run(cmd: &mut Command) -> bool {
let status = match cmd.status() {
Ok(status) => status,
Err(e) => fail(&format!(
Expand Down