-
Notifications
You must be signed in to change notification settings - Fork 13
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
Implement release process for rustup #84
base: master
Are you sure you want to change the base?
Changes from 1 commit
00e1a90
0a64d25
8d4a447
d40ff45
b803170
da3164d
49980a3
c4cff77
7ccff04
ede844c
1e516af
b1dd2a2
b843e51
f5c3c5e
a947033
883b5ed
ad86e13
694e3a4
8132d69
89f7324
f82d531
9b7dba8
5aad90d
a52f975
839b35d
ad0c357
fd3f06d
a3a0d89
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,142 @@ | ||
use crate::Context; | ||
use std::fs; | ||
use std::path::{Path, PathBuf}; | ||
|
||
use anyhow::{anyhow, Error}; | ||
|
||
use crate::config::Channel; | ||
use crate::{run, Context}; | ||
|
||
impl Context { | ||
pub fn do_rustup(&mut self) -> anyhow::Result<()> { | ||
/// Promote a `rustup` release | ||
/// | ||
/// The [release process] for `rustup` involves copying existing artifacts from one S3 bucket to | ||
/// another, updating the manifest, and archiving the artifacts for long-term storage. | ||
/// | ||
/// `rustup` uses different branches to manage releases. Whenever a commit is pushed to the | ||
/// `stable` branch in [rust-lang/rustup], GitHub Actions workflows build release artifacts and | ||
/// copy them into `s3://dev-static-rust-lang-org/rustup/dist/`. | ||
/// | ||
/// When a new release is done and this method is invoked, it downloads the artifacts from that | ||
/// bucket (which must always be set as the `DOWNLOAD_BUCKET` variable). A copy of the artifacts | ||
/// is archived in `s3://${UPLOAD_BUCKET}/rustup/archive/${version}/`, where `version` is passed | ||
/// to this program as a command-line argument. `UPLOAD_BUCKET` can either be the `dev-static` | ||
/// or the `static` bucket. | ||
/// | ||
/// If the release is for the `stable` channel, the artifacts are also copied to the `dist/` | ||
/// path in the `UPLOAD_BUCKET` bucket. The `dist/` path is used by the `rustup` installer to | ||
/// download the latest release. | ||
/// | ||
/// Then, the `release-stable.toml` manifest is updated with the new version and copied to | ||
/// `s3://${UPLOAD_BUCKET}/rustup/release-stable.toml`. | ||
/// | ||
/// [release process]: https://rust-lang.github.io/rustup/dev-guide/release-process.html | ||
/// [rust-lang/rustup]: https://github.com/rust-lang/rustup | ||
pub fn promote_rustup(&mut self) -> anyhow::Result<()> { | ||
println!("Checking channel..."); | ||
if self.config.channel != Channel::Stable && self.config.channel != Channel::Beta { | ||
return Err(anyhow!( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This feels really weird, since the current environments for There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why are we making the channel be the condition here? Promote release already has dev static and static, as two separate environments - should be able to ignore channels I'd expect. (In a manner similar to promote branches ignoring them iirc) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Without channels, how would you distinguish between beta and stable releases? Only be setting the respective environment variables (e.g. |
||
"promoting rustup is only supported for the stable and beta channels" | ||
)); | ||
} | ||
|
||
// Download the rustup artifacts from S3 | ||
println!("Downloading artifacts from dev-static..."); | ||
let dist_dir = self.download_rustup_artifacts()?; | ||
|
||
// Archive the artifacts | ||
println!("Archiving artifacts..."); | ||
self.archive_rustup_artifacts(&dist_dir)?; | ||
|
||
if self.config.channel == Channel::Stable { | ||
// Promote the artifacts to the release bucket | ||
println!("Promoting artifacts to dist/..."); | ||
self.promote_rustup_artifacts(&dist_dir)?; | ||
} | ||
|
||
// Update the release number | ||
println!("Updating version and manifest..."); | ||
self.update_rustup_release()?; | ||
|
||
Ok(()) | ||
} | ||
|
||
fn download_rustup_artifacts(&mut self) -> Result<PathBuf, Error> { | ||
let dl = self.dl_dir().join("dist"); | ||
// Remove the directory if it exists, otherwise just ignore. | ||
let _ = fs::remove_dir_all(&dl); | ||
fs::create_dir_all(&dl)?; | ||
|
||
run(self | ||
.aws_s3() | ||
.arg("cp") | ||
.arg("--recursive") | ||
.arg("--only-show-errors") | ||
.arg(&self.s3_artifacts_url("dist/")) | ||
.arg(format!("{}/", dl.display())))?; | ||
|
||
Ok(dl) | ||
} | ||
|
||
fn archive_rustup_artifacts(&mut self, dist_dir: &Path) -> Result<(), Error> { | ||
let version = self | ||
.current_version | ||
.as_ref() | ||
.ok_or_else(|| anyhow!("failed to get current version for rustup release"))?; | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This doesn't work, since The easiest workaround might be setting a new There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we should figure out how to persist the intended version with the artifacts. That should be possible similarly to how rust has a version containing file checked in, and we can upload that directly to the S3 bucket in rustup's existing "CI". There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree with Mark that the best approach here is for rustup to store the version number in a file in the repository, so that they can update it on their own. In general, regarding arguments, I don't see any problem with sticking with environment variables. We never invoke the CodeBuild job directly, we always go through https://github.com/rust-lang/simpleinfra/blob/master/release-scripts/promote-release.py, which has a CLI parser and then sets the environment variables. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. An early step in our release process is a version bump in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The version is now read from the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @djc Now that we're at it, maybe it makes sense to use workspace-wide version numbers? I don't see how There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @jdno FYI: |
||
let path = format!("archive/{}/", version); | ||
|
||
self.upload_rustup_artifacts(dist_dir, &path) | ||
} | ||
|
||
fn promote_rustup_artifacts(&mut self, dist_dir: &Path) -> Result<(), Error> { | ||
let release_bucket_url = format!( | ||
"s3://{}/{}/{}", | ||
self.config.upload_bucket, | ||
self.config.download_dir, | ||
dist_dir.display(), | ||
); | ||
|
||
run(self | ||
.aws_s3() | ||
.arg("cp") | ||
.arg("--recursive") | ||
.arg("--only-show-errors") | ||
.arg(format!("{}/", dist_dir.display())) | ||
.arg(&release_bucket_url)) | ||
} | ||
|
||
fn upload_rustup_artifacts(&mut self, dist_dir: &Path, target_path: &str) -> Result<(), Error> { | ||
run(self | ||
.aws_s3() | ||
.arg("cp") | ||
.arg("--recursive") | ||
.arg("--only-show-errors") | ||
.arg(format!("{}/", dist_dir.display())) | ||
.arg(&self.s3_artifacts_url(target_path))) | ||
} | ||
|
||
fn update_rustup_release(&mut self) -> Result<(), Error> { | ||
let version = self | ||
.current_version | ||
.as_ref() | ||
.ok_or_else(|| anyhow!("failed to get current version for rustup release"))?; | ||
|
||
let manifest_path = self.dl_dir().join("release-stable.toml"); | ||
let manifest = format!( | ||
r#" | ||
schema-version = '1' | ||
version = '{}' | ||
"#, | ||
version | ||
); | ||
|
||
fs::write(&manifest_path, manifest)?; | ||
|
||
run(self | ||
.aws_s3() | ||
.arg("cp") | ||
.arg("--only-show-errors") | ||
.arg(manifest_path) | ||
.arg(&self.s3_artifacts_url("release-stable.toml"))) | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not great, I'd prefer if we tweak rustup's CI to upload them to a location like
s3://rustup-artifacts/${commit}
, check what is the latest commit hash on the stable branch, and download from that location. Every build overriding the previous build feels iffy.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The bucket has been created here and a PR is open here to upload artifacts to
s3://rustup-builds/builds/${commit}/
.