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.

Sign up for GitHub

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

rustbuild: avoid trying to inversely cross-compile for build triple from host triples #76415

Merged
merged 3 commits into from
Sep 11, 2020
Merged

rustbuild: avoid trying to inversely cross-compile for build triple from host triples #76415

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3193d52
Remove host parameter from step configurations
Mark-Simulacrum Sep 6, 2020
b4eb099
Verify we compile std without involving a b host compiler
Mark-Simulacrum Sep 6, 2020
78125ec
Stop implicitly appending triples to config.toml hosts and targets
Mark-Simulacrum Sep 6, 2020
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
15 changes: 7 additions & 8 deletions config.toml.example
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,19 +120,18 @@
# Defaults to host platform
#build = "x86_64-unknown-linux-gnu"

# In addition to the build triple, other triples to produce full compiler
# toolchains for. Each of these triples will be bootstrapped from the build
# triple and then will continue to bootstrap themselves. This platform must
# currently be able to run all of the triples provided here.
# Which triples to produce a compiler toolchain for. Each of these triples will
# be bootstrapped from the build triple themselves.
#
# Defaults to just the build triple
#host = ["x86_64-unknown-linux-gnu"]

# In addition to all host triples, other triples to produce the standard library
# for. Each host triple will be used to produce a copy of the standard library
# for each target triple.
# Which triples to build libraries (core/alloc/std/test/proc_macro) for. Each of
# these triples will be bootstrapped from the build triple themselves.
#
# Defaults to just the build triple
# Defaults to `host`. If you set this explicitly, you likely want to add all
# host triples to this list as well in order for those host toolchains to be
# able to compile programs for their native target.
#target = ["x86_64-unknown-linux-gnu"]

# Use this directory to store build artifacts.
Expand Down
21 changes: 9 additions & 12 deletions src/bootstrap/builder.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,11 +87,16 @@ pub trait Step: 'static + Clone + Debug + PartialEq + Eq + Hash {

pub struct RunConfig<'a> {
pub builder: &'a Builder<'a>,
pub host: TargetSelection,
pub target: TargetSelection,
pub path: PathBuf,
}

impl RunConfig<'_> {
pub fn build_triple(&self) -> TargetSelection {
self.builder.build.build
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

build, build, build, 🔧 🏗️

}
}

struct StepDescription {
default: bool,
only_hosts: bool,
Expand Down Expand Up @@ -165,7 +170,6 @@ impl StepDescription {
pathset, self.name, builder.config.exclude
);
}
let hosts = &builder.hosts;

// Determine the targets participating in this rule.
let targets = if self.only_hosts {
Expand All @@ -178,16 +182,9 @@ impl StepDescription {
&builder.targets
};

for host in hosts {
for target in targets {
let run = RunConfig {
builder,
path: pathset.path(builder),
host: *host,
target: *target,
};
(self.make_run)(run);
}
for target in targets {
let run = RunConfig { builder, path: pathset.path(builder), target: *target };
(self.make_run)(run);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIRC this previous logic matches the ancient logic of the makefiles, but I don't think it ever fully made sense. The main thing to worry about here is the impact on all the builders on CI, since some of them may accidentally be relying on the previous behavior. Everything is so specific nowadays though it may be unlikely. In any case, just something to watch out for.

}
}

Expand Down
20 changes: 10 additions & 10 deletions src/bootstrap/builder/tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,16 @@ mod dist {
dist::Std { compiler: Compiler { host: a, stage: 1 }, target: b },
]
);
assert_eq!(
first(builder.cache.all::<compile::Std>()),
&[
compile::Std { compiler: Compiler { host: a, stage: 0 }, target: a },
compile::Std { compiler: Compiler { host: a, stage: 1 }, target: a },
compile::Std { compiler: Compiler { host: a, stage: 2 }, target: a },
compile::Std { compiler: Compiler { host: a, stage: 1 }, target: b },
compile::Std { compiler: Compiler { host: a, stage: 2 }, target: b },
],
);
assert_eq!(first(builder.cache.all::<dist::Src>()), &[dist::Src]);
}

