Skip to content

Configure nightly branch name in stage0.json #99149

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

Merged
merged 2 commits into from
Jul 13, 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
1 change: 1 addition & 0 deletions src/bootstrap/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,7 @@ pub struct Stage0Config {
pub artifacts_server: String,
pub artifacts_with_llvm_assertions_server: String,
pub git_merge_commit_email: String,
pub nightly_branch: String,
}
#[derive(Default, Deserialize)]
#[cfg_attr(test, derive(Clone))]
Expand Down
13 changes: 5 additions & 8 deletions src/bootstrap/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1280,14 +1280,11 @@ impl Build {
// Figure out how many merge commits happened since we branched off master.
// That's our beta number!
// (Note that we use a `..` range, not the `...` symmetric difference.)
let count = output(
self.config
.git()
.arg("rev-list")
.arg("--count")
.arg("--merges")
.arg("refs/remotes/origin/master..HEAD"),
);
let count =
output(self.config.git().arg("rev-list").arg("--count").arg("--merges").arg(format!(
"refs/remotes/origin/{}..HEAD",
self.config.stage0_metadata.config.nightly_branch
)));
let n = count.trim().parse().unwrap();
self.prerelease_version.set(Some(n));
n
Expand Down
3 changes: 2 additions & 1 deletion src/stage0.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@
"dist_server": "https://static.rust-lang.org",
"artifacts_server": "https://ci-artifacts.rust-lang.org/rustc-builds",
"artifacts_with_llvm_assertions_server": "https://ci-artifacts.rust-lang.org/rustc-builds-alt",
"git_merge_commit_email": "[email protected]"
"git_merge_commit_email": "[email protected]",
"nightly_branch": "master"
},
"__comments": [
"The configuration above this comment is editable, and can be changed",
Expand Down
2 changes: 1 addition & 1 deletion src/tools/bump-stage0/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -10,5 +10,5 @@ anyhow = "1.0.34"
curl = "0.4.38"
indexmap = { version = "1.7.0", features = ["serde"] }
serde = { version = "1.0.125", features = ["derive"] }
serde_json = "1.0.59"
serde_json = { version = "1.0.59", features = ["preserve_order"] }
toml = "0.5.7"
9 changes: 6 additions & 3 deletions src/tools/bump-stage0/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -198,9 +198,12 @@ struct Stage0 {
#[derive(Debug, serde::Serialize, serde::Deserialize)]
struct Config {
dist_server: String,
artifacts_server: String,
artifacts_with_llvm_assertions_server: String,
git_merge_commit_email: String,
// There are other fields in the configuration, which will be read by src/bootstrap or other
// tools consuming stage0.json. To avoid the need to update bump-stage0 every time a new field
// is added, we collect all the fields in an untyped Value and serialize them back with the
// same order and structure they were deserialized in.
Copy link
Member

Choose a reason for hiding this comment

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

I am not sure this will just work - https://github.com/serde-rs/json/blob/master/Cargo.toml#L59 might be enabled in our workspace, but I'm not immediately confident it is. I'm not sure optimizing to avoid editing this code is really necessary, and it seems like a good idea to have an explicit list so we don't clobber something in that list.

Copy link
Member Author

Choose a reason for hiding this comment

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

I am not sure this will just work - https://github.com/serde-rs/json/blob/master/Cargo.toml#L59 might be enabled in our workspace, but I'm not immediately confident it is.

Hmm, I explicitly enabled preserve_order in bump-stage0's Cargo.toml: it'd be quite problematic if that didn't enable the feature. Or do you mean something else?

I'm not sure optimizing to avoid editing this code is really necessary, and it seems like a good idea to have an explicit list so we don't clobber something in that list.

I added this commit because I originally forgot to add the new field to bump-stage0 😅

Copy link
Member

Choose a reason for hiding this comment

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

I... completely missed that change to Cargo.toml, not sure how. OK, taking another look it looks like I also managed to miss that we do keep a full list in bootstrap's testing, so that solves that concern too.

However, enabling that feature probably comes at some performance cost, and I think will affect the workspace? At least, it'll affect all bootstrap tools -- which I think might these days include some things that we ship, though not confident on that -- so let's run a try build and perf just in case, and then r=me.

#[serde(flatten)]
other: serde_json::Value,
}

#[derive(Debug, serde::Serialize, serde::Deserialize)]
Expand Down