Skip to content

Commit

Permalink
Rollup merge of #80514 - pietroalbini:fix-install, r=Mark-Simulacrum
Browse files Browse the repository at this point in the history 
Fix broken ./x.py install

During my tarball refactorings in #79788 I changed the directory layout used by the tarball generation code, and that broke the other parts of rustbuild which hardcoded the paths of those directories. Namely, `./x.py install` relied on the uncompressed copy of the tarball left behind by `fabricate`/`rust-installer`, causing #80494.

While the easy fix for #80494 would've been to just update the hardcoded paths to match the new structure, that fix would leave us in the same situation if we were to change the directory layout again in the future. Instead I refactored the code to return a `GeneratedTarball` struct as the output of all the dist steps, and I put all the paths the rest of rustbuild needs to care about in its fields. That way, future changes to `src/bootstrap/tarball.rs` will not break other stuff.

This PR is best reviewed commit-by-commit.
r? `@Mark-Simulacrum`
`@rustbot` modify labels: beta-nominated beta-accepted T-release
  • Loading branch information
@Dylan-DPC
Dylan-DPC committed Dec 31, 2020
2 parents 7d247c9 + 8e0ab0f commit 5bffc26
Show file tree
Hide file tree
Showing 4 changed files with 110 additions and 136 deletions.
82 changes: 41 additions & 41 deletions src/bootstrap/dist.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ use crate::builder::{Builder, RunConfig, ShouldRun, Step};
use crate::cache::{Interned, INTERNER};
use crate::compile;
use crate::config::TargetSelection;
use crate::tarball::{OverlayKind, Tarball};
use crate::tarball::{GeneratedTarball, OverlayKind, Tarball};
use crate::tool::{self, Tool};
use crate::util::{exe, is_dylib, timeit};
use crate::{Compiler, DependencyType, Mode, LLVM_TOOLS};
Expand Down Expand Up @@ -51,7 +51,7 @@ pub struct Docs {
}

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

fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
Expand All @@ -63,7 +63,7 @@ impl Step for Docs {
}

