Skip to content

Commit

Permalink
Rollup merge of #77400 - alarsyo:xpy-setup-suggestions, r=jyn514
Browse files Browse the repository at this point in the history
Fix suggestions for x.py setup

#76631 introduced a new `setup` command to x.py

By default the command prompts for a profile to use:

```
Welcome to the Rust project! What do you want to do with x.py?
a) Contribute to the standard library
b) Contribute to the compiler
c) Contribute to the compiler, and also modify LLVM or codegen
d) Install Rust from source
```

and then displays command suggestions, depending on which profile was chosen. However [the mapping between chosen profile](https://github.com/rust-lang/rust/blob/9cba260df0f1c67ea3690035cd5611a7465a1560/src/bootstrap/setup.rs#L75-L85) and [suggestion](https://github.com/rust-lang/rust/blob/9cba260df0f1c67ea3690035cd5611a7465a1560/src/bootstrap/setup.rs#L42-L47) isn't exact, leading to suggestions not being shown if the user presses `c` or `d`. (because "c" is translated to "llvm" and "d" to "maintainer", but suggestions trigger for "codegen" and "user" respectively)

A more thorough refactor would stop using "strings-as-type" to make sure this kind of error doesn't happen, but it may be overkill for that kind of "script" program?

Tagging the setup command author: @jyn514
  • Loading branch information
Dylan-DPC authored Oct 6, 2020
2 parents 5ae45ea + d67a7e6 commit 59707c5
Show file tree
Hide file tree
Showing 3 changed files with 81 additions and 38 deletions.
23 changes: 16 additions & 7 deletions src/bootstrap/flags.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ use getopts::Options;

use crate::builder::Builder;
use crate::config::{Config, TargetSelection};
use crate::setup::Profile;
use crate::{Build, DocTests};

/// Deserialized version of all flags for this compile.
Expand Down Expand Up @@ -94,7 +95,7 @@ pub enum Subcommand {
paths: Vec<PathBuf>,
},
Setup {
path: String,
profile: Profile,
},
}

