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

Allow building rustdoc without first building rustc (MVP) #79540

Merged
merged 2 commits into from
Feb 9, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 6 additions & 0 deletions config.toml.example
Original file line number Diff line number Diff line change
Expand Up @@ -358,6 +358,12 @@ changelog-seen = 2
#
#debug = false

# Whether to download the stage 1 and 2 compilers from CI.
# This is mostly useful for tools; if you have changes to `compiler/` they will be ignored.
#
# FIXME: currently, this also uses the downloaded compiler for stage0, but that causes unnecessary rebuilds.
#download-rustc = false

# Number of codegen units to use for each compiler invocation. A value of 0
# means "the number of cores on this machine", and 1+ is passed through to the
# compiler.
Expand Down
74 changes: 62 additions & 12 deletions src/bootstrap/bootstrap.py
Original file line number Diff line number Diff line change
Expand Up @@ -378,6 +378,7 @@ def __init__(self):
self.verbose = False
self.git_version = None
self.nix_deps_dir = None
self.rustc_commit = None

def download_stage0(self):
"""Fetch the build system for Rust, written in Rust
Expand All @@ -394,20 +395,27 @@ def download_stage0(self):

if self.rustc().startswith(self.bin_root()) and \
(not os.path.exists(self.rustc()) or
self.program_out_of_date(self.rustc_stamp(), self.date)):
self.program_out_of_date(self.rustc_stamp(), self.date + str(self.rustc_commit))):
if os.path.exists(self.bin_root()):
shutil.rmtree(self.bin_root())
download_rustc = self.rustc_commit is not None
tarball_suffix = '.tar.xz' if support_xz() else '.tar.gz'
filename = "rust-std-{}-{}{}".format(
rustc_channel, self.build, tarball_suffix)
pattern = "rust-std-{}".format(self.build)
self._download_stage0_helper(filename, pattern, tarball_suffix)
self._download_component_helper(filename, pattern, tarball_suffix, download_rustc)
filename = "rustc-{}-{}{}".format(rustc_channel, self.build,
tarball_suffix)
self._download_stage0_helper(filename, "rustc", tarball_suffix)
self._download_component_helper(filename, "rustc", tarball_suffix, download_rustc)
filename = "cargo-{}-{}{}".format(rustc_channel, self.build,
tarball_suffix)
self._download_stage0_helper(filename, "cargo", tarball_suffix)
self._download_component_helper(filename, "cargo", tarball_suffix)
if self.rustc_commit is not None:
filename = "rustc-dev-{}-{}{}".format(rustc_channel, self.build, tarball_suffix)
self._download_component_helper(
filename, "rustc-dev", tarball_suffix, download_rustc
)

self.fix_bin_or_dylib("{}/bin/rustc".format(self.bin_root()))
self.fix_bin_or_dylib("{}/bin/rustdoc".format(self.bin_root()))
self.fix_bin_or_dylib("{}/bin/cargo".format(self.bin_root()))
Expand All @@ -416,7 +424,7 @@ def download_stage0(self):
if lib.endswith(".so"):
self.fix_bin_or_dylib(os.path.join(lib_dir, lib), rpath_libz=True)
with output(self.rustc_stamp()) as rust_stamp:
rust_stamp.write(self.date)
rust_stamp.write(self.date + str(self.rustc_commit))

if self.rustfmt() and self.rustfmt().startswith(self.bin_root()) and (
not os.path.exists(self.rustfmt())
Expand All @@ -426,7 +434,9 @@ def download_stage0(self):
tarball_suffix = '.tar.xz' if support_xz() else '.tar.gz'
[channel, date] = rustfmt_channel.split('-', 1)
filename = "rustfmt-{}-{}{}".format(channel, self.build, tarball_suffix)
self._download_stage0_helper(filename, "rustfmt-preview", tarball_suffix, date)
self._download_component_helper(
filename, "rustfmt-preview", tarball_suffix, key=date
)
self.fix_bin_or_dylib("{}/bin/rustfmt".format(self.bin_root()))
self.fix_bin_or_dylib("{}/bin/cargo-fmt".format(self.bin_root()))
with output(self.rustfmt_stamp()) as rustfmt_stamp:
Expand Down Expand Up @@ -482,18 +492,27 @@ def downloading_llvm(self):
return opt == "true" \
or (opt == "if-available" and self.build in supported_platforms)

def _download_stage0_helper(self, filename, pattern, tarball_suffix, date=None):
if date is None:
date = self.date
def _download_component_helper(
self, filename, pattern, tarball_suffix, download_rustc=False, key=None
):
if key is None:
if download_rustc:
key = self.rustc_commit
else:
key = self.date
cache_dst = os.path.join(self.build_dir, "cache")
rustc_cache = os.path.join(cache_dst, date)
rustc_cache = os.path.join(cache_dst, key)
if not os.path.exists(rustc_cache):
os.makedirs(rustc_cache)

url = "{}/dist/{}".format(self._download_url, date)
if download_rustc:
url = "https://ci-artifacts.rust-lang.org/rustc-builds/{}".format(self.rustc_commit)
else:
url = "{}/dist/{}".format(self._download_url, key)
tarball = os.path.join(rustc_cache, filename)
if not os.path.exists(tarball):
get("{}/{}".format(url, filename), tarball, verbose=self.verbose)
do_verify = not download_rustc
get("{}/{}".format(url, filename), tarball, verbose=self.verbose, do_verify=do_verify)
unpack(tarball, tarball_suffix, self.bin_root(), match=pattern, verbose=self.verbose)

def _download_ci_llvm(self, llvm_sha, llvm_assertions):
Expand Down Expand Up @@ -613,6 +632,30 @@ def fix_bin_or_dylib(self, fname, rpath_libz=False):
print("warning: failed to call patchelf:", reason)
return

# Return the stage1 compiler to download, if any.
def maybe_download_rustc(self):
# If `download-rustc` is not set, default to rebuilding.
if self.get_toml("download-rustc", section="rust") != "true":
return None

# Handle running from a directory other than the top level
rev_parse = ["git", "rev-parse", "--show-toplevel"]
top_level = subprocess.check_output(rev_parse, universal_newlines=True).strip()
compiler = "{}/compiler/".format(top_level)

# Look for a version to compare to based on the current commit.
# Ideally this would just use `merge-base`, but on beta and stable branches that wouldn't
# come up with any commits, so hack it and use `author=bors` instead.
merge_base = ["git", "log", "--author=bors", "--pretty=%H", "-n1", "--", compiler]
Copy link
Member Author

@jyn514 jyn514 Feb 9, 2021

Choose a reason for hiding this comment

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

This already went wrong :/ I just rebased over 9a1d617, which is a bors commit, but git doesn't recognize as modifying any code:

$ git show 9a1d6174c925f54c923599e29b09d6855e6b3a78
commit 9a1d6174c925f54c923599e29b09d6855e6b3a78 (origin/master, origin/HEAD, master)
Merge: 37d067f5d71 11e7897eae7
Author: bors <[email protected]>
Date:   Sat Feb 6 18:03:37 2021 +0000

    Auto merge of #81832 - jonas-schievink:rollup-3nw53p0, r=jonas-schievink
    
    Rollup of 7 pull requests
    
    Successful merges:
    
     - #81402 (tidy: Run tidy style against markdown files.)
     - #81434 (BTree: fix documentation of unstable public members)
     - #81680 (Refactor `PrimitiveTypeTable` for Clippy)
     - #81737 (typeck: Emit structured suggestions for tuple struct syntax)
     - #81738 (Miscellaneous small diagnostics cleanup)
     - #81766 (Enable 'task list' markdown extension)
     - #81812 (Add a test for escaping LLVMisms in inline asm)
    
    Failed merges:
    
    r? `@ghost`
    `@rustbot` modify labels: rollup
$

merge-base correctly says 9a1d617 was most recent change, but log --author=bors shows cfba499 instead, which is wrong, because compiler/ was modified since then (specifically in 7acf9ec, 85fb5cd, f631410, and 96e843c). In this case it didn't hurt anything, but if any of the APIs used by rustdoc had changed, it would have caused a build failure.

I really do think merge-base is more correct here. But I'm ok with fixing it in a follow-up if you like.

Copy link
Member

@Mark-Simulacrum Mark-Simulacrum Feb 9, 2021

Choose a reason for hiding this comment

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

It should be possible to pass -m1 or something to log to treat merge commits as modification; it shouldn't be a fundamental limitation. But maybe I need to read and understand merge base. Future PR, I think.

Copy link
Member Author

Choose a reason for hiding this comment

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

I tried -m and it didn't help. But I agree this can wait for a follow-up :)

Copy link
Member Author

Choose a reason for hiding this comment

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

Another alternative is to run git log --author=bors without adding compiler/ to the end, and then run git log <output of previous command> compiler/ on top of that. But that doesn't seem any simpler than merge-base.

commit = subprocess.check_output(merge_base, universal_newlines=True).strip()

# Warn if there were changes to the compiler since the ancestor commit.
status = subprocess.call(["git", "diff-index", "--quiet", commit, "--", compiler])
if status != 0:
print("warning: `download-rustc` is enabled, but there are changes to compiler/")

return commit
Mark-Simulacrum marked this conversation as resolved.
Show resolved Hide resolved

def rustc_stamp(self):
"""Return the path for .rustc-stamp

