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

Re-add help_on_error for download-ci-llvm #97519

Merged
merged 1 commit into from
May 30, 2022
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
18 changes: 8 additions & 10 deletions src/bootstrap/bootstrap.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ def support_xz():
except tarfile.CompressionError:
return False

def get(base, url, path, checksums, verbose=False, do_verify=True, help_on_error=None):
def get(base, url, path, checksums, verbose=False, do_verify=True):
with tempfile.NamedTemporaryFile(delete=False) as temp_file:
temp_path = temp_file.name

Expand All @@ -86,7 +86,7 @@ def get(base, url, path, checksums, verbose=False, do_verify=True, help_on_error
print("ignoring already-download file",
path, "due to failed verification")
os.unlink(path)
download(temp_path, "{}/{}".format(base, url), True, verbose, help_on_error=help_on_error)
download(temp_path, "{}/{}".format(base, url), True, verbose)
if do_verify and not verify(temp_path, sha256, verbose):
raise RuntimeError("failed verification")
if verbose:
Expand All @@ -99,17 +99,17 @@ def get(base, url, path, checksums, verbose=False, do_verify=True, help_on_error
os.unlink(temp_path)


def download(path, url, probably_big, verbose, help_on_error=None):
def download(path, url, probably_big, verbose):
for _ in range(0, 4):
try:
_download(path, url, probably_big, verbose, True, help_on_error=help_on_error)
_download(path, url, probably_big, verbose, True)
return
except RuntimeError:
print("\nspurious failure, trying again")
_download(path, url, probably_big, verbose, False, help_on_error=help_on_error)
_download(path, url, probably_big, verbose, False)


def _download(path, url, probably_big, verbose, exception, help_on_error=None):
def _download(path, url, probably_big, verbose, exception):
# Try to use curl (potentially available on win32
# https://devblogs.microsoft.com/commandline/tar-and-curl-come-to-windows/)
# If an error occurs:
Expand All @@ -134,7 +134,7 @@ def _download(path, url, probably_big, verbose, exception, help_on_error=None):
"--retry", "3", "-Sf", "-o", path, url],
verbose=verbose,
exception=True, # Will raise RuntimeError on failure
help_on_error=help_on_error)
)
except (subprocess.CalledProcessError, OSError, RuntimeError):
# see http://serverfault.com/questions/301128/how-to-download
if platform_is_win32:
Expand Down Expand Up @@ -186,7 +186,7 @@ def unpack(tarball, tarball_suffix, dst, verbose=False, match=None):
shutil.rmtree(os.path.join(dst, fname))


def run(args, verbose=False, exception=False, is_bootstrap=False, help_on_error=None, **kwargs):
def run(args, verbose=False, exception=False, is_bootstrap=False, **kwargs):
"""Run a child program in a new process"""
if verbose:
print("running: " + ' '.join(args))
Expand All @@ -197,8 +197,6 @@ def run(args, verbose=False, exception=False, is_bootstrap=False, help_on_error=
code = ret.wait()
if code != 0:
err = "failed to run: " + ' '.join(args)
if help_on_error is not None:
err += "\n" + help_on_error
if verbose or exception:
Copy link
Member

Choose a reason for hiding this comment

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

Please also remove this from the function parameters (for run, and for all functions which call it, recursively)

raise RuntimeError(err)
# For most failures, we definitely do want to print this error, or the user will have no
Expand Down
15 changes: 12 additions & 3 deletions src/bootstrap/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -869,15 +869,21 @@ impl<'a> Builder<'a> {
self.try_run(patchelf.arg(fname));
}

pub(crate) fn download_component(&self, base: &str, url: &str, dest_path: &Path) {
pub(crate) fn download_component(
&self,
base: &str,
url: &str,
dest_path: &Path,
help_on_error: &str,
) {
// Use a temporary file in case we crash while downloading, to avoid a corrupt download in cache/.
let tempfile = self.tempdir().join(dest_path.file_name().unwrap());
// FIXME: support `do_verify` (only really needed for nightly rustfmt)
self.download_with_retries(&tempfile, &format!("{}/{}", base, url));
self.download_with_retries(&tempfile, &format!("{}/{}", base, url), help_on_error);
t!(std::fs::rename(&tempfile, dest_path));
}

fn download_with_retries(&self, tempfile: &Path, url: &str) {
fn download_with_retries(&self, tempfile: &Path, url: &str, help_on_error: &str) {
println!("downloading {}", url);
// Try curl. If that fails and we are on windows, fallback to PowerShell.
let mut curl = Command::new("curl");
Expand Down Expand Up @@ -914,6 +920,9 @@ impl<'a> Builder<'a> {
println!("\nspurious failure, trying again");
}
}
if !help_on_error.is_empty() {
eprintln!("{}", help_on_error);
}
std::process::exit(1);
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/bootstrap/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1491,7 +1491,7 @@ fn download_component(builder: &Builder<'_>, filename: String, prefix: &str, com
let url = format!("rustc-builds/{commit}");
let tarball = rustc_cache.join(&filename);
if !tarball.exists() {
builder.download_component(base, &format!("{url}/{filename}"), &tarball);
builder.download_component(base, &format!("{url}/{filename}"), &tarball, "");
}
let bin_root = builder.out.join(builder.config.build.triple).join("ci-rustc");
builder.unpack(&tarball, &bin_root, prefix)
Expand Down
10 changes: 9 additions & 1 deletion src/bootstrap/native.rs
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,15 @@ fn download_ci_llvm(builder: &Builder<'_>, llvm_sha: &str) {
let filename = format!("rust-dev-nightly-{}.tar.xz", builder.build.build.triple);
let tarball = rustc_cache.join(&filename);
if !tarball.exists() {
builder.download_component(base, &format!("{}/{}", url, filename), &tarball);
let help_on_error = "error: failed to download llvm from ci\n
\nhelp: old builds get deleted after a certain time
\nhelp: if trying to compile an old commit of rustc, disable `download-ci-llvm` in config.toml:
\n
\n[llvm]
\ndownload-ci-llvm = false
\n
";
builder.download_component(base, &format!("{}/{}", url, filename), &tarball, help_on_error);
}
let llvm_root = builder.config.ci_llvm_root();
builder.unpack(&tarball, &llvm_root, "rust-dev");
Expand Down