Skip to content

Commit

Permalink
Make tools which may not build return Option.
Browse files Browse the repository at this point in the history
This makes it mandatory for other steps to have to handle the potential
failure instead of failing in an odd way later down the road.
  • Loading branch information
Mark-Simulacrum committed Oct 19, 2017
1 parent 0fcd3e7 commit 686c101
Show file tree
Hide file tree
Showing 3 changed files with 65 additions and 53 deletions.
74 changes: 40 additions & 34 deletions src/bootstrap/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -340,25 +340,28 @@ impl Step for Miri {
let host = self.host;
let compiler = builder.compiler(1, host);

let miri = builder.ensure(tool::Miri { compiler, target: self.host });
let mut cargo = builder.cargo(compiler, Mode::Tool, host, "test");
cargo.arg("--manifest-path").arg(build.src.join("src/tools/miri/Cargo.toml"));

// Don't build tests dynamically, just a pain to work with
cargo.env("RUSTC_NO_PREFER_DYNAMIC", "1");
// miri tests need to know about the stage sysroot
cargo.env("MIRI_SYSROOT", builder.sysroot(compiler));
cargo.env("RUSTC_TEST_SUITE", builder.rustc(compiler));
cargo.env("RUSTC_LIB_PATH", builder.rustc_libdir(compiler));
cargo.env("MIRI_PATH", miri);

builder.add_rustc_lib_path(compiler, &mut cargo);

try_run_expecting(
build,
&mut cargo,
builder.build.config.toolstate.miri.passes(ToolState::Testing),
);
if let Some(miri) = builder.ensure(tool::Miri { compiler, target: self.host }) {
let mut cargo = builder.cargo(compiler, Mode::Tool, host, "test");
cargo.arg("--manifest-path").arg(build.src.join("src/tools/miri/Cargo.toml"));

// Don't build tests dynamically, just a pain to work with
cargo.env("RUSTC_NO_PREFER_DYNAMIC", "1");
// miri tests need to know about the stage sysroot
cargo.env("MIRI_SYSROOT", builder.sysroot(compiler));
cargo.env("RUSTC_TEST_SUITE", builder.rustc(compiler));
cargo.env("RUSTC_LIB_PATH", builder.rustc_libdir(compiler));
cargo.env("MIRI_PATH", miri);

builder.add_rustc_lib_path(compiler, &mut cargo);

try_run_expecting(
build,
&mut cargo,
builder.build.config.toolstate.miri.passes(ToolState::Testing),
);
} else {
eprintln!("failed to test miri: could not build");
}
}
}

Expand Down Expand Up @@ -391,24 +394,27 @@ impl Step for Clippy {
let host = self.host;
let compiler = builder.compiler(stage, host);

let clippy = builder.ensure(tool::Clippy { compiler, target: self.host });
let mut cargo = builder.cargo(compiler, Mode::Tool, host, "test");
cargo.arg("--manifest-path").arg(build.src.join("src/tools/clippy/Cargo.toml"));
if let Some(clippy) = builder.ensure(tool::Clippy { compiler, target: self.host }) {
let mut cargo = builder.cargo(compiler, Mode::Tool, host, "test");
cargo.arg("--manifest-path").arg(build.src.join("src/tools/clippy/Cargo.toml"));

// Don't build tests dynamically, just a pain to work with
cargo.env("RUSTC_NO_PREFER_DYNAMIC", "1");
// clippy tests need to know about the stage sysroot
cargo.env("SYSROOT", builder.sysroot(compiler));
// clippy tests need to find the driver
cargo.env("CLIPPY_DRIVER_PATH", clippy);
// Don't build tests dynamically, just a pain to work with
cargo.env("RUSTC_NO_PREFER_DYNAMIC", "1");
// clippy tests need to know about the stage sysroot
cargo.env("SYSROOT", builder.sysroot(compiler));
// clippy tests need to find the driver
cargo.env("CLIPPY_DRIVER_PATH", clippy);

builder.add_rustc_lib_path(compiler, &mut cargo);
builder.add_rustc_lib_path(compiler, &mut cargo);

try_run_expecting(
build,
&mut cargo,
builder.build.config.toolstate.clippy.passes(ToolState::Testing),
);
try_run_expecting(
build,
&mut cargo,
builder.build.config.toolstate.clippy.passes(ToolState::Testing),
);
} else {
eprintln!("failed to test clippy: could not build");
}
}
}

Expand Down
4 changes: 3 additions & 1 deletion src/bootstrap/dist.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1073,10 +1073,12 @@ impl Step for Rls {
t!(fs::create_dir_all(&image));

// Prepare the image directory
// We expect RLS to build, because we've exited this step above if tool
// state for RLS isn't testing.
let rls = builder.ensure(tool::Rls {
compiler: builder.compiler(stage, build.build),
target
});
}).expect("Rls to build: toolstate is testing");
install(&rls, &image.join("bin"), 0o755);
let doc = image.join("share/doc/rls");
install(&src.join("README.md"), &doc, 0o644);
Expand Down
40 changes: 22 additions & 18 deletions src/bootstrap/tool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ struct ToolBuild {
}

