Skip to content

Commit

Permalink
Rollup merge of #96584 - bentongxyz:x-setup-h-v-should-work, r=jyn514
Browse files Browse the repository at this point in the history
Fix `x setup -h -v` should work

r? `@jyn514`

I have to convert profile to path and back in order to remove special-casing in bootstrap. I also check for `dry_run` so that `config.toml` and/ or `.git/hooks/pre-push` will not be created if `--dry-run` is specified.

Please help me see if this is ok, thanks alot!
  • Loading branch information
matthiaskrgr authored Dec 18, 2022
2 parents 35a99ee + fca8290 commit 3fb5d8e
Show file tree
Hide file tree
Showing 4 changed files with 74 additions and 21 deletions.
15 changes: 12 additions & 3 deletions src/bootstrap/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ use crate::flags::{Color, Subcommand};
use crate::install;
use crate::native;
use crate::run;
use crate::setup;
use crate::test;
use crate::tool::{self, SourceType};
use crate::util::{self, add_dylib_path, add_link_lib_path, exe, libdir, output, t};
Expand Down Expand Up @@ -433,8 +434,11 @@ impl<'a> ShouldRun<'a> {

// single alias, which does not correspond to any on-disk path
pub fn alias(mut self, alias: &str) -> Self {
// exceptional case for `Kind::Setup` because its `library`
// and `compiler` options would otherwise naively match with
// `compiler` and `library` folders respectively.
assert!(
!self.builder.src.join(alias).exists(),
self.kind == Kind::Setup || !self.builder.src.join(alias).exists(),
"use `builder.path()` for real paths: {}",
alias
);
Expand Down Expand Up @@ -758,8 +762,9 @@ impl<'a> Builder<'a> {
run::CollectLicenseMetadata,
run::GenerateCopyright,
),
Kind::Setup => describe!(setup::Profile),
// These commands either don't use paths, or they're special-cased in Build::build()
Kind::Clean | Kind::Format | Kind::Setup => vec![],
Kind::Clean | Kind::Format => vec![],
}
}

Expand Down Expand Up @@ -822,7 +827,11 @@ impl<'a> Builder<'a> {
Subcommand::Install { ref paths } => (Kind::Install, &paths[..]),
Subcommand::Run { ref paths, .. } => (Kind::Run, &paths[..]),
Subcommand::Format { .. } => (Kind::Format, &[][..]),
Subcommand::Clean { .. } | Subcommand::Setup { .. } => {
Subcommand::Setup { profile: ref path } => (
Kind::Setup,
path.as_ref().map_or([].as_slice(), |path| std::slice::from_ref(path)),
),
Subcommand::Clean { .. } => {
panic!()
}
};
Expand Down
6 changes: 3 additions & 3 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: Option<Profile>,
profile: Option<PathBuf>,
},
}

Expand Down Expand Up @@ -351,7 +351,7 @@ To learn more about a subcommand, run `./x.py <subcommand> -h`",

// fn usage()
let usage = |exit_code: i32, opts: &Options, verbose: bool, subcommand_help: &str| -> ! {
let config = Config::parse(&["build".to_string()]);
let config = Config::parse(&["setup".to_string()]);
let build = Build::new(config);
let paths = Builder::get_help(&build, subcommand);

Expand Down Expand Up @@ -621,7 +621,7 @@ Arguments:
}
Kind::Setup => {
let profile = if paths.len() > 1 {
println!("\nat most one profile can be passed to setup\n");
eprintln!("\nerror: At most one profile can be passed to setup\n");
usage(1, &opts, verbose, &subcommand_help)
} else if let Some(path) = paths.pop() {
let profile_string = t!(path.into_os_string().into_string().map_err(
Expand Down
4 changes: 0 additions & 4 deletions src/bootstrap/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -730,10 +730,6 @@ impl Build {
return clean::clean(self, all);
}

if let Subcommand::Setup { profile } = &self.config.cmd {
return setup::setup(&self.config, *profile);
}

// Download rustfmt early so that it can be used in rust-analyzer configs.
let _ = &builder::Builder::new(&self).initial_rustfmt();

Expand Down
70 changes: 59 additions & 11 deletions src/bootstrap/setup.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use crate::builder::{Builder, RunConfig, ShouldRun, Step};
use crate::Config;
use crate::{t, VERSION};
use std::env::consts::EXE_SUFFIX;
Expand All @@ -9,7 +10,7 @@ use std::process::Command;
use std::str::FromStr;
use std::{fmt, fs, io};

#[derive(Clone, Copy, Debug, Eq, PartialEq)]
#[derive(Clone, Copy, Debug, Eq, PartialEq, Hash)]
pub enum Profile {
Compiler,
Codegen,
Expand Down Expand Up @@ -48,6 +49,16 @@ impl Profile {
}
out
}

pub fn as_str(&self) -> &'static str {
match self {
Profile::Compiler => "compiler",
Profile::Codegen => "codegen",
Profile::Library => "library",
Profile::Tools => "tools",
Profile::User => "user",
}
}
}

impl FromStr for Profile {
Expand All @@ -69,24 +80,58 @@ impl FromStr for Profile {

impl fmt::Display for Profile {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
match self {
Profile::Compiler => write!(f, "compiler"),
Profile::Codegen => write!(f, "codegen"),
Profile::Library => write!(f, "library"),
Profile::User => write!(f, "user"),
Profile::Tools => write!(f, "tools"),
f.write_str(self.as_str())
}
}

impl Step for Profile {
type Output = ();
const DEFAULT: bool = true;

fn should_run(mut run: ShouldRun<'_>) -> ShouldRun<'_> {
for choice in Profile::all() {
run = run.alias(choice.as_str());
}
run
}

fn make_run(run: RunConfig<'_>) {
// for Profile, `run.paths` will have 1 and only 1 element
// this is because we only accept at most 1 path from user input.
// If user calls `x.py setup` without arguments, the interactive TUI
// will guide user to provide one.
let profile = if run.paths.len() > 1 {
// HACK: `builder` runs this step with all paths if no path was passed.
t!(interactive_path())
} else {
run.paths
.first()
.unwrap()
.assert_single_path()
.path
.as_path()
.as_os_str()
.to_str()
.unwrap()
.parse()
.unwrap()
};

run.builder.ensure(profile);
}

fn run(self, builder: &Builder<'_>) {
setup(&builder.build.config, self)
}
}

pub fn setup(config: &Config, profile: Option<Profile>) {
let profile = profile.unwrap_or_else(|| t!(interactive_path()));
pub fn setup(config: &Config, profile: Profile) {
let stage_path =
["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");
} else if stage_dir_exists(&stage_path[..]) {
} else if stage_dir_exists(&stage_path[..]) && !config.dry_run() {
attempt_toolchain_link(&stage_path[..]);
}

Expand All @@ -104,7 +149,9 @@ pub fn setup(config: &Config, profile: Option<Profile>) {
Profile::User => &["dist", "build"],
};

t!(install_git_hook_maybe(&config));
if !config.dry_run() {
t!(install_git_hook_maybe(&config));
}

println!();

Expand Down Expand Up @@ -144,6 +191,7 @@ fn setup_config_toml(path: &PathBuf, profile: Profile, config: &Config) {
changelog-seen = {}\n",
profile, VERSION
);

t!(fs::write(path, settings));

let include_path = profile.include_path(&config.src);
Expand Down

0 comments on commit 3fb5d8e

Please sign in to comment.