Expand Down Expand Up @@ -1090,6 +1133,13 @@ def bootstrap(help_triggered):
build.update_submodules()

# Fetch/build the bootstrap
build.rustc_commit = build.maybe_download_rustc()
if build.rustc_commit is not None:
if build.verbose:
commit = build.rustc_commit
print("using downloaded stage1 artifacts from CI (commit {})".format(commit))
# FIXME: support downloading artifacts from the beta channel
build.rustc_channel = "nightly"
jyn514 marked this conversation as resolved.
Show resolved Hide resolved
build.download_stage0()
sys.stdout.flush()
build.ensure_vendored()
Expand Down
26 changes: 24 additions & 2 deletions src/bootstrap/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,14 @@ pub trait Step: 'static + Clone + Debug + PartialEq + Eq + Hash {
/// `true` here can still be overwritten by `should_run` calling `default_condition`.
const DEFAULT: bool = false;

/// Whether this step should be run even when `download-rustc` is set.
///
/// Most steps are not important when the compiler is downloaded, since they will be included in
/// the pre-compiled sysroot. Steps can set this to `true` to be built anyway.
///
/// When in doubt, set this to `false`.
const ENABLE_DOWNLOAD_RUSTC: bool = false;

/// If true, then this rule should be skipped if --target was specified, but --host was not
const ONLY_HOSTS: bool = false;

