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.

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

Remove FIXME about supporting beta for download-rustc #84929

Closed
wants to merge 1 commit into from
Closed

Remove FIXME about supporting beta for download-rustc #84929

wants to merge 1 commit into from

Conversation

jyn514
Copy link
Member

@jyn514 jyn514 commented May 4, 2021

@jyn514
Remove FIXME about supporting beta for download-rustc
e37c52d 
It already works as long as `dev: 1` isn't set.
@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 May 4, 2021
Mark-Simulacrum
Mark-Simulacrum reviewed May 6, 2021
View reviewed changes
src/bootstrap/bootstrap.py
@@ -664,7 +664,7 @@ def maybe_download_ci_toolchain(self):
if self.verbose:
print("using downloaded stage1 artifacts from CI (commit {})".format(commit))
self.rustc_commit = commit
# FIXME: support downloading artifacts from the beta channel
# NOTE: CI artifacts are stored under nightly even when this is run from the beta branch
Copy link
Member

Choose a reason for hiding this comment

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

This isn't true once beta has branched a while back I think; the last bors commit may be either beta or nightly, and it's hard to tell -- I guess we can look if that commit's src/ci/run.sh has a release-channel=beta but that feels pretty annoying.

Copy link
Member Author

@jyn514 jyn514 May 7, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Hmm, I'm not sure I understand when commits show up under beta - is it when a beta backport PR lands?

I guess the way to fix it is to check /beta if /nightly 404s? That seems simple enough; not sure if I understand this well enough to extend it to stable though.

@Mark-Simulacrum Mark-Simulacrum added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 6, 2021
@Mark-Simulacrum
Copy link
Member

Please also check if the LLVM download needs similar treatment; I think I ran into channel trouble there while looking at some bugs during the promotion, but I'm not sure.

@jyn514
Copy link
Member Author

jyn514 commented May 7, 2021

Please also check if the LLVM download needs similar treatment; I think I ran into channel trouble there while looking at some bugs during the promotion, but I'm not sure.

It worked ok for me on 4bac69d, maybe you ran into the issue where some things are uploaded under /beta? If you can find which commits that happens for it would be really helpful :)

@jyn514 jyn514 added T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-release Relevant to the release subteam, which will review and decide on the PR/issue. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels May 7, 2021
@Mark-Simulacrum
Copy link
Member

I don't really know what you mean by "uploaded under /beta" - I don't think we upload to channel specific locations at all?

I think the error might have been because the llvm toolchain was bumped on beta, which meant that getting the llvm download had to be from beta? I don't think this works as-is in all cases and since the cost of the FIXME is low I'd rather leave it in. I don't really have time to experiment with various cases though myself.

@Mark-Simulacrum
Copy link
Member

Mark-Simulacrum commented May 14, 2021 •
edited
Loading

Going to go ahead and move this over to waiting-on-author as I don't have time to do the investigation myself and it's not a big pain point to have the FIXME in. Feel free to close, if you also don't have the time.

@Mark-Simulacrum Mark-Simulacrum added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 14, 2021
@jyn514
Copy link
Member Author

jyn514 commented May 19, 2021

I think the error might have been because the llvm toolchain was bumped on beta, which meant that getting the llvm download had to be from beta?

Ah ok, that makes sense to me (in general, I'm not sure why we include the release channel and version in the LLVM .so). I don't have time to look into it now but I'm fine with leaving the FIXME in until I look into the LLVM issue.

@jyn514 jyn514 closed this May 19, 2021
@jyn514 jyn514 deleted the remove-beta-fixme branch May 19, 2021 02:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Mark-Simulacrum Mark-Simulacrum Mark-Simulacrum left review comments

Assignees

@Mark-Simulacrum Mark-Simulacrum

Labels
S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-release Relevant to the release subteam, which will review and decide on the PR/issue.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@jyn514 @rust-highfive @Mark-Simulacrum