impl Step for ToolBuild {
type Output = PathBuf;
type Output = Option<PathBuf>;

fn should_run(run: ShouldRun) -> ShouldRun {
run.never()
Expand All @@ -96,7 +96,7 @@ impl Step for ToolBuild {
///
/// This will build the specified tool with the specified `host` compiler in
/// `stage` into the normal cargo output directory.
fn run(self, builder: &Builder) -> PathBuf {
fn run(self, builder: &Builder) -> Option<PathBuf> {
let build = builder.build;
let compiler = self.compiler;
let target = self.target;
Expand All @@ -116,11 +116,15 @@ impl Step for ToolBuild {

let mut cargo = prepare_tool_cargo(builder, compiler, target, "build", path);
build.run_expecting(&mut cargo, expectation);
let cargo_out = build.cargo_out(compiler, Mode::Tool, target)
.join(exe(tool, &compiler.host));
let bin = build.tools_dir(compiler).join(exe(tool, &compiler.host));
copy(&cargo_out, &bin);
bin
if expectation == BuildExpectation::Succeeding || expectation == BuildExpectation::None {
let cargo_out = build.cargo_out(compiler, Mode::Tool, target)
.join(exe(tool, &compiler.host));
let bin = build.tools_dir(compiler).join(exe(tool, &compiler.host));
copy(&cargo_out, &bin);
Some(bin)
} else {
None
}
}
}

Expand Down Expand Up @@ -229,7 +233,7 @@ macro_rules! tool {
mode: $mode,
path: $path,
expectation: BuildExpectation::None,
})
}).expect("expected to build -- BuildExpectation::None")
}
}
)+
Expand Down Expand Up @@ -277,7 +281,7 @@ impl Step for RemoteTestServer {
mode: Mode::Libstd,
path: "src/tools/remote-test-server",
expectation: BuildExpectation::None,
})
}).expect("expected to build -- BuildExpectation::None")
}
}

Expand Down Expand Up @@ -395,7 +399,7 @@ impl Step for Cargo {
mode: Mode::Librustc,
path: "src/tools/cargo",
expectation: BuildExpectation::None,
})
}).expect("BuildExpectation::None - expected to build")
}
}

Expand All @@ -406,7 +410,7 @@ pub struct Clippy {
}

impl Step for Clippy {
type Output = PathBuf;
type Output = Option<PathBuf>;
const DEFAULT: bool = true;
const ONLY_HOSTS: bool = true;

Expand All @@ -421,7 +425,7 @@ impl Step for Clippy {
});
}

fn run(self, builder: &Builder) -> PathBuf {
fn run(self, builder: &Builder) -> Option<PathBuf> {
// Clippy depends on procedural macros (serde), which requires a full host
// compiler to be available, so we need to depend on that.
builder.ensure(compile::Rustc {
Expand All @@ -446,7 +450,7 @@ pub struct Rls {
}

impl Step for Rls {
type Output = PathBuf;
type Output = Option<PathBuf>;
const DEFAULT: bool = true;
const ONLY_HOSTS: bool = true;

Expand All @@ -462,7 +466,7 @@ impl Step for Rls {
});
}

fn run(self, builder: &Builder) -> PathBuf {
fn run(self, builder: &Builder) -> Option<PathBuf> {
builder.ensure(native::Openssl {
target: self.target,
});
Expand Down Expand Up @@ -490,7 +494,7 @@ pub struct Rustfmt {
}

impl Step for Rustfmt {
type Output = PathBuf;
type Output = Option<PathBuf>;
const DEFAULT: bool = true;
const ONLY_HOSTS: bool = true;

Expand All @@ -506,7 +510,7 @@ impl Step for Rustfmt {
});
}

fn run(self, builder: &Builder) -> PathBuf {
fn run(self, builder: &Builder) -> Option<PathBuf> {
builder.ensure(ToolBuild {
compiler: self.compiler,
target: self.target,
Expand All @@ -526,7 +530,7 @@ pub struct Miri {
}

impl Step for Miri {
type Output = PathBuf;
type Output = Option<PathBuf>;
const DEFAULT: bool = true;
const ONLY_HOSTS: bool = true;

Expand All @@ -542,7 +546,7 @@ impl Step for Miri {
});
}

fn run(self, builder: &Builder) -> PathBuf {
fn run(self, builder: &Builder) -> Option<PathBuf> {
builder.ensure(ToolBuild {
compiler: self.compiler,
target: self.target,
Expand Down

0 comments on commit 686c101

Please sign in to comment.