Expand Down Expand Up @@ -533,18 +534,26 @@ Arguments:
Subcommand::Run { paths }
}
"setup" => {
let path = if paths.len() > 1 {
let profile = if paths.len() > 1 {
println!("\nat most one profile can be passed to setup\n");
usage(1, &opts, verbose, &subcommand_help)
} else if let Some(path) = paths.pop() {
t!(path.into_os_string().into_string().map_err(|path| format!(
"{} is not a valid UTF8 string",
path.to_string_lossy()
)))
let profile_string = t!(path.into_os_string().into_string().map_err(
|path| format!("{} is not a valid UTF8 string", path.to_string_lossy())
));

profile_string.parse().unwrap_or_else(|err| {
eprintln!("error: {}", err);
eprintln!("help: the available profiles are:");
for choice in Profile::all() {
eprintln!("- {}", choice);
}
std::process::exit(1);
})
} else {
t!(crate::setup::interactive_path())
};
Subcommand::Setup { path }
Subcommand::Setup { profile }
}
_ => {
usage(1, &opts, verbose, &subcommand_help);
Expand Down
4 changes: 2 additions & 2 deletions src/bootstrap/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -471,8 +471,8 @@ impl Build {
return clean::clean(self, all);
}

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

{
Expand Down
92 changes: 63 additions & 29 deletions src/bootstrap/setup.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,55 @@
use crate::t;
use std::path::{Path, PathBuf};
use std::str::FromStr;
use std::{
env, fs,
env, fmt, fs,
io::{self, Write},
};

pub fn setup(src_path: &Path, include_name: &str) {
#[derive(Clone, Copy, Eq, PartialEq)]
pub enum Profile {
Compiler,
Codegen,
Library,
User,
}

impl Profile {
fn include_path(&self, src_path: &Path) -> PathBuf {
PathBuf::from(format!("{}/src/bootstrap/defaults/config.{}.toml", src_path.display(), self))
}

pub fn all() -> impl Iterator<Item = Self> {
[Profile::Compiler, Profile::Codegen, Profile::Library, Profile::User].iter().copied()
}
}

impl FromStr for Profile {
type Err = String;

fn from_str(s: &str) -> Result<Self, Self::Err> {
match s {
"a" | "lib" | "library" => Ok(Profile::Library),
"b" | "compiler" => Ok(Profile::Compiler),
"c" | "llvm" | "codegen" => Ok(Profile::Codegen),
"d" | "maintainer" | "user" => Ok(Profile::User),
_ => Err(format!("unknown profile: '{}'", s)),
}
}
}

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"),
}
}
}

pub fn setup(src_path: &Path, profile: Profile) {
let cfg_file = env::var_os("BOOTSTRAP_CONFIG").map(PathBuf::from);

if cfg_file.as_ref().map_or(false, |f| f.exists()) {
Expand All @@ -14,15 +58,10 @@ pub fn setup(src_path: &Path, include_name: &str) {
"error: you asked `x.py` to setup a new config file, but one already exists at `{}`",
file.display()
);
println!("help: try adding `profile = \"{}\"` at the top of {}", profile, file.display());
println!(
"help: try adding `profile = \"{}\"` at the top of {}",
include_name,
file.display()
);
println!(
"note: this will use the configuration in {}/src/bootstrap/defaults/config.{}.toml",
src_path.display(),
include_name
"note: this will use the configuration in {}",
profile.include_path(src_path).display()
);
std::process::exit(1);
}
Expand All @@ -31,19 +70,17 @@ pub fn setup(src_path: &Path, include_name: &str) {
let settings = format!(
"# Includes one of the default files in src/bootstrap/defaults\n\
profile = \"{}\"\n",
include_name
profile
);
t!(fs::write(path, settings));

let include_path =
format!("{}/src/bootstrap/defaults/config.{}.toml", src_path.display(), include_name);
println!("`x.py` will now use the configuration at {}", include_path);
let include_path = profile.include_path(src_path);
println!("`x.py` will now use the configuration at {}", include_path.display());

let suggestions = match include_name {
"codegen" | "compiler" => &["check", "build", "test"][..],
"library" => &["check", "build", "test library/std", "doc"],
"user" => &["dist", "build"],
_ => return,
let suggestions = match profile {
Profile::Codegen | Profile::Compiler => &["check", "build", "test"][..],
Profile::Library => &["check", "build", "test library/std", "doc"],
Profile::User => &["dist", "build"],
};

println!();
Expand All @@ -57,15 +94,15 @@ pub fn setup(src_path: &Path, include_name: &str) {
println!("- `x.py {}`", cmd);
}

if include_name != "user" {
if profile != Profile::User {
println!(
"For more suggestions, see https://rustc-dev-guide.rust-lang.org/building/suggested.html"
);
}
}

// Used to get the path for `Subcommand::Setup`
pub fn interactive_path() -> io::Result<String> {
pub fn interactive_path() -> io::Result<Profile> {
let mut input = String::new();
println!(
"Welcome to the Rust project! What do you want to do with x.py?
Expand All @@ -78,19 +115,16 @@ d) Install Rust from source"
print!("Please choose one (a/b/c/d): ");
io::stdout().flush()?;
io::stdin().read_line(&mut input)?;
break match input.trim().to_lowercase().as_str() {
"a" | "lib" | "library" => "library",
"b" | "compiler" => "compiler",
"c" | "llvm" => "llvm",
"d" | "user" | "maintainer" => "maintainer",
_ => {
println!("error: unrecognized option '{}'", input.trim());
break match input.trim().to_lowercase().parse() {
Ok(profile) => profile,
Err(err) => {
println!("error: {}", err);
println!("note: press Ctrl+C to exit");
continue;
}
};
};
Ok(template.to_owned())
Ok(template)
}

// install a git hook to automatically run tidy --bless, if they want
Expand Down

0 comments on commit 59707c5

Please sign in to comment.