Skip to content

Commit

Permalink
Rollup merge of #104952 - jyn514:setup, r=Mark-Simulacrum
Browse files Browse the repository at this point in the history
Streamline the user experience for `x.py setup`

## Don't update submodules for x setup

Before, the submodule handling was very jank and would update *between two interactive prompts*:
```
; x setup
Building rustbuild
    Finished dev [unoptimized] target(s) in 0.05s
Welcome to the Rust project! What do you want to do with x.py?
a) library: Contribute to the standard library
Please choose one (a/b/c/d/e): a
Updating submodule library/backtrace
Submodule 'library/backtrace' (https://github.com/rust-lang/backtrace-rs.git) registered for path 'library/backtrace'
error: you asked `x.py` to setup a new config file, but one already exists at `config.toml`
Build completed unsuccessfully in 0:00:02
```

That's not a great user experience because you need to wait a long time between prompts.
It would be possible to move the submodule handling either before or after the prompt, but it seems
better to just not require submodules to be checked out at all, to minimize the time spend waiting
just to create a new configuration.

## Revamp the order setup executes

- Create `config.toml` last. It's the most likely to error, and used to stop later steps from executing
- Don't print an error message + exit if the git hook already exists; that's expected
  • Loading branch information
matthiaskrgr authored Dec 5, 2022
2 parents 8ad447c + b771d90 commit 4b6010c
Show file tree
Hide file tree
Showing 3 changed files with 77 additions and 67 deletions.
9 changes: 5 additions & 4 deletions src/bootstrap/flags.rs
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ pub enum Subcommand {
args: Vec<String>,
},
Setup {
profile: Profile,
profile: Option<Profile>,
},
}

Expand Down Expand Up @@ -628,14 +628,15 @@ Arguments:
|path| format!("{} is not a valid UTF8 string", path.to_string_lossy())
));

profile_string.parse().unwrap_or_else(|err| {
let profile = profile_string.parse().unwrap_or_else(|err| {
eprintln!("error: {}", err);
eprintln!("help: the available profiles are:");
eprint!("{}", Profile::all_for_help("- "));
crate::detail_exit(1);
})
});
Some(profile)
} else {
t!(crate::setup::interactive_path())
None
};
Subcommand::Setup { profile }
}
Expand Down
44 changes: 25 additions & 19 deletions src/bootstrap/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -542,16 +542,6 @@ impl Build {
metrics: metrics::BuildMetrics::init(),
};

build.verbose("finding compilers");
cc_detect::find(&mut build);
// When running `setup`, the profile is about to change, so any requirements we have now may
// be different on the next invocation. Don't check for them until the next time x.py is
// run. This is ok because `setup` never runs any build commands, so it won't fail if commands are missing.
if !matches!(build.config.cmd, Subcommand::Setup { .. }) {
build.verbose("running sanity check");
sanity::check(&mut build);
}

// If local-rust is the same major.minor as the current version, then force a
// local-rebuild
let local_version_verbose =
Expand All @@ -567,16 +557,32 @@ impl Build {
build.local_rebuild = true;
}

// Make sure we update these before gathering metadata so we don't get an error about missing
// Cargo.toml files.
let rust_submodules =
["src/tools/rust-installer", "src/tools/cargo", "library/backtrace", "library/stdarch"];
for s in rust_submodules {
build.update_submodule(Path::new(s));
}
build.verbose("finding compilers");
cc_detect::find(&mut build);
// When running `setup`, the profile is about to change, so any requirements we have now may
// be different on the next invocation. Don't check for them until the next time x.py is
// run. This is ok because `setup` never runs any build commands, so it won't fail if commands are missing.
//
// Similarly, for `setup` we don't actually need submodules or cargo metadata.
if !matches!(build.config.cmd, Subcommand::Setup { .. }) {
build.verbose("running sanity check");
sanity::check(&mut build);

build.verbose("learning about cargo");
metadata::build(&mut build);
// Make sure we update these before gathering metadata so we don't get an error about missing
// Cargo.toml files.
let rust_submodules = [
"src/tools/rust-installer",
"src/tools/cargo",
"library/backtrace",
"library/stdarch",
];
for s in rust_submodules {
build.update_submodule(Path::new(s));
}

build.verbose("learning about cargo");
metadata::build(&mut build);
}

build
}
Expand Down
91 changes: 47 additions & 44 deletions src/bootstrap/setup.rs
Original file line number Diff line number Diff line change
@@ -1,15 +1,13 @@
use crate::Config;
use crate::{t, VERSION};
use crate::{Config, TargetSelection};
use std::env::consts::EXE_SUFFIX;
use std::fmt::Write as _;
use std::fs::File;
use std::io::Write;
use std::path::{Path, PathBuf, MAIN_SEPARATOR};
use std::process::Command;
use std::str::FromStr;
use std::{
env, fmt, fs,
io::{self, Write},
};
use std::{fmt, fs, io};