/// Builds the `rust-docs` installer component.
fn run(self, builder: &Builder<'_>) -> Option<PathBuf> {
fn run(self, builder: &Builder<'_>) -> Option<GeneratedTarball> {
let host = self.host;
if !builder.config.docs {
return None;
Expand All @@ -86,7 +86,7 @@ pub struct RustcDocs {
}

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

fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
Expand All @@ -98,7 +98,7 @@ impl Step for RustcDocs {
}

/// Builds the `rustc-docs` installer component.
fn run(self, builder: &Builder<'_>) -> Option<PathBuf> {
fn run(self, builder: &Builder<'_>) -> Option<GeneratedTarball> {
let host = self.host;
if !builder.config.compiler_docs {
return None;
Expand Down Expand Up @@ -267,7 +267,7 @@ pub struct Mingw {
}

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

fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
Expand All @@ -282,7 +282,7 @@ impl Step for Mingw {
///
/// This contains all the bits and pieces to run the MinGW Windows targets
/// without any extra installed software (e.g., we bundle gcc, libraries, etc).
fn run(self, builder: &Builder<'_>) -> Option<PathBuf> {
fn run(self, builder: &Builder<'_>) -> Option<GeneratedTarball> {
let host = self.host;
if !host.contains("pc-windows-gnu") {
return None;
Expand All @@ -307,7 +307,7 @@ pub struct Rustc {
}

impl Step for Rustc {
type Output = PathBuf;
type Output = GeneratedTarball;
const DEFAULT: bool = true;
const ONLY_HOSTS: bool = true;

Expand All @@ -321,7 +321,7 @@ impl Step for Rustc {
}

/// Creates the `rustc` installer component.
fn run(self, builder: &Builder<'_>) -> PathBuf {
fn run(self, builder: &Builder<'_>) -> GeneratedTarball {
let compiler = self.compiler;
let host = self.compiler.host;

Expand Down Expand Up @@ -555,7 +555,7 @@ pub struct Std {
}

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

fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
Expand All @@ -573,7 +573,7 @@ impl Step for Std {
});
}

fn run(self, builder: &Builder<'_>) -> Option<PathBuf> {
fn run(self, builder: &Builder<'_>) -> Option<GeneratedTarball> {
let compiler = self.compiler;
let target = self.target;

Expand Down Expand Up @@ -601,7 +601,7 @@ pub struct RustcDev {
}

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

Expand All @@ -620,7 +620,7 @@ impl Step for RustcDev {
});
}

fn run(self, builder: &Builder<'_>) -> Option<PathBuf> {
fn run(self, builder: &Builder<'_>) -> Option<GeneratedTarball> {
let compiler = self.compiler;
let target = self.target;
if skip_host_target_lib(builder, compiler) {
Expand Down Expand Up @@ -660,7 +660,7 @@ pub struct Analysis {
}

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

fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
Expand All @@ -683,7 +683,7 @@ impl Step for Analysis {
}

/// Creates a tarball of save-analysis metadata, if available.
fn run(self, builder: &Builder<'_>) -> Option<PathBuf> {
fn run(self, builder: &Builder<'_>) -> Option<GeneratedTarball> {
let compiler = self.compiler;
let target = self.target;
assert!(builder.config.extended);
Expand Down Expand Up @@ -796,7 +796,7 @@ pub struct Src;

impl Step for Src {
/// The output path of the src installer tarball
type Output = PathBuf;
type Output = GeneratedTarball;
const DEFAULT: bool = true;
const ONLY_HOSTS: bool = true;

Expand All @@ -809,7 +809,7 @@ impl Step for Src {
}

/// Creates the `rust-src` installer component
fn run(self, builder: &Builder<'_>) -> PathBuf {
fn run(self, builder: &Builder<'_>) -> GeneratedTarball {
let tarball = Tarball::new_targetless(builder, "rust-src");

// A lot of tools expect the rust-src component to be entirely in this directory, so if you
Expand Down Expand Up @@ -848,7 +848,7 @@ pub struct PlainSourceTarball;

impl Step for PlainSourceTarball {
/// Produces the location of the tarball generated
type Output = PathBuf;
type Output = GeneratedTarball;
const DEFAULT: bool = true;
const ONLY_HOSTS: bool = true;

Expand All @@ -862,7 +862,7 @@ impl Step for PlainSourceTarball {
}

/// Creates the plain source tarball
fn run(self, builder: &Builder<'_>) -> PathBuf {
fn run(self, builder: &Builder<'_>) -> GeneratedTarball {
let tarball = Tarball::new(builder, "rustc", "src");
let plain_dst_src = tarball.image_dir();

Expand Down Expand Up @@ -941,7 +941,7 @@ pub struct Cargo {
}

impl Step for Cargo {
type Output = PathBuf;
type Output = GeneratedTarball;
const ONLY_HOSTS: bool = true;

fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
Expand All @@ -959,7 +959,7 @@ impl Step for Cargo {
});
}

fn run(self, builder: &Builder<'_>) -> PathBuf {
fn run(self, builder: &Builder<'_>) -> GeneratedTarball {
let compiler = self.compiler;
let target = self.target;

Expand Down Expand Up @@ -995,7 +995,7 @@ pub struct Rls {
}

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

fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
Expand All @@ -1013,7 +1013,7 @@ impl Step for Rls {
});
}

fn run(self, builder: &Builder<'_>) -> Option<PathBuf> {
fn run(self, builder: &Builder<'_>) -> Option<GeneratedTarball> {
let compiler = self.compiler;
let target = self.target;
assert!(builder.config.extended);
Expand Down Expand Up @@ -1041,7 +1041,7 @@ pub struct RustAnalyzer {
}

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

fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
Expand All @@ -1059,7 +1059,7 @@ impl Step for RustAnalyzer {
});
}

fn run(self, builder: &Builder<'_>) -> Option<PathBuf> {
fn run(self, builder: &Builder<'_>) -> Option<GeneratedTarball> {
let compiler = self.compiler;
let target = self.target;
assert!(builder.config.extended);
Expand Down Expand Up @@ -1090,7 +1090,7 @@ pub struct Clippy {
}

impl Step for Clippy {
type Output = PathBuf;
type Output = GeneratedTarball;
const ONLY_HOSTS: bool = true;

fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
Expand All @@ -1108,7 +1108,7 @@ impl Step for Clippy {
});
}

fn run(self, builder: &Builder<'_>) -> PathBuf {
fn run(self, builder: &Builder<'_>) -> GeneratedTarball {
let compiler = self.compiler;
let target = self.target;
assert!(builder.config.extended);
Expand Down Expand Up @@ -1140,7 +1140,7 @@ pub struct Miri {
}

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

fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
Expand All @@ -1158,7 +1158,7 @@ impl Step for Miri {
});
}

fn run(self, builder: &Builder<'_>) -> Option<PathBuf> {
fn run(self, builder: &Builder<'_>) -> Option<GeneratedTarball> {
let compiler = self.compiler;
let target = self.target;
assert!(builder.config.extended);
Expand Down Expand Up @@ -1193,7 +1193,7 @@ pub struct Rustfmt {
}

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

fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
Expand All @@ -1211,7 +1211,7 @@ impl Step for Rustfmt {
});
}

fn run(self, builder: &Builder<'_>) -> Option<PathBuf> {
fn run(self, builder: &Builder<'_>) -> Option<GeneratedTarball> {
let compiler = self.compiler;
let target = self.target;

Expand Down Expand Up @@ -1316,11 +1316,11 @@ impl Step for Extended {
tarballs.push(mingw_installer.unwrap());
}

let mut tarball = Tarball::new(builder, "rust", &target.triple);
let work = tarball.persist_work_dir();
tarball.combine(&tarballs);
let tarball = Tarball::new(builder, "rust", &target.triple);
let generated = tarball.combine(&tarballs);

let tmp = tmpdir(builder).join("combined-tarball");
let work = generated.work_dir();

let mut license = String::new();
license += &builder.read(&builder.src.join("COPYRIGHT"));
Expand Down Expand Up @@ -1870,7 +1870,7 @@ pub struct LlvmTools {
}

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

fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
Expand All @@ -1881,7 +1881,7 @@ impl Step for LlvmTools {
run.builder.ensure(LlvmTools { target: run.target });
}

fn run(self, builder: &Builder<'_>) -> Option<PathBuf> {
fn run(self, builder: &Builder<'_>) -> Option<GeneratedTarball> {
let target = self.target;
assert!(builder.config.extended);

Expand Down Expand Up @@ -1924,7 +1924,7 @@ pub struct RustDev {
}

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

Expand All @@ -1936,7 +1936,7 @@ impl Step for RustDev {
run.builder.ensure(RustDev { target: run.target });
}

fn run(self, builder: &Builder<'_>) -> Option<PathBuf> {
fn run(self, builder: &Builder<'_>) -> Option<GeneratedTarball> {
let target = self.target;

/* run only if llvm-config isn't used */
Expand Down Expand Up @@ -1989,7 +1989,7 @@ pub struct BuildManifest {
}

impl Step for BuildManifest {
type Output = PathBuf;
type Output = GeneratedTarball;
const DEFAULT: bool = false;
const ONLY_HOSTS: bool = true;

Expand All @@ -2001,7 +2001,7 @@ impl Step for BuildManifest {
run.builder.ensure(BuildManifest { target: run.target });
}

fn run(self, builder: &Builder<'_>) -> PathBuf {
fn run(self, builder: &Builder<'_>) -> GeneratedTarball {
let build_manifest = builder.tool_exe(Tool::BuildManifest);

let tarball = Tarball::new(builder, "build-manifest", &self.target.triple);
Expand All @@ -2021,7 +2021,7 @@ pub struct ReproducibleArtifacts {
}

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

Expand Down
Loading

0 comments on commit 5bffc26

Please sign in to comment.