From 17d877cce28b22b9b345ec7ef8b14859d8b165c4 Mon Sep 17 00:00:00 2001 From: flip1995 Date: Sun, 3 May 2020 15:59:57 +0200 Subject: [PATCH 1/6] Update contributing section about syncing Clippy --- CONTRIBUTING.md | 62 +++++++++++++++++++++---------------------------- 1 file changed, 26 insertions(+), 36 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 50a5ee8bbf3..079a51eae3b 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -155,47 +155,37 @@ That's why the `else_if_without_else` example uses the `register_early_pass` fun ## Fixing build failures caused by Rust -Clippy will sometimes fail to build from source because building it depends on unstable internal Rust features. Most of -the times we have to adapt to the changes and only very rarely there's an actual bug in Rust. Fixing build failures -caused by Rust updates, can be a good way to learn about Rust internals. +Clippy currently gets build with `rustc` of the `rust-lang/rust` `master` +branch. Most of the times we have to adapt to the changes and only very rarely +there's an actual bug in Rust. -In order to find out why Clippy does not work properly with a new Rust commit, you can use the [rust-toolstate commit -history][toolstate_commit_history]. You will then have to look for the last commit that contains -`test-pass -> build-fail` or `test-pass -> test-fail` for the `clippy-driver` component. -[Here][toolstate_commit] is an example. +If you decide to make Clippy work again with a Rust commit that breaks it, you +have to sync the `rust-lang/rust-clippy` repository with the `subtree` copy of +Clippy in the `rust-lang/rust` repository. -The commit message contains a link to the PR. The PRs are usually small enough to discover the breaking API change and -if they are bigger, they likely include some discussion that may help you to fix Clippy. +For general information about `subtree`s in the Rust repository see [Rust's +`CONTRIBUTING.md`][subtree]. -To check if Clippy is available for a specific target platform, you can check -the [rustup component history][rustup_component_history]. +Here is a TL;DR version of the sync process: -If you decide to make Clippy work again with a Rust commit that breaks it, -you probably want to install the latest Rust from master locally and run Clippy -using that version of Rust. +1. Clone the [`rust-lang/rust`] repository (all of the following commands have + to be run inside the `rust` directory) +2. Sync the changes to the copy of Clippy to your fork of the Clippy repository: + ```bash + # Make sure to change `your-github-name` to your github name in the following command + git subtree push -P src/tools/clippy git@github.com:your-github-name/rust-clippy sync-from-rust + ``` +3. Open a PR to `rust-lang/rust-clippy` and wait for it to get merged (to + accelerate the process ping the `@rust-lang/clippy` team in your PR and/or + ~~annoy~~ ask them in the [Discord] channel.) +4. Sync the `rust-lang/rust-clippy` master to the copy of Clippy: + ```bash + git checkout -b sync-from-clippy + git subtree pull -P src/tools/clippy https://github.com/rust-lang/rust-clippy master + ``` +5. Open a PR to [`rust-lang/rust`] -You can set up the master toolchain by running `./setup-toolchain.sh`. That script will install -[rustup-toolchain-install-master][rtim] and master toolchain, then run `rustup override set master`. - -After fixing the build failure on this repository, we can submit a pull request -to [`rust-lang/rust`] to fix the toolstate. - -To submit a pull request, you should follow these steps: - -```bash -# Assuming you already cloned the rust-lang/rust repo and you're in the correct directory -git submodule update --remote src/tools/clippy -cargo update -p clippy -git add -u -git commit -m "Update Clippy" -./x.py test -i --stage 1 src/tools/clippy # This is optional and should succeed anyway -# Open a PR in rust-lang/rust -``` - -[rustup_component_history]: https://rust-lang.github.io/rustup-components-history -[toolstate_commit_history]: https://github.com/rust-lang-nursery/rust-toolstate/commits/master -[toolstate_commit]: https://github.com/rust-lang-nursery/rust-toolstate/commit/aad74d8294e198a7cf8ac81a91aebb7f3bbcf727 -[rtim]: https://github.com/kennytm/rustup-toolchain-install-master +[subtree]: https://github.com/rust-lang/rust/blob/master/CONTRIBUTING.md#external-dependencies-subtree [`rust-lang/rust`]: https://github.com/rust-lang/rust ## Issue and PR triage From c1698fedeb69109f9b1aebc0bfccd9bf3112ccad Mon Sep 17 00:00:00 2001 From: Philipp Krones Date: Sun, 3 May 2020 16:47:57 +0200 Subject: [PATCH 2/6] Apply suggestions from code review Co-authored-by: Phil Hansch --- CONTRIBUTING.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 079a51eae3b..f7a60938374 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -155,7 +155,7 @@ That's why the `else_if_without_else` example uses the `register_early_pass` fun ## Fixing build failures caused by Rust -Clippy currently gets build with `rustc` of the `rust-lang/rust` `master` +Clippy currently gets built with `rustc` of the `rust-lang/rust` `master` branch. Most of the times we have to adapt to the changes and only very rarely there's an actual bug in Rust. @@ -170,7 +170,7 @@ Here is a TL;DR version of the sync process: 1. Clone the [`rust-lang/rust`] repository (all of the following commands have to be run inside the `rust` directory) -2. Sync the changes to the copy of Clippy to your fork of the Clippy repository: +2. Sync the changes to the rust-copy of Clippy to your Clippy fork: ```bash # Make sure to change `your-github-name` to your github name in the following command git subtree push -P src/tools/clippy git@github.com:your-github-name/rust-clippy sync-from-rust @@ -178,7 +178,7 @@ Here is a TL;DR version of the sync process: 3. Open a PR to `rust-lang/rust-clippy` and wait for it to get merged (to accelerate the process ping the `@rust-lang/clippy` team in your PR and/or ~~annoy~~ ask them in the [Discord] channel.) -4. Sync the `rust-lang/rust-clippy` master to the copy of Clippy: +4. Sync the `rust-lang/rust-clippy` master to the rust-copy of Clippy: ```bash git checkout -b sync-from-clippy git subtree pull -P src/tools/clippy https://github.com/rust-lang/rust-clippy master From e5b5f6f8a99253560096a5718fc7fc81daa23e02 Mon Sep 17 00:00:00 2001 From: Philipp Krones Date: Sun, 17 May 2020 01:38:01 +0200 Subject: [PATCH 3/6] Better explain remotes in the sync process. --- CONTRIBUTING.md | 30 +++++++++++++++++++++++++++--- 1 file changed, 27 insertions(+), 3 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index f7a60938374..6697ff2f40d 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -166,15 +166,18 @@ Clippy in the `rust-lang/rust` repository. For general information about `subtree`s in the Rust repository see [Rust's `CONTRIBUTING.md`][subtree]. -Here is a TL;DR version of the sync process: +Here is a TL;DR version of the sync process (all of the following commands have +to be run inside the `rust` directory): -1. Clone the [`rust-lang/rust`] repository (all of the following commands have - to be run inside the `rust` directory) +1. Clone the [`rust-lang/rust`] repository 2. Sync the changes to the rust-copy of Clippy to your Clippy fork: ```bash # Make sure to change `your-github-name` to your github name in the following command git subtree push -P src/tools/clippy git@github.com:your-github-name/rust-clippy sync-from-rust ``` + _Note:_ This will directly push to the remote repository. You can also push + to your local copy by replacing the remote address with `/path/to/rust-clippy` + directory. 3. Open a PR to `rust-lang/rust-clippy` and wait for it to get merged (to accelerate the process ping the `@rust-lang/clippy` team in your PR and/or ~~annoy~~ ask them in the [Discord] channel.) @@ -185,6 +188,27 @@ Here is a TL;DR version of the sync process: ``` 5. Open a PR to [`rust-lang/rust`] +Also, you may want to define remotes, so you don't have to type out the remote +addresses on every sync. You can do this with the following commands (these +commands still have to be run inside the `rust` directory): + +```bash +# Set clippy-upstream remote for pulls +$ git remote add clippy-upstream https://github.com/rust-lang/rust-clippy +# Make sure to not push to the upstream repo +$ git remote set-url --push clippy-upstream DISABLED +# Set clippy-origin remote to your fork for pushes +$ git remote add clippy-origin git@github.com:your-github-name/rust-clippy +# Set a local remote +$ git remote add clippy-local /path/to/rust-clippy +``` + +You can then sync with the remote names from above, e.g.: + +```bash +$ git subtree push -P src/tools/clippy clippy-local sync-from-rust +``` + [subtree]: https://github.com/rust-lang/rust/blob/master/CONTRIBUTING.md#external-dependencies-subtree [`rust-lang/rust`]: https://github.com/rust-lang/rust From 842dd072612a5a53bef37f242f8f2be8896902cc Mon Sep 17 00:00:00 2001 From: flip1995 Date: Tue, 19 May 2020 15:18:16 +0200 Subject: [PATCH 4/6] Add note that a subtree fix and stack limit increase is required --- CONTRIBUTING.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 6697ff2f40d..c6a3998ec4e 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -209,6 +209,13 @@ You can then sync with the remote names from above, e.g.: $ git subtree push -P src/tools/clippy clippy-local sync-from-rust ``` +_Note:_ The first time running `git subtree push` a cache has to be built. This +involves going through the complete Clippy history once. For this you have to +increase the stack limit though, which you can do with `ulimit -s 60000`. For +this to work, you will need the fix of `git subtree` available +[here][gitgitgadget-pr]. + +[gitgitgadget-pr]: https://github.com/gitgitgadget/git/pull/493 [subtree]: https://github.com/rust-lang/rust/blob/master/CONTRIBUTING.md#external-dependencies-subtree [`rust-lang/rust`]: https://github.com/rust-lang/rust From 1a9ba3b2c208f8131587ece3ad8fb159336dd694 Mon Sep 17 00:00:00 2001 From: flip1995 Date: Tue, 19 May 2020 16:36:14 +0200 Subject: [PATCH 5/6] Add note, that a merge commit after push is necessary --- CONTRIBUTING.md | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index c6a3998ec4e..c9180e58fc2 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -178,6 +178,15 @@ to be run inside the `rust` directory): _Note:_ This will directly push to the remote repository. You can also push to your local copy by replacing the remote address with `/path/to/rust-clippy` directory. + + _Note:_ Most of the time you have to create a merge commit in the + `rust-clippy` repo (this has to be done in the Clippy repo, not in the + rust-copy of Clippy): + ```bash + git checkout sync-from-rust + git fetch upstream + git merge upstream/master + ``` 3. Open a PR to `rust-lang/rust-clippy` and wait for it to get merged (to accelerate the process ping the `@rust-lang/clippy` team in your PR and/or ~~annoy~~ ask them in the [Discord] channel.) From 7a83eafd44b57196a454d10628d1cce1bfd60bd2 Mon Sep 17 00:00:00 2001 From: Philipp Krones Date: Mon, 25 May 2020 17:11:07 +0200 Subject: [PATCH 6/6] Also fetch origin before merging master into the rustup branch --- CONTRIBUTING.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index c9180e58fc2..0f47ac98fd2 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -183,8 +183,8 @@ to be run inside the `rust` directory): `rust-clippy` repo (this has to be done in the Clippy repo, not in the rust-copy of Clippy): ```bash + git fetch origin && git fetch upstream git checkout sync-from-rust - git fetch upstream git merge upstream/master ``` 3. Open a PR to `rust-lang/rust-clippy` and wait for it to get merged (to