From aac0493cff7578eea9bdf2d8d945101e2d02f570 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Thu, 9 Jan 2020 12:54:42 -0500 Subject: [PATCH 1/7] add cleanup crew --- src/SUMMARY.md | 1 + src/ice-breaker/cleanup-crew.md | 66 +++++++++++++++++++++++++++++++++ 2 files changed, 67 insertions(+) create mode 100644 src/ice-breaker/cleanup-crew.md diff --git a/src/SUMMARY.md b/src/SUMMARY.md index 125e20cf6..d88d23082 100644 --- a/src/SUMMARY.md +++ b/src/SUMMARY.md @@ -30,6 +30,7 @@ - [`LintStore`](./diagnostics/lintstore.md) - [Diagnostic Codes](./diagnostics/diagnostic-codes.md) - [ICE-breaker teams](ice-breaker/about.md) + - ["Cleanup Crew" ICE-breakers](ice-breaker/cleanup-crew.md) - [LLVM ICE-breakers](ice-breaker/llvm.md) - [Part 2: How rustc works](./part-2-intro.md) - [High-level overview of the compiler source](./high-level-overview.md) diff --git a/src/ice-breaker/cleanup-crew.md b/src/ice-breaker/cleanup-crew.md new file mode 100644 index 000000000..947191524 --- /dev/null +++ b/src/ice-breaker/cleanup-crew.md @@ -0,0 +1,66 @@ +# Cleanup Crew + +**Github Label:** [ICEBreaker-Cleanup-Crew] + +[ICEBreaker-LLVM]: https://github.com/rust-lang/rust/labels/ICEBreaker-Cleanup-Crew + +The "Cleanup Crew" are focused on improving bug reports. Specifically, +the goal is to try to ensure that every bug report has all the +information that will be needed for someone to fix it: + +* a minimal, standalone example that shows the problem +* links to duplicates or related bugs +* if the bug is a regression (something that used to work, but no longer does), + then a bisection to the PR or nightly that caused the regression + +This kind of cleanup is invaluable in getting bugs fixed. Better +still, it can be done by anybody who knows Rust, without any +particularly deep knowledge of the compiler. + +Let's look a bit at the workflow for doing "cleanup crew" actions. + +## Finding a minimal, standalone example + +Here the ultimate goal is to produce an example that reproduces the same +problem but without relying on any external crates. Such a test ought to contain +as little code as possible, as well. This will make it much easier to isolate the problem. + +However, even if the "ultimate minimal test" cannot be achieved, it's +still useful to post incremental minimizations. For example, if you +can eliminate some of the external dependencies, that is helpful, and +so forth. + +It's particularly useful to reduce to an example that works +in the [Rust playground](https://play.rust-lang.org/), rather than +requiring people to checkout a cargo build. + +There are many resources for how to produce minimized test cases. Here +are a few: + +* The [rust-reduce](https://github.com/jethrogb/rust-reduce) tool can try to reduce + code automatically. + * The [C-reduce](https://embed.cs.utah.edu/creduce/) tool also works + on Rust code, though it requires that you start from a single + file. (XXX link to some post explaining how to do it?) +* pnkfelix's [Rust Bug Minimization Patterns] blog post + * This post focuses on "heavy bore" techniques, where you are + starting with a large, complex cargo project that you wish to + narrow down to something standalone. + +[Rust Bug Minimization Patterns]: http://blog.pnkfx.org/blog/2019/11/18/rust-bug-minimization-patterns/ + +## Links to duplicate or related bugs + +If you are on the "Cleanup Crew", you will sometimes see multiple bug +reports that seem very similar. You can link one to the other just by +mentioning the other bug number in a Github comment. Sometimes it is +useful to close duplicate bugs. But if you do so, you should always +copy any test case from the bug you are closing to the other bug that +remains open, as sometimes duplicate-looking bugs will expose +different facets of the same problem. + +## Bisection + +For regressions (something that used to work, but no longer does), we +have a great tool that + From eacd7f54ea88dbc70fe4034bbae4588fa66631a3 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Tue, 14 Jan 2020 05:24:54 -0500 Subject: [PATCH 2/7] Update src/ice-breaker/cleanup-crew.md Co-Authored-By: lqd --- src/ice-breaker/cleanup-crew.md | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/ice-breaker/cleanup-crew.md b/src/ice-breaker/cleanup-crew.md index 947191524..9b03ea8c9 100644 --- a/src/ice-breaker/cleanup-crew.md +++ b/src/ice-breaker/cleanup-crew.md @@ -2,7 +2,7 @@ **Github Label:** [ICEBreaker-Cleanup-Crew] -[ICEBreaker-LLVM]: https://github.com/rust-lang/rust/labels/ICEBreaker-Cleanup-Crew +[ICEBreaker-Cleanup-Crew]: https://github.com/rust-lang/rust/labels/ICEBreaker-Cleanup-Crew The "Cleanup Crew" are focused on improving bug reports. Specifically, the goal is to try to ensure that every bug report has all the @@ -63,4 +63,3 @@ different facets of the same problem. For regressions (something that used to work, but no longer does), we have a great tool that - From 5027c74f04da09df6db227ff927cb5b2953ce5cd Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Sun, 12 Jan 2020 17:10:03 -0500 Subject: [PATCH 3/7] update the text about carog-bisect-rustc etc --- src/ice-breaker/cleanup-crew.md | 40 ++++++++++++++++++++++++++++++--- 1 file changed, 37 insertions(+), 3 deletions(-) diff --git a/src/ice-breaker/cleanup-crew.md b/src/ice-breaker/cleanup-crew.md index 9b03ea8c9..849d0d500 100644 --- a/src/ice-breaker/cleanup-crew.md +++ b/src/ice-breaker/cleanup-crew.md @@ -59,7 +59,41 @@ copy any test case from the bug you are closing to the other bug that remains open, as sometimes duplicate-looking bugs will expose different facets of the same problem. -## Bisection +## Bisecting regressions + +For regressions (something that used to work, but no longer does), it +is super useful if we can figure out precisely when the code stopped +working. The gold standard is to be able to identify the precise +**PR** that broke the code, so we can ping the author, but even +narrowing it down to a nightly build is helpful, especially as that +then gives us a range of PRs. (One other challenge is that we +sometimes land "rollup" PRs, which combine multiple PRs into one.) + +### cargo-bisect-rustc + +To help in figuring out the cause of a regression we have a tool +called [cargo-bisect-rustc]. It will automatically download and ftest +various builds of rustc. For recent regressions, it is even able to +use the builds from our CI to track down the regression to a specific +PR; for older regressions, it will simply identify a nightly. + +To learn to use [cargo-bisect-rustc], check out [this blog +post][learn], which gives a quick introduction to how it works. You +can also ask questions at the Zulip stream +`#t-compiler/cargo-bisect-rustc`, or help in improving the tool. + +[cargo-bisect-rustc]: https://github.com/rust-lang/cargo-bisect-rustc/ +[learn]: https://blog.rust-lang.org/inside-rust/2019/12/18/bisecting-rust-compiler.html + +### identifying the range of PRs in a nightly + +If you've managed to narrow things down to a particular nightly build, +it is then helpful if we can identify the set of PRs that this +corresponds to. One helpful command in that regard is `rustc +nightly +-vV`, which will cause it to output a number of useful bits of version +info, including the `commit-hash`. Given the commit-hash of two +nightly versions, you can find all of PRs that have landed in between. + +(XXX Is there a more streamlined way to do this? Can we make one?) + -For regressions (something that used to work, but no longer does), we -have a great tool that From 91c7c2286636af5146a0bf73f3f9d31971b2e3b9 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Thu, 16 Jan 2020 14:17:15 -0500 Subject: [PATCH 4/7] finish the 3rd section --- src/ice-breaker/cleanup-crew.md | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/src/ice-breaker/cleanup-crew.md b/src/ice-breaker/cleanup-crew.md index 849d0d500..4b488a8aa 100644 --- a/src/ice-breaker/cleanup-crew.md +++ b/src/ice-breaker/cleanup-crew.md @@ -92,8 +92,19 @@ it is then helpful if we can identify the set of PRs that this corresponds to. One helpful command in that regard is `rustc +nightly -vV`, which will cause it to output a number of useful bits of version info, including the `commit-hash`. Given the commit-hash of two -nightly versions, you can find all of PRs that have landed in between. - -(XXX Is there a more streamlined way to do this? Can we make one?) - - +nightly versions, you can find all of PRs that have landed in between +by taking the following steps: + +1. Go to an update checkout of the [rust-lang/rust] repository +2. Execute the command `git log --author=bors --format=oneline SHA1..SHA2` + * This will list out all of the commits by bors, which is our merge bot + * Each commit corresponds to one PR, and information about the PR should be in the description +3. Copy and paste that information into the bug report + +Often, just eye-balling the PR descriptions (which are included in the +commit messages) will give you a good idea which one likely caused the +problem. But if you're unsure feel free to just ping the compiler team +(`@rust-lang/compiler`) or else to ping the authors of the PR +themselves. + +[rust-lang/rust]: https://github.com/rust-lang/rust/ From fa3c9c6688d6fe461f62e1c3af721d2f9bf5507c Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Thu, 16 Jan 2020 16:03:39 -0500 Subject: [PATCH 5/7] fix typo --- src/ice-breaker/cleanup-crew.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ice-breaker/cleanup-crew.md b/src/ice-breaker/cleanup-crew.md index 4b488a8aa..92d03a0cb 100644 --- a/src/ice-breaker/cleanup-crew.md +++ b/src/ice-breaker/cleanup-crew.md @@ -72,7 +72,7 @@ sometimes land "rollup" PRs, which combine multiple PRs into one.) ### cargo-bisect-rustc To help in figuring out the cause of a regression we have a tool -called [cargo-bisect-rustc]. It will automatically download and ftest +called [cargo-bisect-rustc]. It will automatically download and test various builds of rustc. For recent regressions, it is even able to use the builds from our CI to track down the regression to a specific PR; for older regressions, it will simply identify a nightly. From 3223bf9636f9d50e7f931f57600cabac729fa0c0 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Tue, 21 Jan 2020 14:56:55 -0500 Subject: [PATCH 6/7] clarify when you need to find PR range --- src/ice-breaker/cleanup-crew.md | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/src/ice-breaker/cleanup-crew.md b/src/ice-breaker/cleanup-crew.md index 92d03a0cb..80590ab43 100644 --- a/src/ice-breaker/cleanup-crew.md +++ b/src/ice-breaker/cleanup-crew.md @@ -87,13 +87,16 @@ can also ask questions at the Zulip stream ### identifying the range of PRs in a nightly -If you've managed to narrow things down to a particular nightly build, -it is then helpful if we can identify the set of PRs that this -corresponds to. One helpful command in that regard is `rustc +nightly --vV`, which will cause it to output a number of useful bits of version -info, including the `commit-hash`. Given the commit-hash of two -nightly versions, you can find all of PRs that have landed in between -by taking the following steps: +If the regression occurred more than 90 days ago, then +cargo-bisect-rustc will not able to identify the particular PR that +caused the regression, just the nightly build. In that case, we can +identify the set of PRs that this corresponds to by using the git +history. + +The command `rustc +nightly -vV` will cause rustc to output a number +of useful bits of version info, including the `commit-hash`. Given the +commit-hash of two nightly versions, you can find all of PRs that have +landed in between by taking the following steps: 1. Go to an update checkout of the [rust-lang/rust] repository 2. Execute the command `git log --author=bors --format=oneline SHA1..SHA2` From 4086ebe83c821233bdc100548376d5e573c305d8 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Wed, 22 Jan 2020 02:51:03 -0500 Subject: [PATCH 7/7] add link to zulip stream --- src/ice-breaker/cleanup-crew.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/ice-breaker/cleanup-crew.md b/src/ice-breaker/cleanup-crew.md index 80590ab43..436b51fd1 100644 --- a/src/ice-breaker/cleanup-crew.md +++ b/src/ice-breaker/cleanup-crew.md @@ -80,10 +80,11 @@ PR; for older regressions, it will simply identify a nightly. To learn to use [cargo-bisect-rustc], check out [this blog post][learn], which gives a quick introduction to how it works. You can also ask questions at the Zulip stream -`#t-compiler/cargo-bisect-rustc`, or help in improving the tool. +[`#t-compiler/cargo-bisect-rustc`][zcbr], or help in improving the tool. [cargo-bisect-rustc]: https://github.com/rust-lang/cargo-bisect-rustc/ [learn]: https://blog.rust-lang.org/inside-rust/2019/12/18/bisecting-rust-compiler.html +[zcbr]: https://rust-lang.zulipchat.com/#narrow/stream/217417-t-compiler.2Fcargo-bisect-rustc ### identifying the range of PRs in a nightly