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

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

Merged
merged 2 commits into from
Nov 13, 2022
Merged
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/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