From 809d1eb7f9a3109dfd6c876571a9ec910e4802bb Mon Sep 17 00:00:00 2001 From: Erin Power Date: Sun, 20 Oct 2019 21:44:35 +0200 Subject: [PATCH 1/7] Add Rollup Procedure --- src/SUMMARY.md | 1 + src/release/rollups.md | 60 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 61 insertions(+) create mode 100644 src/release/rollups.md diff --git a/src/SUMMARY.md b/src/SUMMARY.md index 908641af5..5b9d9e8db 100644 --- a/src/SUMMARY.md +++ b/src/SUMMARY.md @@ -20,6 +20,7 @@ - [Platform Support](./release/platform-support.md) - [Preparing Release Notes](./release/release-notes.md) - [Release Process](./release/process.md) + - [Rollup Procedure](./release/rollups.md) - [Triage Procedure](./release/triage-procedure.md) - [Archive](./archive/README.md) - [Friends of the Tree](./archive/fott.md) diff --git a/src/release/rollups.md b/src/release/rollups.md new file mode 100644 index 000000000..2d2f05549 --- /dev/null +++ b/src/release/rollups.md @@ -0,0 +1,60 @@ +# Rollup Procedure + +## Background + +The Rust project has a policy that every pull request must be tested after merge +before it can be pushed to master. As PR volume increases this can scale poorly, +especially given the long (>2hr) current CI duration for Rust. + +Enter rollups - low risk changes (often doc fixes or other non-functional +changes) are marked with the `rollup` command to bors (`@bors r+ rollup` to +approve a PR and mark as a rollup, `@bors rollup` to mark a previously approved +PR, `@bors rollup-` to unmark as a rollup). 'Performing a Rollup' then means +collecting these changes into one PR and merging them all at once. + +You can see the list of rollup PRs on Rust's [Homu queue], they are +listed at the bottom of the 'approved' queue with a priority of 'rollup' meaning +they will not be merged by themselves until everything in front of them in the +queue has been merged. + +## Making a Rollup + +1. Using the interface on [Homu queue], select a few pull requests and then use + "rollup" button to make one. +2. Use the `@bors r+ rollup=never p=` command in the + pull request thread. +3. Mark the pull request with the label `rollup`. +4. If the rollup fails, use the logs rust-highfive (really it is + rust-log-analyzer) provides to bisect the failure to a specific PR and do + `@bors r-`. If the PR is running, you need to do `@bors r-` retry. Otherwise, + success and proceed to the next rollup (every now and then let `rollup=never` + and toolstate PRs progess). +5. Recreate the rollup without the offending PR starting again from **1.** + +## Selecting Pull Requests + +This is something you will learn to improve over time. Some basic tips include +(from obvious to less): + +1. Avoid `rollup=never` PRs (these are red in the interface). +2. Include all PRs marked with `rollup=always` (these are green). Try to check + if some of the pull requests in this list shouldn't be rolled up — in the + interest of time you can do so sometimes after you've made the rollup PR. +3. Avoid pull requests that touch the CI configuration or bootstrap. +4. Avoid having too many large diff pull requests in the same rollup. +5. Avoid having too many submodule updates in the same rollup (especially LLVM). +6. Do not include toolstate PRs like those fixing Miri, Clippy, etc. +7. Do include docs PRs (they should hopefully be marked as green). + +## Failed rollups +If the rollup has failed, run the `@bors retry` command figure out if the +failure was spurious. If so, create a new PR, if not, find the possible +candidate PR(s) and unmark it (them) both as rollup and as being reviewed with +`@bors rollup- r-`, also commenting on the PR with the error. Hopefully the +author or a reviewer will give feedback to get the PR fixed or confirm that it's +not at fault. + +If a rollup continues to fail you can run the `@bors rollup=never` command to +never rollup the PR in question. + +[Homu queue]: https://buildbot2.rust-lang.org/homu/queue/rust From 722136727db88ef1369caaa5de8ecf602cdf4507 Mon Sep 17 00:00:00 2001 From: XAMPPRocky <4464295+XAMPPRocky@users.noreply.github.com> Date: Mon, 21 Oct 2019 09:06:42 +0200 Subject: [PATCH 2/7] Update src/release/rollups.md Co-Authored-By: Mazdak Farrokhzad --- src/release/rollups.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/release/rollups.md b/src/release/rollups.md index 2d2f05549..f4dd5f5f5 100644 --- a/src/release/rollups.md +++ b/src/release/rollups.md @@ -27,7 +27,7 @@ queue has been merged. 4. If the rollup fails, use the logs rust-highfive (really it is rust-log-analyzer) provides to bisect the failure to a specific PR and do `@bors r-`. If the PR is running, you need to do `@bors r-` retry. Otherwise, - success and proceed to the next rollup (every now and then let `rollup=never` + your rollup succeeded. If it did, proceed to the next rollup (every now and then let `rollup=never` and toolstate PRs progess). 5. Recreate the rollup without the offending PR starting again from **1.** From c220649e10ae2d7e40f39fa0f3886dc4bd418a5d Mon Sep 17 00:00:00 2001 From: XAMPPRocky <4464295+XAMPPRocky@users.noreply.github.com> Date: Mon, 21 Oct 2019 09:08:09 +0200 Subject: [PATCH 3/7] Apply suggestions from code review Co-Authored-By: Mazdak Farrokhzad --- src/release/rollups.md | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/src/release/rollups.md b/src/release/rollups.md index f4dd5f5f5..1d3927687 100644 --- a/src/release/rollups.md +++ b/src/release/rollups.md @@ -4,7 +4,7 @@ The Rust project has a policy that every pull request must be tested after merge before it can be pushed to master. As PR volume increases this can scale poorly, -especially given the long (>2hr) current CI duration for Rust. +especially given the long (~3.5hr) current CI duration for Rust. Enter rollups - low risk changes (often doc fixes or other non-functional changes) are marked with the `rollup` command to bors (`@bors r+ rollup` to @@ -20,13 +20,13 @@ queue has been merged. ## Making a Rollup 1. Using the interface on [Homu queue], select a few pull requests and then use - "rollup" button to make one. + "rollup" button to make one. (The text about fairness can be ignored.) 2. Use the `@bors r+ rollup=never p=` command in the pull request thread. 3. Mark the pull request with the label `rollup`. 4. If the rollup fails, use the logs rust-highfive (really it is rust-log-analyzer) provides to bisect the failure to a specific PR and do - `@bors r-`. If the PR is running, you need to do `@bors r-` retry. Otherwise, + `@bors r-`. If the PR is running, you need to do `@bors r- retry`. Otherwise, your rollup succeeded. If it did, proceed to the next rollup (every now and then let `rollup=never` and toolstate PRs progess). 5. Recreate the rollup without the offending PR starting again from **1.** @@ -41,14 +41,27 @@ This is something you will learn to improve over time. Some basic tips include if some of the pull requests in this list shouldn't be rolled up — in the interest of time you can do so sometimes after you've made the rollup PR. 3. Avoid pull requests that touch the CI configuration or bootstrap. + (Unless the CI PRs have been marked as `rollup`. -- see 2.) 4. Avoid having too many large diff pull requests in the same rollup. 5. Avoid having too many submodule updates in the same rollup (especially LLVM). + (Updating LLVM frequently forces most devs to rebuild LLVM which is not fun.) 6. Do not include toolstate PRs like those fixing Miri, Clippy, etc. 7. Do include docs PRs (they should hopefully be marked as green). ## Failed rollups -If the rollup has failed, run the `@bors retry` command figure out if the -failure was spurious. If so, create a new PR, if not, find the possible +If the rollup has failed, run the `@bors retry` command if the +failure was spurious (e.g. due to a network problem or a timeout). If it wasn't spurious, +find the offending PR and throw it out by copying a link to the rust-highfive comment, +and writing `Failed in , @bors r- retry`. Hopefully, +the author or reviewer will give feedback to get the PR fixed or confirm that it's not +at fault. + +Once you've removed the offending PR, re-create your rollup without it (see 1.). +Sometimes however, it is hard to find the offending PR. If so, use your intuition +to avoid the PRs that you suspect are the problem and recreate the rollup. +Another strategy is to raise the priority of the PRs you suspect, +mark them as `rollup=never` and let bors test them standalone to dismiss +or confirm your hypothesis. candidate PR(s) and unmark it (them) both as rollup and as being reviewed with `@bors rollup- r-`, also commenting on the PR with the error. Hopefully the author or a reviewer will give feedback to get the PR fixed or confirm that it's From 0ebf01448d9d4aa6eb200b863a2a9584599fcf0c Mon Sep 17 00:00:00 2001 From: XAMPPRocky <4464295+XAMPPRocky@users.noreply.github.com> Date: Mon, 21 Oct 2019 12:26:05 +0200 Subject: [PATCH 4/7] Update src/release/rollups.md Co-Authored-By: Yuki Okushi --- src/release/rollups.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/release/rollups.md b/src/release/rollups.md index 1d3927687..871205eec 100644 --- a/src/release/rollups.md +++ b/src/release/rollups.md @@ -28,7 +28,7 @@ queue has been merged. rust-log-analyzer) provides to bisect the failure to a specific PR and do `@bors r-`. If the PR is running, you need to do `@bors r- retry`. Otherwise, your rollup succeeded. If it did, proceed to the next rollup (every now and then let `rollup=never` - and toolstate PRs progess). + and toolstate PRs progress). 5. Recreate the rollup without the offending PR starting again from **1.** ## Selecting Pull Requests From 7f6eca7a5675ab9378d4932d42b7ea0385305a3b Mon Sep 17 00:00:00 2001 From: XAMPPRocky <4464295+XAMPPRocky@users.noreply.github.com> Date: Wed, 23 Oct 2019 12:48:18 +0200 Subject: [PATCH 5/7] Update src/release/rollups.md Co-Authored-By: Yuki Okushi --- src/release/rollups.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/release/rollups.md b/src/release/rollups.md index 871205eec..daacc02ae 100644 --- a/src/release/rollups.md +++ b/src/release/rollups.md @@ -52,7 +52,7 @@ This is something you will learn to improve over time. Some basic tips include If the rollup has failed, run the `@bors retry` command if the failure was spurious (e.g. due to a network problem or a timeout). If it wasn't spurious, find the offending PR and throw it out by copying a link to the rust-highfive comment, -and writing `Failed in , @bors r- retry`. Hopefully, +and writing `Failed in , @bors r-`. Hopefully, the author or reviewer will give feedback to get the PR fixed or confirm that it's not at fault. From dc2db65641fb7b72fc643d10d571f2b3bf8e6594 Mon Sep 17 00:00:00 2001 From: XAMPPRocky <4464295+XAMPPRocky@users.noreply.github.com> Date: Wed, 23 Oct 2019 13:12:09 +0200 Subject: [PATCH 6/7] Update rollups.md --- src/release/rollups.md | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/src/release/rollups.md b/src/release/rollups.md index daacc02ae..ee8b88d8c 100644 --- a/src/release/rollups.md +++ b/src/release/rollups.md @@ -6,11 +6,17 @@ The Rust project has a policy that every pull request must be tested after merge before it can be pushed to master. As PR volume increases this can scale poorly, especially given the long (~3.5hr) current CI duration for Rust. -Enter rollups - low risk changes (often doc fixes or other non-functional -changes) are marked with the `rollup` command to bors (`@bors r+ rollup` to +Enter rollups - changes that small, not performance sensitive, or not platform +dependent are marked with the `rollup` command to bors (`@bors r+ rollup` to approve a PR and mark as a rollup, `@bors rollup` to mark a previously approved -PR, `@bors rollup-` to unmark as a rollup). 'Performing a Rollup' then means -collecting these changes into one PR and merging them all at once. +PR, `@bors rollup-` to un-mark as a rollup). 'Performing a Rollup' then means +collecting these changes into one PR and merging them all at once. The rollup +command accepts three values `always`, `maybe`, and `never`. `@bors rollup` is +equivalent to `rollup=always` (which will always put a PR in a rollup), and +`@bors rollup-` is equivalent to `@bors rollup=maybe` (which will try to put +the PR into a rollup). `rollup=never` will never a PR in a rollup, this should +generally only be used for PRs which are large additions or changes which could +cause breakage or large perf changes. You can see the list of rollup PRs on Rust's [Homu queue], they are listed at the bottom of the 'approved' queue with a priority of 'rollup' meaning @@ -27,8 +33,8 @@ queue has been merged. 4. If the rollup fails, use the logs rust-highfive (really it is rust-log-analyzer) provides to bisect the failure to a specific PR and do `@bors r-`. If the PR is running, you need to do `@bors r- retry`. Otherwise, - your rollup succeeded. If it did, proceed to the next rollup (every now and then let `rollup=never` - and toolstate PRs progress). + your rollup succeeded. If it did, proceed to the next rollup (every now and + then let `rollup=never` and toolstate PRs progress). 5. Recreate the rollup without the offending PR starting again from **1.** ## Selecting Pull Requests @@ -62,10 +68,6 @@ to avoid the PRs that you suspect are the problem and recreate the rollup. Another strategy is to raise the priority of the PRs you suspect, mark them as `rollup=never` and let bors test them standalone to dismiss or confirm your hypothesis. -candidate PR(s) and unmark it (them) both as rollup and as being reviewed with -`@bors rollup- r-`, also commenting on the PR with the error. Hopefully the -author or a reviewer will give feedback to get the PR fixed or confirm that it's -not at fault. If a rollup continues to fail you can run the `@bors rollup=never` command to never rollup the PR in question. From e8d313ad3e6d4ecb9dbf2620ba3f17acd144ef2e Mon Sep 17 00:00:00 2001 From: XAMPPRocky <4464295+XAMPPRocky@users.noreply.github.com> Date: Wed, 23 Oct 2019 17:50:54 +0200 Subject: [PATCH 7/7] Update rollups.md --- src/release/rollups.md | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/release/rollups.md b/src/release/rollups.md index ee8b88d8c..ed552fb4f 100644 --- a/src/release/rollups.md +++ b/src/release/rollups.md @@ -12,11 +12,12 @@ approve a PR and mark as a rollup, `@bors rollup` to mark a previously approved PR, `@bors rollup-` to un-mark as a rollup). 'Performing a Rollup' then means collecting these changes into one PR and merging them all at once. The rollup command accepts three values `always`, `maybe`, and `never`. `@bors rollup` is -equivalent to `rollup=always` (which will always put a PR in a rollup), and -`@bors rollup-` is equivalent to `@bors rollup=maybe` (which will try to put -the PR into a rollup). `rollup=never` will never a PR in a rollup, this should -generally only be used for PRs which are large additions or changes which could -cause breakage or large perf changes. +equivalent to `rollup=always` (which will indicate that a PR should always be +included in a rollup), and `@bors rollup-` is equivalent to `@bors rollup=maybe` +which is used to indicate that someone should try rollup the PR. `rollup=never` +indicates that a PR should never be included in a rollup, this should generally +only be used for PRs which are large additions or changes which could cause +breakage or large perf changes. You can see the list of rollup PRs on Rust's [Homu queue], they are listed at the bottom of the 'approved' queue with a priority of 'rollup' meaning