Expand Down Expand Up @@ -384,12 +394,9 @@ mod dist {
compile::Std { compiler: Compiler { host: a, stage: 0 }, target: a },
compile::Std { compiler: Compiler { host: a, stage: 1 }, target: a },
compile::Std { compiler: Compiler { host: a, stage: 2 }, target: a },
compile::Std { compiler: Compiler { host: b, stage: 2 }, target: a },
compile::Std { compiler: Compiler { host: a, stage: 1 }, target: b },
compile::Std { compiler: Compiler { host: a, stage: 2 }, target: b },
compile::Std { compiler: Compiler { host: b, stage: 2 }, target: b },
compile::Std { compiler: Compiler { host: a, stage: 2 }, target: c },
compile::Std { compiler: Compiler { host: b, stage: 2 }, target: c },
]
);
assert!(!builder.cache.all::<compile::Assemble>().is_empty());
Expand All @@ -399,10 +406,8 @@ mod dist {
compile::Rustc { compiler: Compiler { host: a, stage: 0 }, target: a },
compile::Rustc { compiler: Compiler { host: a, stage: 1 }, target: a },
compile::Rustc { compiler: Compiler { host: a, stage: 2 }, target: a },
compile::Rustc { compiler: Compiler { host: b, stage: 2 }, target: a },
compile::Rustc { compiler: Compiler { host: a, stage: 1 }, target: b },
compile::Rustc { compiler: Compiler { host: a, stage: 2 }, target: b },
compile::Rustc { compiler: Compiler { host: b, stage: 2 }, target: b },
]
);
}
Expand All @@ -425,12 +430,9 @@ mod dist {
compile::Std { compiler: Compiler { host: a, stage: 0 }, target: a },
compile::Std { compiler: Compiler { host: a, stage: 1 }, target: a },
compile::Std { compiler: Compiler { host: a, stage: 2 }, target: a },
compile::Std { compiler: Compiler { host: b, stage: 2 }, target: a },
compile::Std { compiler: Compiler { host: a, stage: 1 }, target: b },
compile::Std { compiler: Compiler { host: a, stage: 2 }, target: b },
compile::Std { compiler: Compiler { host: b, stage: 2 }, target: b },
compile::Std { compiler: Compiler { host: a, stage: 2 }, target: c },
compile::Std { compiler: Compiler { host: b, stage: 2 }, target: c },
]
);
assert_eq!(
Expand All @@ -439,15 +441,13 @@ mod dist {
compile::Assemble { target_compiler: Compiler { host: a, stage: 0 } },
compile::Assemble { target_compiler: Compiler { host: a, stage: 1 } },
compile::Assemble { target_compiler: Compiler { host: a, stage: 2 } },
compile::Assemble { target_compiler: Compiler { host: b, stage: 2 } },
]
);
assert_eq!(
first(builder.cache.all::<compile::Rustc>()),
&[
compile::Rustc { compiler: Compiler { host: a, stage: 0 }, target: a },
compile::Rustc { compiler: Compiler { host: a, stage: 1 }, target: a },
compile::Rustc { compiler: Compiler { host: a, stage: 1 }, target: b },
]
);
}
Expand Down
6 changes: 3 additions & 3 deletions src/bootstrap/compile.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ impl Step for Std {

fn make_run(run: RunConfig<'_>) {
run.builder.ensure(Std {
compiler: run.builder.compiler(run.builder.top_stage, run.host),
compiler: run.builder.compiler(run.builder.top_stage, run.build_triple()),
target: run.target,
});
}
Expand Down Expand Up @@ -385,7 +385,7 @@ impl Step for StartupObjects {

fn make_run(run: RunConfig<'_>) {
run.builder.ensure(StartupObjects {
compiler: run.builder.compiler(run.builder.top_stage, run.host),
compiler: run.builder.compiler(run.builder.top_stage, run.build_triple()),
target: run.target,
});
}
Expand Down Expand Up @@ -454,7 +454,7 @@ impl Step for Rustc {

fn make_run(run: RunConfig<'_>) {
run.builder.ensure(Rustc {
compiler: run.builder.compiler(run.builder.top_stage, run.host),
compiler: run.builder.compiler(run.builder.top_stage, run.build_triple()),
target: run.target,
});
}
Expand Down
52 changes: 24 additions & 28 deletions src/bootstrap/config.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -273,10 +273,8 @@ struct TomlConfig {
#[serde(deny_unknown_fields, rename_all = "kebab-case")]
struct Build {
build: Option<String>,
#[serde(default)]
host: Vec<String>,
#[serde(default)]
target: Vec<String>,
host: Option<Vec<String>>,
target: Option<Vec<String>>,
// This is ignored, the rust code always gets the build directory from the `BUILD_DIR` env variable
build_dir: Option<String>,
cargo: Option<String>,
Expand Down Expand Up @@ -505,11 +503,6 @@ impl Config {
config.out = dir;
}

// If --target was specified but --host wasn't specified, don't run any host-only tests.
let has_hosts = !flags.host.is_empty();
let has_targets = !flags.target.is_empty();
config.skip_only_host_steps = !has_hosts && has_targets;

let toml = file
.map(|file| {
let contents = t!(fs::read_to_string(&file));
Expand All @@ -528,25 +521,28 @@ impl Config {
.unwrap_or_else(TomlConfig::default);

let build = toml.build.clone().unwrap_or_default();
// set by bootstrap.py
config.hosts.push(config.build);
for host in build.host.iter().map(|h| TargetSelection::from_user(h)) {
if !config.hosts.contains(&host) {
config.hosts.push(host);
}
}
for target in config
.hosts
.iter()
.copied()
.chain(build.target.iter().map(|h| TargetSelection::from_user(h)))
{
if !config.targets.contains(&target) {
config.targets.push(target);
}
}
config.hosts = if !flags.host.is_empty() { flags.host } else { config.hosts };
config.targets = if !flags.target.is_empty() { flags.target } else { config.targets };

// If --target was specified but --host wasn't specified, don't run any host-only tests.
let has_hosts = build.host.is_some() || flags.host.is_some();
let has_targets = build.target.is_some() || flags.target.is_some();
config.skip_only_host_steps = !has_hosts && has_targets;

config.hosts = if let Some(arg_host) = flags.host.clone() {
arg_host
} else if let Some(file_host) = build.host {
file_host.iter().map(|h| TargetSelection::from_user(h)).collect()
} else {
vec![config.build]
};
config.targets = if let Some(arg_target) = flags.target.clone() {
arg_target
} else if let Some(file_target) = build.target {
file_target.iter().map(|h| TargetSelection::from_user(h)).collect()
} else {
// If target is *not* configured, then default to the host
// toolchains.
config.hosts.clone()
};

config.nodejs = build.nodejs.map(PathBuf::from);
config.gdb = build.gdb.map(PathBuf::from);
Expand Down
4 changes: 3 additions & 1 deletion src/bootstrap/dist.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -605,7 +605,9 @@ impl Step for DebuggerScripts {

fn make_run(run: RunConfig<'_>) {
run.builder.ensure(DebuggerScripts {
sysroot: run.builder.sysroot(run.builder.compiler(run.builder.top_stage, run.host)),
sysroot: run
.builder
.sysroot(run.builder.compiler(run.builder.top_stage, run.build_triple())),
host: run.target,
});
}
Expand Down
32 changes: 22 additions & 10 deletions src/bootstrap/flags.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@ pub struct Flags {
pub stage: Option<u32>,
pub keep_stage: Vec<u32>,

pub host: Vec<TargetSelection>,
pub target: Vec<TargetSelection>,
pub host: Option<Vec<TargetSelection>>,
pub target: Option<Vec<TargetSelection>>,
pub config: Option<PathBuf>,
pub jobs: Option<u32>,
pub cmd: Subcommand,
Expand Down Expand Up @@ -526,14 +526,26 @@ Arguments:
.into_iter()
.map(|j| j.parse().expect("`keep-stage` should be a number"))
.collect(),
host: split(&matches.opt_strs("host"))
.into_iter()
.map(|x| TargetSelection::from_user(&x))
.collect::<Vec<_>>(),
target: split(&matches.opt_strs("target"))
.into_iter()
.map(|x| TargetSelection::from_user(&x))
.collect::<Vec<_>>(),
host: if matches.opt_present("host") {
Some(
split(&matches.opt_strs("host"))
.into_iter()
.map(|x| TargetSelection::from_user(&x))
.collect::<Vec<_>>(),
)
} else {
None
},
target: if matches.opt_present("target") {
Some(
split(&matches.opt_strs("target"))
.into_iter()
.map(|x| TargetSelection::from_user(&x))
.collect::<Vec<_>>(),
)
} else {
None
},
config: cfg_file,
jobs: matches.opt_str("jobs").map(|j| j.parse().expect("`jobs` should be a number")),
cmd,
Expand Down
Loading