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 configuring where artifacts are downloaded from #97828

Merged
merged 7 commits into from
Jun 18, 2022

Conversation

pietroalbini
Copy link
Member

Bootstrap has support for downloading prebuilt LLVM and rustc artifacts to speed up local builds, but that currently works only for users working on rust-lang/rust. Forks of the repository (for example Ferrocene) might have different URLs to download artifacts from, or might use a different email address on merge commits, breaking both LLVM and rustc artifact downloads.

This PR refactors bootstrap to load the download URLs and other constants from src/stage0.json, allowing downstream forks to tweak those values. It also future-proofs the download code to easily allow forks to add their own custom protocols (like s3://).

This PR is best reviewed commit-by-commit.

@rust-highfive
Copy link
Collaborator

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 Jun 7, 2022
@rust-log-analyzer

This comment has been minimized.

Copy link
Member

@jyn514 jyn514 left a comment

Choose a reason for hiding this comment

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

A lot of this code is being changed significantly in my rustfmt PR (#97507) - I'd prefer to wait until that's merged.

src/bootstrap/config.rs Outdated Show resolved Hide resolved
@bors
Copy link
Contributor

bors commented Jun 8, 2022

☔ The latest upstream changes (presumably #97887) made this pull request unmergeable. Please resolve the merge conflicts.

pietroalbini and others added 5 commits June 9, 2022 17:54
This commit allows users to change the contents of the "config" key in
src/stage0.json without having it overridden the next time the
bump-stage0 tool is executed.
Co-authored-by: bjorn3 <bjorn3@users.noreply.github.com>
@pietroalbini pietroalbini force-pushed the pa-config-artifacts branch 2 times, most recently from 1c571fb to 754af72 Compare June 9, 2022 17:45
@pietroalbini
Copy link
Member Author

Rebased on top of master!

r? @jyn514

Copy link
Member

@jyn514 jyn514 left a comment

Choose a reason for hiding this comment

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

Changes look good to me - I assume you've already tested this PR with your fork and made sure you can download the artifacts by changing only stage0.json? Testing that there aren't merge conflicts seems like overkill, but you'll have to edit stage0.json after this is merged anyway.

src/stage0.json Show resolved Hide resolved
@pietroalbini
Copy link
Member Author

I assume you've already tested this PR with your fork and made sure you can download the artifacts by changing only stage0.json?

I can double-check on Monday (tested an old iteration of this PR).

@pietroalbini
Copy link
Member Author

Just confirmed that yes, this works! Also pushed a small commit to move Stage0 closer to Config (it's easier to see where it is, and it avoids a merge conflict on the Ferrocene side).

@jyn514
Copy link
Member

jyn514 commented Jun 17, 2022

Missed that this was waiting on me, sorry - this lgtm :)

@bors r+

@bors
Copy link
Contributor

bors commented Jun 17, 2022

📌 Commit d3b1532 has been approved by jyn514

@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 Jun 17, 2022
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 18, 2022
Rollup of 5 pull requests

Successful merges:

 - rust-lang#97803 (Impl Termination for Infallible and then make the Result impls of Termination more generic)
 - rust-lang#97828 (Allow configuring where artifacts are downloaded from)
 - rust-lang#98150 (Emscripten target: replace -g4 with -g, and -g3 with --profiling-funcs)
 - rust-lang#98195 (Fix rustdoc json primitive handling)
 - rust-lang#98205 (Remove a possible unnecessary assignment)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit d09a568 into rust-lang:master Jun 18, 2022
@rustbot rustbot added this to the 1.63.0 milestone Jun 18, 2022
@pietroalbini pietroalbini deleted the pa-config-artifacts branch June 20, 2022 16:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants