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

Give dynamically generated instructions on how to replicate x.py errors #86022

Closed
wants to merge 7 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
174 changes: 171 additions & 3 deletions src/bootstrap/builder.rs
@@ -1,9 +1,10 @@
use std::any::Any;
use std::borrow::Cow;
use std::cell::{Cell, RefCell};
use std::collections::BTreeSet;
use std::env;
use std::ffi::OsStr;
use std::fmt::Debug;
use std::fmt::{self, Debug, Display};
use std::fs;
use std::hash::Hash;
use std::ops::Deref;
Expand Down Expand Up @@ -50,7 +51,7 @@ impl<'a> Deref for Builder<'a> {
}
}

pub trait Step: 'static + Clone + Debug + PartialEq + Eq + Hash {
pub(crate) trait Step: 'static + Clone + Debug + PartialEq + Eq + Hash {
/// `PathBuf` when directories are created or to return a `Compiler` once
/// it's been assembled.
type Output: Clone;
Expand All @@ -62,6 +63,13 @@ pub trait Step: 'static + Clone + Debug + PartialEq + Eq + Hash {
/// If true, then this rule should be skipped if --target was specified, but --host was not
const ONLY_HOSTS: bool = false;

/// A user-visible name to display if this step fails.
fn name(&self) -> &'static str {
std::any::type_name::<Self>()
}

fn info(_step_info: &mut StepInfo<'_, '_, Self>);

/// Primary function to execute this rule. Can call `builder.ensure()`
/// with other steps to run those.
fn run(self, builder: &Builder<'_>) -> Self::Output;
Expand Down Expand Up @@ -351,6 +359,26 @@ pub enum Kind {
Run,
}

impl Display for Kind {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
use Kind::*;
let s = match self {
Build => "build",
Check => "check",
Clippy => "clippy",
Fix => "fix",
Format => "fmt",
Test => "test",
Bench => "bench",
Dist => "dist",
Doc => "doc",
Install => "install",
Run => "run",
};
f.write_str(s)
}
}