#[derive(Clone, Copy, Debug, Eq, PartialEq)]
pub enum Profile {
Expand Down Expand Up @@ -81,38 +79,10 @@ impl fmt::Display for Profile {
}
}

pub fn setup(config: &Config, profile: Profile) {
let path = &config.config.clone().unwrap_or(PathBuf::from("config.toml"));

if path.exists() {
eprintln!(
"error: you asked `x.py` to setup a new config file, but one already exists at `{}`",
path.display()
);
eprintln!("help: try adding `profile = \"{}\"` at the top of {}", profile, path.display());
eprintln!(
"note: this will use the configuration in {}",
profile.include_path(&config.src).display()
);
crate::detail_exit(1);
}

let settings = format!(
"# Includes one of the default files in src/bootstrap/defaults\n\
profile = \"{}\"\n\
changelog-seen = {}\n",
profile, VERSION
);
t!(fs::write(path, settings));

let include_path = profile.include_path(&config.src);
println!("`x.py` will now use the configuration at {}", include_path.display());

let build = TargetSelection::from_user(&env!("BUILD_TRIPLE"));
pub fn setup(config: &Config, profile: Option<Profile>) {
let profile = profile.unwrap_or_else(|| t!(interactive_path()));
let stage_path =
["build", build.rustc_target_arg(), "stage1"].join(&MAIN_SEPARATOR.to_string());

println!();
["build", config.build.rustc_target_arg(), "stage1"].join(&MAIN_SEPARATOR.to_string());

if !rustup_installed() && profile != Profile::User {
eprintln!("`rustup` is not installed; cannot link `stage1` toolchain");
Expand All @@ -134,8 +104,6 @@ pub fn setup(config: &Config, profile: Profile) {
Profile::User => &["dist", "build"],
};

println!();

t!(install_git_hook_maybe(&config));

println!();
Expand All @@ -150,6 +118,36 @@ pub fn setup(config: &Config, profile: Profile) {
"For more suggestions, see https://rustc-dev-guide.rust-lang.org/building/suggested.html"
);
}

let path = &config.config.clone().unwrap_or(PathBuf::from("config.toml"));
setup_config_toml(path, profile, config);
}

fn setup_config_toml(path: &PathBuf, profile: Profile, config: &Config) {
if path.exists() {
eprintln!();
eprintln!(
"error: you asked `x.py` to setup a new config file, but one already exists at `{}`",
path.display()
);
eprintln!("help: try adding `profile = \"{}\"` at the top of {}", profile, path.display());
eprintln!(
"note: this will use the configuration in {}",
profile.include_path(&config.src).display()
);
crate::detail_exit(1);
}

let settings = format!(
"# Includes one of the default files in src/bootstrap/defaults\n\
profile = \"{}\"\n\
changelog-seen = {}\n",
profile, VERSION
);
t!(fs::write(path, settings));

let include_path = profile.include_path(&config.src);
println!("`x.py` will now use the configuration at {}", include_path.display());
}

fn rustup_installed() -> bool {
Expand Down Expand Up @@ -303,7 +301,18 @@ pub fn interactive_path() -> io::Result<Profile> {

// install a git hook to automatically run tidy --bless, if they want
fn install_git_hook_maybe(config: &Config) -> io::Result<()> {
let git = t!(config.git().args(&["rev-parse", "--git-common-dir"]).output().map(|output| {
assert!(output.status.success(), "failed to run `git`");
PathBuf::from(t!(String::from_utf8(output.stdout)).trim())
}));
let dst = git.join("hooks").join("pre-push");
if dst.exists() {
// The git hook has already been set up, or the user already has a custom hook.
return Ok(());
}

let mut input = String::new();
println!();
println!(
"Rust's CI will automatically fail if it doesn't pass `tidy`, the internal tool for ensuring code quality.
If you'd like, x.py can install a git hook for you that will automatically run `tidy --bless` before
Expand All @@ -329,12 +338,6 @@ undesirable, simply delete the `pre-push` file from .git/hooks."

if should_install {
let src = config.src.join("src").join("etc").join("pre-push.sh");
let git =
t!(config.git().args(&["rev-parse", "--git-common-dir"]).output().map(|output| {
assert!(output.status.success(), "failed to run `git`");
PathBuf::from(t!(String::from_utf8(output.stdout)).trim())
}));
let dst = git.join("hooks").join("pre-push");
match fs::hard_link(src, &dst) {
Err(e) => eprintln!(
"error: could not create hook {}: do you already have the git hook installed?\n{}",
Expand Down

0 comments on commit 4b6010c

Please sign in to comment.