Skip to content

Commit

Permalink
Auto merge of #104078 - jyn514:dry-run-progress, r=Mark-Simulacrum
Browse files Browse the repository at this point in the history
Print "Checking/Building ..." message even when --dry-run is passed

Print "Checking/Building ..." message even when --dry-run is passed

This makes it a lot easier to understand what commands will be run without
having to parse the `-vv` output, which isn't meant to be user facing.

I also want to change these messages at some point (#102003) and this change will make it easier to paste a before/after comparison without having to actually build a stage 2 compiler.
  • Loading branch information
bors committed Nov 13, 2022
2 parents fb6667a + 2437888 commit 229e875
Show file tree
Hide file tree
Showing 15 changed files with 100 additions and 80 deletions.
12 changes: 6 additions & 6 deletions src/bootstrap/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1074,11 +1074,11 @@ impl<'a> Builder<'a> {
let mut hasher = sha2::Sha256::new();
// FIXME: this is ok for rustfmt (4.1 MB large at time of writing), but it seems memory-intensive for rustc and larger components.
// Consider using streaming IO instead?
let contents = if self.config.dry_run { vec![] } else { t!(fs::read(path)) };
let contents = if self.config.dry_run() { vec![] } else { t!(fs::read(path)) };
hasher.update(&contents);
let found = hex::encode(hasher.finalize().as_slice());
let verified = found == expected;
if !verified && !self.config.dry_run {
if !verified && !self.config.dry_run() {
println!(
"invalid checksum: \n\
found: {found}\n\
Expand Down Expand Up @@ -1292,7 +1292,7 @@ impl<'a> Builder<'a> {
/// Note that this returns `None` if LLVM is disabled, or if we're in a
/// check build or dry-run, where there's no need to build all of LLVM.
fn llvm_config(&self, target: TargetSelection) -> Option<PathBuf> {
if self.config.llvm_enabled() && self.kind != Kind::Check && !self.config.dry_run {
if self.config.llvm_enabled() && self.kind != Kind::Check && !self.config.dry_run() {
let llvm_config = self.ensure(native::Llvm { target });
if llvm_config.is_file() {
return Some(llvm_config);
Expand Down Expand Up @@ -1644,7 +1644,7 @@ impl<'a> Builder<'a> {
//
// Only clear out the directory if we're compiling std; otherwise, we
// should let Cargo take care of things for us (via depdep info)
if !self.config.dry_run && mode == Mode::Std && cmd == "build" {
if !self.config.dry_run() && mode == Mode::Std && cmd == "build" {
self.clear_if_dirty(&out_dir, &self.rustc(compiler));
}

Expand Down Expand Up @@ -2142,7 +2142,7 @@ impl<'a> Builder<'a> {
(out, dur - deps)
};

if self.config.print_step_timings && !self.config.dry_run {
if self.config.print_step_timings && !self.config.dry_run() {
let step_string = format!("{:?}", step);
let brace_index = step_string.find("{").unwrap_or(0);
let type_string = type_name::<S>();
Expand Down Expand Up @@ -2216,7 +2216,7 @@ impl<'a> Builder<'a> {
}

pub(crate) fn open_in_browser(&self, path: impl AsRef<Path>) {
if self.config.dry_run || !self.config.cmd.open() {
if self.config.dry_run() || !self.config.cmd.open() {
return;
}

Expand Down
4 changes: 2 additions & 2 deletions src/bootstrap/builder/tests.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use super::*;
use crate::config::{Config, TargetSelection};
use crate::config::{Config, DryRun, TargetSelection};
use std::thread;

fn configure(cmd: &str, host: &[&str], target: &[&str]) -> Config {
Expand All @@ -10,7 +10,7 @@ fn configure_with_args(cmd: &[String], host: &[&str], target: &[&str]) -> Config
let mut config = Config::parse(cmd);
// don't save toolstates
config.save_toolstates = None;
config.dry_run = true;
config.dry_run = DryRun::SelfCheck;

// Ignore most submodules, since we don't need them for a dry run.
// But make sure to check out the `doc` and `rust-analyzer` submodules, since some steps need them
Expand Down
12 changes: 6 additions & 6 deletions src/bootstrap/compile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -447,7 +447,7 @@ fn copy_sanitizers(
) -> Vec<PathBuf> {
let runtimes: Vec<native::SanitizerRuntime> = builder.ensure(native::Sanitizers { target });

if builder.config.dry_run {
if builder.config.dry_run() {
return Vec::new();
}

Expand Down Expand Up @@ -986,7 +986,7 @@ impl Step for CodegenBackend {
compiler.stage, backend, &compiler.host, target
));
let files = run_cargo(builder, cargo, vec![], &tmp_stamp, vec![], false);
if builder.config.dry_run {
if builder.config.dry_run() {
return;
}
let mut files = files.into_iter().filter(|f| {
Expand Down Expand Up @@ -1034,7 +1034,7 @@ fn copy_codegen_backends_to_sysroot(
let dst = builder.sysroot_codegen_backends(target_compiler);
t!(fs::create_dir_all(&dst), dst);

if builder.config.dry_run {
if builder.config.dry_run() {
return;
}

Expand Down Expand Up @@ -1332,7 +1332,7 @@ impl Step for Assemble {

if builder.config.rust_codegen_backends.contains(&INTERNER.intern_str("llvm")) {
let llvm_config_bin = builder.ensure(native::Llvm { target: target_compiler.host });
if !builder.config.dry_run {
if !builder.config.dry_run() {
let llvm_bin_dir = output(Command::new(llvm_config_bin).arg("--bindir"));
let llvm_bin_dir = Path::new(llvm_bin_dir.trim());

Expand Down Expand Up @@ -1402,7 +1402,7 @@ pub fn run_cargo(
additional_target_deps: Vec<(PathBuf, DependencyType)>,
is_check: bool,
) -> Vec<PathBuf> {
if builder.config.dry_run {
if builder.config.dry_run() {
return Vec::new();
}

Expand Down Expand Up @@ -1542,7 +1542,7 @@ pub fn stream_cargo(
cb: &mut dyn FnMut(CargoMessage<'_>),
) -> bool {
let mut cargo = Command::from(cargo);
if builder.config.dry_run {
if builder.config.dry_run() {
return true;
}
// Instruct Cargo to give us json messages on stdout, critically leaving
Expand Down
30 changes: 24 additions & 6 deletions src/bootstrap/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,17 @@ macro_rules! check_ci_llvm {
};
}

#[derive(Clone, Default)]
pub enum DryRun {
/// This isn't a dry run.
#[default]
Disabled,
/// This is a dry run enabled by bootstrap itself, so it can verify that no work is done.
SelfCheck,
/// This is a dry run enabled by the `--dry-run` flag.
UserSelected,
}

/// Global configuration for the entire build and/or bootstrap.
///
/// This structure is derived from a combination of both `config.toml` and
Expand Down Expand Up @@ -84,7 +95,7 @@ pub struct Config {
pub jobs: Option<u32>,
pub cmd: Subcommand,
pub incremental: bool,
pub dry_run: bool,
pub dry_run: DryRun,
/// `None` if we shouldn't download CI compiler artifacts, or the commit to download if we should.
#[cfg(not(test))]
download_rustc_commit: Option<String>,
Expand Down Expand Up @@ -820,7 +831,7 @@ impl Config {
config.jobs = flags.jobs.map(threads_from_config);
config.cmd = flags.cmd;
config.incremental = flags.incremental;
config.dry_run = flags.dry_run;
config.dry_run = if flags.dry_run { DryRun::UserSelected } else { DryRun::Disabled };
config.keep_stage = flags.keep_stage;
config.keep_stage_std = flags.keep_stage_std;
config.color = flags.color;
Expand Down Expand Up @@ -965,7 +976,7 @@ impl Config {
.unwrap_or_else(|| config.out.join(config.build.triple).join("stage0/bin/cargo"));

// NOTE: it's important this comes *after* we set `initial_rustc` just above.
if config.dry_run {
if config.dry_run() {
let dir = config.out.join("tmp-dry-run");
t!(fs::create_dir_all(&dir));
config.out = dir;
Expand Down Expand Up @@ -1372,6 +1383,13 @@ impl Config {
config
}

pub(crate) fn dry_run(&self) -> bool {
match self.dry_run {
DryRun::Disabled => false,
DryRun::SelfCheck | DryRun::UserSelected => true,
}
}

/// A git invocation which runs inside the source directory.
///
/// Use this rather than `Command::new("git")` in order to support out-of-tree builds.
Expand Down Expand Up @@ -1461,7 +1479,7 @@ impl Config {
/// This is computed on demand since LLVM might have to first be downloaded from CI.
pub(crate) fn llvm_link_shared(builder: &Builder<'_>) -> bool {
let mut opt = builder.config.llvm_link_shared.get();
if opt.is_none() && builder.config.dry_run {
if opt.is_none() && builder.config.dry_run() {
// just assume static for now - dynamic linking isn't supported on all platforms
return false;
}
Expand All @@ -1488,7 +1506,7 @@ impl Config {
/// Return whether we will use a downloaded, pre-compiled version of rustc, or just build from source.
pub(crate) fn download_rustc(builder: &Builder<'_>) -> bool {
static DOWNLOAD_RUSTC: OnceCell<bool> = OnceCell::new();
if builder.config.dry_run && DOWNLOAD_RUSTC.get().is_none() {
if builder.config.dry_run() && DOWNLOAD_RUSTC.get().is_none() {
// avoid trying to actually download the commit
return false;
}
Expand All @@ -1507,7 +1525,7 @@ impl Config {
RustfmtState::SystemToolchain(p) | RustfmtState::Downloaded(p) => Some(p.clone()),
RustfmtState::Unavailable => None,
r @ RustfmtState::LazyEvaluated => {
if builder.config.dry_run {
if builder.config.dry_run() {
return Some(PathBuf::new());
}
let path = maybe_download_rustfmt(builder);
Expand Down
12 changes: 6 additions & 6 deletions src/bootstrap/dist.rs
Original file line number Diff line number Diff line change
Expand Up @@ -945,7 +945,7 @@ impl Step for PlainSourceTarball {
.arg(builder.src.join("./src/bootstrap/Cargo.toml"))
.current_dir(&plain_dst_src);

let config = if !builder.config.dry_run {
let config = if !builder.config.dry_run() {
t!(String::from_utf8(t!(cmd.output()).stdout))
} else {
String::new()
Expand Down Expand Up @@ -1386,7 +1386,7 @@ impl Step for Extended {
let etc = builder.src.join("src/etc/installer");

// Avoid producing tarballs during a dry run.
if builder.config.dry_run {
if builder.config.dry_run() {
return;
}

Expand Down Expand Up @@ -1818,7 +1818,7 @@ impl Step for Extended {
let _time = timeit(builder);
builder.run(&mut cmd);

if !builder.config.dry_run {
if !builder.config.dry_run() {
t!(fs::rename(exe.join(&filename), distdir(builder).join(&filename)));
}
}
Expand Down Expand Up @@ -1882,12 +1882,12 @@ fn maybe_install_llvm(builder: &Builder<'_>, target: TargetSelection, dst_libdir
if llvm_dylib_path.exists() {
builder.install(&llvm_dylib_path, dst_libdir, 0o644);
}
!builder.config.dry_run
!builder.config.dry_run()
} else if let Ok(llvm_config) = crate::native::prebuilt_llvm_config(builder, target) {
let mut cmd = Command::new(llvm_config);
cmd.arg("--libfiles");
builder.verbose(&format!("running {:?}", cmd));
let files = if builder.config.dry_run { "".into() } else { output(&mut cmd) };
let files = if builder.config.dry_run() { "".into() } else { output(&mut cmd) };
let build_llvm_out = &builder.llvm_out(builder.config.build);
let target_llvm_out = &builder.llvm_out(target);
for file in files.trim_end().split(' ') {
Expand All @@ -1899,7 +1899,7 @@ fn maybe_install_llvm(builder: &Builder<'_>, target: TargetSelection, dst_libdir
};
builder.install(&file, dst_libdir, 0o644);
}
!builder.config.dry_run
!builder.config.dry_run()
} else {
false
}
Expand Down
10 changes: 5 additions & 5 deletions src/bootstrap/doc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ impl Step for RustbookSrc {
let index = out.join("index.html");
let rustbook = builder.tool_exe(Tool::Rustbook);
let mut rustbook_cmd = builder.tool_cmd(Tool::Rustbook);
if builder.config.dry_run || up_to_date(&src, &index) && up_to_date(&rustbook, &index) {
if builder.config.dry_run() || up_to_date(&src, &index) && up_to_date(&rustbook, &index) {
return;
}
builder.info(&format!("Rustbook ({}) - {}", target, name));
Expand Down Expand Up @@ -331,8 +331,8 @@ impl Step for Standalone {
&& up_to_date(&footer, &html)
&& up_to_date(&favicon, &html)
&& up_to_date(&full_toc, &html)
&& (builder.config.dry_run || up_to_date(&version_info, &html))
&& (builder.config.dry_run || up_to_date(&rustdoc, &html))
&& (builder.config.dry_run() || up_to_date(&version_info, &html))
&& (builder.config.dry_run() || up_to_date(&rustdoc, &html))
{
continue;
}
Expand Down Expand Up @@ -402,7 +402,7 @@ impl Step for SharedAssets {

let version_input = builder.src.join("src").join("doc").join("version_info.html.template");
let version_info = out.join("version_info.html");
if !builder.config.dry_run && !up_to_date(&version_input, &version_info) {
if !builder.config.dry_run() && !up_to_date(&version_input, &version_info) {
let info = t!(fs::read_to_string(&version_input))
.replace("VERSION", &builder.rust_release())
.replace("SHORT_HASH", builder.rust_info.sha_short().unwrap_or(""))
Expand Down Expand Up @@ -900,7 +900,7 @@ impl Step for UnstableBookGen {
}

fn symlink_dir_force(config: &Config, src: &Path, dst: &Path) -> io::Result<()> {
if config.dry_run {
if config.dry_run() {
return Ok(());
}
if let Ok(m) = fs::symlink_metadata(dst) {
Expand Down
2 changes: 1 addition & 1 deletion src/bootstrap/format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ struct RustfmtConfig {
}

pub fn format(build: &Builder<'_>, check: bool, paths: &[PathBuf]) {
if build.config.dry_run {
if build.config.dry_run() {
return;
}
let mut builder = ignore::types::TypesBuilder::new();
Expand Down
Loading

0 comments on commit 229e875

Please sign in to comment.