impl<'a> Builder<'a> {
fn get_step_descriptions(kind: Kind) -> Vec<StepDescription> {
macro_rules! describe {
Expand Down Expand Up @@ -610,6 +638,12 @@ impl<'a> Builder<'a> {
StepDescription::run(v, self, paths);
}

pub(crate) fn step_info(&self, step: &impl Step) {
let mut info = StepInfo::new(self, step);
Step::info(&mut info);
info.print();
}

/// Obtain a compiler at a given stage and for a given host. Explicitly does
/// not take `Compiler` since all `Compiler` instances are meant to be
/// obtained through this function, since it ensures that they are valid
Expand Down Expand Up @@ -661,6 +695,11 @@ impl<'a> Builder<'a> {
run.never()
}

fn info(step_info: &mut StepInfo<'_, '_, Self>) {
let step = step_info.step;
step_info.compiler(&step.compiler).target(step.target).cmd(Kind::Check);
}

fn run(self, builder: &Builder<'_>) -> Interned<PathBuf> {
let lib = builder.sysroot_libdir_relative(self.compiler);
let sysroot = builder
Expand Down Expand Up @@ -1532,7 +1571,7 @@ impl<'a> Builder<'a> {
/// Ensure that a given step is built, returning its output. This will
/// cache the step, so it is safe (and good!) to call this as often as
/// needed to ensure that all dependencies are built.
pub fn ensure<S: Step>(&'a self, step: S) -> S::Output {
pub(crate) fn ensure<S: Step>(&'a self, step: S) -> S::Output {
{
let mut stack = self.stack.borrow_mut();
for stack_step in stack.iter() {
Expand Down Expand Up @@ -1563,6 +1602,7 @@ impl<'a> Builder<'a> {
let out = step.clone().run(self);
let dur = start.elapsed();
let deps = self.time_spent_on_dependencies.replace(parent + dur);

(out, dur - deps)
};

Expand Down Expand Up @@ -1692,3 +1732,131 @@ impl From<Cargo> for Command {
cargo.command
}
}

pub(crate) struct StepInfo<'a, 'b, S> {
pub(crate) builder: &'a Builder<'b>,
pub(crate) step: &'a S,
compiler: Option<Cow<'a, Compiler>>,
stage: Option<u32>,
host: Option<TargetSelection>,
target: Option<TargetSelection>,
cmd: Option<Kind>,
path: Option<PathBuf>,
}

impl<'a> From<Compiler> for Cow<'a, Compiler> {
fn from(val: Compiler) -> Self {
Self::Owned(val)
}
}

impl<'a> From<&'a Compiler> for Cow<'a, Compiler> {
fn from(val: &'a Compiler) -> Self {
Self::Borrowed(val)
}
}

impl<'a, 'b, S> StepInfo<'a, 'b, S> {
pub(crate) fn new(builder: &'a Builder<'b>, step: &'a S) -> Self {
Self {
builder,
step,
compiler: None,
stage: None,
host: None,
target: None,
cmd: None,
path: None,
}
}

pub(crate) fn compiler(&mut self, val: impl Into<Cow<'a, Compiler>>) -> &mut Self {
if self.compiler.is_some() {
panic!("cannot overwrite compiler");
}
let val = val.into();
self.stage(val.stage).host(val.host);
self.compiler = Some(val);
self
}

pub(crate) fn stage(&mut self, stage: u32) -> &mut Self {
if self.stage.is_some() {
panic!("cannot overwrite stage");
}
self.stage = Some(stage);
self
}

pub(crate) fn host(&mut self, host: TargetSelection) -> &mut Self {
if self.host.is_some() {
panic!("cannot overwrite host");
}
self.host = Some(host);
self
}

pub(crate) fn target(&mut self, target: TargetSelection) -> &mut Self {
if self.target.is_some() {
panic!("cannot overwrite target");
}
self.target = Some(target);
self
}

pub(crate) fn cmd(&mut self, val: Kind) -> &mut Self {
if self.cmd.is_some() {
panic!("cannot overwrite cmd");
}
self.cmd = Some(val);
self
}

pub(crate) fn path(&mut self, val: PathBuf) -> &mut Self {
if self.path.is_some() {
panic!("cannot overwrite path");
}
self.path = Some(val);
self
}

/// Print a command that will run the current step.
///
/// This serves two purposes:
/// 1. Describe what step is currently being run.
/// 2. Describe how to run only this step in case it fails.
pub(crate) fn print(&self)
where
S: Step,
{
let builder = self.builder;
if builder.config.dry_run {
return;
}
let stage = self.stage.unwrap_or(self.builder.top_stage);
let kind = self.cmd.unwrap_or_else(|| panic!("missing kind for {}", self.step.name()));
let path = self.path.clone().unwrap_or_else(|| {
let paths = S::should_run(ShouldRun::new(builder)).paths;
paths.iter().map(|pathset| pathset.path(builder)).next().expect("no paths for step")
});
print!("{} {} --stage {}", kind, path.display(), stage,);
if let Some(host) = self.host {
// Almost always, this will be the same as build. Don't print it if so.
if host != builder.config.build {
print!(" --host {}", host);
}
}
if let Some(target) = self.target {
let different_from_host = self.host.map_or(false, |h| h != target);
if target != builder.config.build || different_from_host {
print!(" --target {}", target);
}
}
if kind == Kind::Test {
for arg in builder.config.cmd.test_args() {
print!(" --test-args \"{}\"", arg);
}
}
println!();
}
}
3 changes: 2 additions & 1 deletion src/bootstrap/builder/tests.rs
Expand Up @@ -511,7 +511,8 @@ mod dist {
target: host,
mode: Mode::Std,
test_kind: test::TestKind::Test,
krate: INTERNER.intern_str("std"),
krate_name: INTERNER.intern_str("std"),
krate_path: "library/std".into(),
},]
);
}
Expand Down
8 changes: 4 additions & 4 deletions src/bootstrap/cache.rs
Expand Up @@ -245,7 +245,7 @@ impl Cache {
Cache(RefCell::new(HashMap::new()))
}

pub fn put<S: Step>(&self, step: S, value: S::Output) {
pub(crate) fn put<S: Step>(&self, step: S, value: S::Output) {
let mut cache = self.0.borrow_mut();
let type_id = TypeId::of::<S>();
let stepcache = cache
Expand All @@ -257,7 +257,7 @@ impl Cache {
stepcache.insert(step, value);
}

pub fn get<S: Step>(&self, step: &S) -> Option<S::Output> {
pub(crate) fn get<S: Step>(&self, step: &S) -> Option<S::Output> {
let mut cache = self.0.borrow_mut();
let type_id = TypeId::of::<S>();
let stepcache = cache
Expand All @@ -271,15 +271,15 @@ impl Cache {

#[cfg(test)]
impl Cache {
pub fn all<S: Ord + Copy + Step>(&mut self) -> Vec<(S, S::Output)> {
pub fn all<S: Ord + Step>(&mut self) -> Vec<(S, S::Output)> {
let cache = self.0.get_mut();
let type_id = TypeId::of::<S>();
let mut v = cache
.remove(&type_id)
.map(|b| b.downcast::<HashMap<S, S::Output>>().expect("correct type"))
.map(|m| m.into_iter().collect::<Vec<_>>())
.unwrap_or_default();
v.sort_by_key(|&(a, _)| a);
v.sort_by_key(|(a, _)| a.clone());
v
}

Expand Down