Skip to content

Don't download cargo twice when download-rustc is set #84750

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

Jump to bottom
Merged
merged 1 commit into from
May 2, 2021
Merged

Don't download cargo twice when download-rustc is set #84750

merged 1 commit into from
May 2, 2021
+7 −5

Conversation

jyn514
Copy link
Member

@jyn514 jyn514 commented Apr 30, 2021 •
edited
Loading

Previously, this caused a bug on NixOS:

  1. bootstrap.py would download and patch stage0/cargo
  2. bootstrap.py would download nightly cargo, but extract it to
    stage0/cargo instead of ci-rustc/cargo. It would still try (and fail) to patch ci-rustc/cargo.
  3. bootstrap.py would fail to build rustbuild because stage0/cargo
    wasn't patched.

The "proper" fix is to extract nightly cargo to ci-rustc instead, but it
doesn't seem to be necessary at all, so this just skips downloading it
instead.

Fixes #84702

@jyn514
Don't download cargo twice when download-rustc is set
69f3ead 
Previously, this caused a bug on NixOS:

1. bootstrap.py would download and patch stage0/cargo
2. bootstrap.py would download nightly cargo, but extract it to
   stage0/cargo instead of ci-rustc/cargo.
3. bootstrap.py would fail to build rustbuild because stage0/cargo
   wasn't patched.

The "proper" fix is to extract nightly cargo to ci-rustc instead, but it
doesn't seem to be necessary at all, so this just skips downloading it
instead.
@jyn514 jyn514 added T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) O-NixOS Operating system: NixOS, https://nixos.org/ labels Apr 30, 2021
@rust-highfive
Copy link
Contributor

r? @Mark-Simulacrum

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 30, 2021
@jyn514
Copy link
Member Author

jyn514 commented Apr 30, 2021

bootstrap.py would download nightly cargo, but extract it to
stage0/cargo instead of ci-rustc/cargo.

This happened because of a missing parameter to _download_component_helper, it was missing stage0. Hmm, I think rustfmt is missing that parameter too actually? But rustfmt is working fine for me on nix :/

@jyn514
Copy link
Member Author

jyn514 commented Apr 30, 2021

Hmm, I think rustfmt is missing that parameter too actually? But rustfmt is working fine for me on nix :/

This is because of the if rustfmt.exists() call:

rust/src/bootstrap/bootstrap.py

Lines 436 to 439 in 49920bc

if self.rustfmt() and self.rustfmt().startswith(bin_root) and (
not os.path.exists(self.rustfmt())
or self.program_out_of_date(self.rustfmt_stamp(), self.rustfmt_channel)
):

self.rustfmt() is the same regardless of whether stage0 is set, so it just never executes that bit of code when downloading the ci-rustc toolchain, effectively the same behavior as this PR already has.

rust/src/bootstrap/bootstrap.py

Lines 795 to 799 in 49920bc

def rustfmt(self):
"""Return config path for rustfmt"""
if not self.rustfmt_channel:
return None
return self.program_config('rustfmt')

@Mark-Simulacrum
Copy link
Member

@bors r+

@bors
Copy link
Collaborator

bors commented May 1, 2021

📌 Commit 69f3ead has been approved by Mark-Simulacrum

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 1, 2021
@bors
Copy link
Collaborator

bors commented May 2, 2021

⌛ Testing commit 69f3ead with merge 6d4e3c1...

@bors
Copy link
Collaborator

bors commented May 2, 2021

☀️ Test successful - checks-actions
Approved by: Mark-Simulacrum
Pushing 6d4e3c1 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label May 2, 2021
@bors bors merged commit 6d4e3c1 into rust-lang:master May 2, 2021
@rustbot rustbot added this to the 1.54.0 milestone May 2, 2021
@jyn514 jyn514 deleted the nix-cargo branch May 2, 2021 13:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@Mark-Simulacrum Mark-Simulacrum

Labels
merged-by-bors This PR was explicitly merged by bors. O-NixOS Operating system: NixOS, https://nixos.org/ S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
Projects
None yet
Milestone
1.54.0
Development

Successfully merging this pull request may close these issues.

bootstrap tries to patch the wrong cargo on NixOS when download-rustc is set
5 participants
@jyn514 @rust-highfive @Mark-Simulacrum @bors @rustbot