Expand Down Expand Up @@ -99,6 +107,7 @@ impl RunConfig<'_> {

struct StepDescription {
default: bool,
enable_download_rustc: bool,
only_hosts: bool,
should_run: fn(ShouldRun<'_>) -> ShouldRun<'_>,
make_run: fn(RunConfig<'_>),
Expand Down Expand Up @@ -153,6 +162,7 @@ impl StepDescription {
fn from<S: Step>() -> StepDescription {
StepDescription {
default: S::DEFAULT,
enable_download_rustc: S::ENABLE_DOWNLOAD_RUSTC,
only_hosts: S::ONLY_HOSTS,
should_run: S::should_run,
make_run: S::make_run,
Expand All @@ -169,6 +179,14 @@ impl StepDescription {
"{:?} not skipped for {:?} -- not in {:?}",
pathset, self.name, builder.config.exclude
);
} else if builder.config.download_rustc && !self.enable_download_rustc {
if !builder.config.dry_run {
eprintln!(
"Not running {} because its artifacts have been downloaded from CI (`download-rustc` is set)",
self.name
);
}
return;
}

// Determine the targets participating in this rule.
Expand Down Expand Up @@ -629,8 +647,12 @@ impl<'a> Builder<'a> {
.join("rustlib")
.join(self.target.triple)
.join("lib");
let _ = fs::remove_dir_all(&sysroot);
t!(fs::create_dir_all(&sysroot));
// Avoid deleting the rustlib/ directory we just copied
// (in `impl Step for Sysroot`).
if !builder.config.download_rustc {
let _ = fs::remove_dir_all(&sysroot);
t!(fs::create_dir_all(&sysroot));
}
INTERNER.intern_path(sysroot)
}
}
Expand Down
4 changes: 4 additions & 0 deletions src/bootstrap/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ fn cargo_subcommand(kind: Kind) -> &'static str {
impl Step for Std {
type Output = ();
const DEFAULT: bool = true;
const ENABLE_DOWNLOAD_RUSTC: bool = true;

fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
run.all_krates("test")
Expand Down Expand Up @@ -155,6 +156,7 @@ impl Step for Rustc {
type Output = ();
const ONLY_HOSTS: bool = true;
const DEFAULT: bool = true;
const ENABLE_DOWNLOAD_RUSTC: bool = true;

fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
run.all_krates("rustc-main")
Expand Down Expand Up @@ -233,6 +235,7 @@ impl Step for CodegenBackend {
type Output = ();
const ONLY_HOSTS: bool = true;
const DEFAULT: bool = true;
const ENABLE_DOWNLOAD_RUSTC: bool = true;

fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
run.paths(&["compiler/rustc_codegen_cranelift", "rustc_codegen_cranelift"])
Expand Down Expand Up @@ -290,6 +293,7 @@ macro_rules! tool_check_step {
type Output = ();
const ONLY_HOSTS: bool = true;
const DEFAULT: bool = true;
const ENABLE_DOWNLOAD_RUSTC: bool = true;

fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
run.path($path)
Expand Down
26 changes: 22 additions & 4 deletions src/bootstrap/compile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,10 @@ impl Step for Std {
const DEFAULT: bool = true;

fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
run.all_krates("test")
// When downloading stage1, the standard library has already been copied to the sysroot, so
// there's no need to rebuild it.
let download_rustc = run.builder.config.download_rustc;
run.all_krates("test").default_condition(!download_rustc)
}

fn make_run(run: RunConfig<'_>) {
Expand Down Expand Up @@ -904,6 +907,18 @@ impl Step for Sysroot {
let _ = fs::remove_dir_all(&sysroot);
t!(fs::create_dir_all(&sysroot));

// If we're downloading a compiler from CI, we can use the same compiler for all stages other than 0.
if builder.config.download_rustc {
assert_eq!(
builder.config.build, compiler.host,
"Cross-compiling is not yet supported with `download-rustc`",
);
// Copy the compiler into the correct sysroot.
let stage0_dir = builder.config.out.join(&*builder.config.build.triple).join("stage0");
builder.cp_r(&stage0_dir, &sysroot);
return INTERNER.intern_path(sysroot);
}

// Symlink the source root into the same location inside the sysroot,
// where `rust-src` component would go (`$sysroot/lib/rustlib/src/rust`),
// so that any tools relying on `rust-src` also work for local builds,
Expand Down Expand Up @@ -975,13 +990,16 @@ impl Step for Assemble {
// produce some other architecture compiler we need to start from
// `build` to get there.
//
// FIXME: Perhaps we should download those libraries?
// It would make builds faster...
//
// FIXME: It may be faster if we build just a stage 1 compiler and then
// use that to bootstrap this compiler forward.
let build_compiler = builder.compiler(target_compiler.stage - 1, builder.config.build);

// If we're downloading a compiler from CI, we can use the same compiler for all stages other than 0.
if builder.config.download_rustc {
builder.ensure(Sysroot { compiler: target_compiler });
return target_compiler;
}

// Build the libraries for this compiler to link to (i.e., the libraries
// it uses at runtime). NOTE: Crates the target compiler compiles don't
// link to these. (FIXME: Is that correct? It seems to be correct most
Expand Down
3 changes: 3 additions & 0 deletions src/bootstrap/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ pub struct Config {
pub cmd: Subcommand,
pub incremental: bool,
pub dry_run: bool,
pub download_rustc: bool,

pub deny_warnings: bool,
pub backtrace_on_ice: bool,
Expand Down Expand Up @@ -503,6 +504,7 @@ struct Rust {
new_symbol_mangling: Option<bool>,
profile_generate: Option<String>,
profile_use: Option<String>,
download_rustc: Option<bool>,
}

/// TOML representation of how each build target is configured.
Expand Down Expand Up @@ -885,6 +887,7 @@ impl Config {
config.rust_codegen_units_std = rust.codegen_units_std.map(threads_from_config);
config.rust_profile_use = flags.rust_profile_use.or(rust.profile_use);
config.rust_profile_generate = flags.rust_profile_generate.or(rust.profile_generate);
config.download_rustc = rust.download_rustc.unwrap_or(false);
} else {
config.rust_profile_use = flags.rust_profile_use;
config.rust_profile_generate = flags.rust_profile_generate;
Expand Down
1 change: 1 addition & 0 deletions src/bootstrap/tool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -477,6 +477,7 @@ pub struct Rustdoc {
impl Step for Rustdoc {
type Output = PathBuf;
const DEFAULT: bool = true;
const ENABLE_DOWNLOAD_RUSTC: bool = true;
const ONLY_HOSTS: bool = true;

fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
Expand Down