Configure nightly branch name in stage0.json
#99149
Conversation
|
(rust-highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
| // 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. |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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 😅
There was a problem hiding this comment.
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.
|
@bors try @rust-timer queue |
|
Awaiting bors try build completion. @rustbot label: +S-waiting-on-perf |
rustbot
added
the
S-waiting-on-perf
|
⌛ Trying commit 6bc97d0 with merge 43835f653775a5c34c03d6beec22f1253809a9b4... |
|
☀️ Try build successful - checks-actions |
|
Queued 43835f653775a5c34c03d6beec22f1253809a9b4 with parent 7d1f57a, future comparison URL. |
|
Finished benchmarking commit (43835f653775a5c34c03d6beec22f1253809a9b4): comparison url. Instruction count
Max RSS (memory usage)Results
CyclesResults
If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf. Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf. @bors rollup=never Footnotes |
rustbot
removed
the
S-waiting-on-perf
|
@bors r+ |
bors
added
S-waiting-on-bors
|
☀️ Test successful - checks-actions |
|
Finished benchmarking commit (95e8b86): comparison url. Instruction count
Max RSS (memory usage)Results
CyclesThis benchmark run did not return any relevant results for this metric. If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf. @rustbot label: -perf-regression Footnotes |
The beta version number detection code relies on git to know how many merge commits were made since we branched off, and in doing so hardcodes
masteras the default branch name. This works for rust-lang/rust, but is problematic for forks that use a different default branch name (in Ferrocene we usemaininstead).This PR changes the code to instead load the default branch name from
src/stage0.json.bump-stage0has also been updated to remove the need to update it every time a new field is added tostage0.json.