Skip to content

Make clippy a subrepo #70650

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

Closed
wants to merge 10 commits into from
Closed
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
The table of contents is too big for display.
Diff view
Diff view
  •  
  •  
  •  
3 changes: 0 additions & 3 deletions .gitmodules
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,6 @@
[submodule "src/tools/rls"]
path = src/tools/rls
url = https://github.com/rust-lang/rls.git
[submodule "src/tools/clippy"]
path = src/tools/clippy
url = https://github.com/rust-lang/rust-clippy.git
[submodule "src/tools/rustfmt"]
path = src/tools/rustfmt
url = https://github.com/rust-lang/rustfmt.git
Expand Down
49 changes: 46 additions & 3 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -188,11 +188,55 @@ with one another are rolled up.
Speaking of tests, Rust has a comprehensive test suite. More information about
it can be found [here][rctd].

### External Dependencies
### External Dependencies (subrepo)

Currently building Rust will also build the following external projects:
As a developer to this repository, you don't have to treat the following external projects
differently from other crates that are directly in this repo:

* [clippy](https://github.com/rust-lang/rust-clippy)

They are just regular files and directories. This is in contrast to `submodule` dependencies
(see below for those).

If you want to synchronize or otherwise work with subrepos, install the `git subrepo` command via
instructions found at https://github.com/ingydotnet/git-subrepo

#### Synchronizing a subrepo

There are two synchronization directions: `subrepo push` and `subrepo pull`.

A `git subrepo push src/tools/clippy`
takes all the changes that
happened to the copy in this repo and creates commits on the remote repo that match the local
changes (so ever local commit that touched the subrepo causes a commit on the remote repo). Again,
Even this `subrepo push` operation creates a commit in this repo that you need to get merged without
rebasing. This is very important in order to make future synchronizations work.

A `git subrepo pull src/tools/clippy` takes all changes since the last `subrepo pull` from the clippy
repo and creates a single commit in the rustc repo with all the changes. Again, do not rebase this
commit under any circumstances. Redo the operation if you need to.

#### Creating a new subrepo dependency

If you want to create a new subrepo dependency from an existing repository, call (from this
repository's root directory!!)

```
git subrepo clone https://github.com/rust-lang/rust-clippy.git src/tools/clippy
```

This will create a new commit, which you may not rebase under any circumstances! Delete the commit
and redo the operation if you need to rebase.

Now you're done, the `src/tools/clippy` directory behaves as if clippy were part of the rustc
monorepo, so no one but you (or others that synchronize subrepos) needs to have `git subrepo`
installed.


### External Dependencies (submodules)

Currently building Rust will also build the following external projects:

* [miri](https://github.com/rust-lang/miri)
* [rustfmt](https://github.com/rust-lang/rustfmt)
* [rls](https://github.com/rust-lang/rls/)
Expand Down Expand Up @@ -221,7 +265,6 @@ before the PR is merged.

Rust's build system builds a number of tools that make use of the
internals of the compiler. This includes
[Clippy](https://github.com/rust-lang/rust-clippy),
[RLS](https://github.com/rust-lang/rls) and
[rustfmt](https://github.com/rust-lang/rustfmt). If these tools
break because of your changes, you may run into a sort of "chicken and egg"
Expand Down
77 changes: 28 additions & 49 deletions src/bootstrap/dist.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1330,7 +1330,7 @@ pub struct Clippy {
}

impl Step for Clippy {
type Output = Option<PathBuf>;
type Output = PathBuf;
const ONLY_HOSTS: bool = true;

fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
Expand All @@ -1348,7 +1348,7 @@ impl Step for Clippy {
});
}

fn run(self, builder: &Builder<'_>) -> Option<PathBuf> {
fn run(self, builder: &Builder<'_>) -> PathBuf {
let compiler = self.compiler;
let target = self.target;
assert!(builder.config.extended);
Expand All @@ -1368,16 +1368,10 @@ impl Step for Clippy {
// state for clippy isn't testing.
let clippy = builder
.ensure(tool::Clippy { compiler, target, extra_features: Vec::new() })
.or_else(|| {
missing_tool("clippy", builder.build.config.missing_tools);
None
})?;
.expect("clippy expected to build - essential tool");
let cargoclippy = builder
.ensure(tool::CargoClippy { compiler, target, extra_features: Vec::new() })
.or_else(|| {
missing_tool("cargo clippy", builder.build.config.missing_tools);
None
})?;
.expect("clippy expected to build - essential tool");
Copy link
Member

@RalfJung RalfJung Apr 1, 2020

Choose a reason for hiding this comment

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

The config.toml.example still lists clippy in the tool list and as an example of an "extended tool". Is that outdated now?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

kinda, you can still turn it off to not include it in the dist


builder.install(&clippy, &image.join("bin"), 0o755);
builder.install(&cargoclippy, &image.join("bin"), 0o755);
Expand Down Expand Up @@ -1416,7 +1410,7 @@ impl Step for Clippy {
builder.info(&format!("Dist clippy stage{} ({})", compiler.stage, target));
let _time = timeit(builder);
builder.run(&mut cmd);
Some(distdir(builder).join(format!("{}-{}.tar.gz", name, target)))
distdir(builder).join(format!("{}-{}.tar.gz", name, target))
}
}

Expand Down Expand Up @@ -1683,7 +1677,7 @@ impl Step for Extended {
tarballs.push(rustc_installer);
tarballs.push(cargo_installer);
tarballs.extend(rls_installer.clone());
tarballs.extend(clippy_installer.clone());
tarballs.push(clippy_installer);
tarballs.extend(miri_installer.clone());
tarballs.extend(rustfmt_installer.clone());
tarballs.extend(llvm_tools_installer);
Expand Down Expand Up @@ -1761,9 +1755,6 @@ impl Step for Extended {
if rls_installer.is_none() {
contents = filter(&contents, "rls");
}
if clippy_installer.is_none() {
contents = filter(&contents, "clippy");
}
if miri_installer.is_none() {
contents = filter(&contents, "miri");
}
Expand Down Expand Up @@ -1805,13 +1796,11 @@ impl Step for Extended {
prepare("rust-docs");
prepare("rust-std");
prepare("rust-analysis");
prepare("clippy");

if rls_installer.is_some() {
prepare("rls");
}
if clippy_installer.is_some() {
prepare("clippy");
}
if miri_installer.is_some() {
prepare("miri");
}
Expand Down Expand Up @@ -1863,12 +1852,10 @@ impl Step for Extended {
prepare("rust-analysis");
prepare("rust-docs");
prepare("rust-std");
prepare("clippy");
if rls_installer.is_some() {
prepare("rls");
}
if clippy_installer.is_some() {
prepare("clippy");
}
if miri_installer.is_some() {
prepare("miri");
}
Expand Down Expand Up @@ -1989,25 +1976,23 @@ impl Step for Extended {
.arg(etc.join("msi/remove-duplicates.xsl")),
);
}
if clippy_installer.is_some() {
builder.run(
Command::new(&heat)
.current_dir(&exe)
.arg("dir")
.arg("clippy")
.args(&heat_flags)
.arg("-cg")
.arg("ClippyGroup")
.arg("-dr")
.arg("Clippy")
.arg("-var")
.arg("var.ClippyDir")
.arg("-out")
.arg(exe.join("ClippyGroup.wxs"))
.arg("-t")
.arg(etc.join("msi/remove-duplicates.xsl")),
);
}
builder.run(
Command::new(&heat)
.current_dir(&exe)
.arg("dir")
.arg("clippy")
.args(&heat_flags)
.arg("-cg")
.arg("ClippyGroup")
.arg("-dr")
.arg("Clippy")
.arg("-var")
.arg("var.ClippyDir")
.arg("-out")
.arg(exe.join("ClippyGroup.wxs"))
.arg("-t")
.arg(etc.join("msi/remove-duplicates.xsl")),
);
if miri_installer.is_some() {
builder.run(
Command::new(&heat)
Expand Down Expand Up @@ -2073,6 +2058,7 @@ impl Step for Extended {
.arg("-dCargoDir=cargo")
.arg("-dStdDir=rust-std")
.arg("-dAnalysisDir=rust-analysis")
.arg("-dClippyDir=clippy")
.arg("-arch")
.arg(&arch)
.arg("-out")
Expand All @@ -2083,9 +2069,6 @@ impl Step for Extended {
if rls_installer.is_some() {
cmd.arg("-dRlsDir=rls");
}
if clippy_installer.is_some() {
cmd.arg("-dClippyDir=clippy");
}
if miri_installer.is_some() {
cmd.arg("-dMiriDir=miri");
}
Expand All @@ -2101,12 +2084,10 @@ impl Step for Extended {
candle("DocsGroup.wxs".as_ref());
candle("CargoGroup.wxs".as_ref());
candle("StdGroup.wxs".as_ref());
candle("ClippyGroup.wxs".as_ref());
if rls_installer.is_some() {
candle("RlsGroup.wxs".as_ref());
}
if clippy_installer.is_some() {
candle("ClippyGroup.wxs".as_ref());
}
if miri_installer.is_some() {
candle("MiriGroup.wxs".as_ref());
}
Expand Down Expand Up @@ -2138,14 +2119,12 @@ impl Step for Extended {
.arg("CargoGroup.wixobj")
.arg("StdGroup.wixobj")
.arg("AnalysisGroup.wixobj")
.arg("ClippyGroup.wixobj")
.current_dir(&exe);

if rls_installer.is_some() {
cmd.arg("RlsGroup.wixobj");
}
if clippy_installer.is_some() {
cmd.arg("ClippyGroup.wixobj");
}
if miri_installer.is_some() {
cmd.arg("MiriGroup.wixobj");
}
Expand Down
6 changes: 2 additions & 4 deletions src/bootstrap/install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -214,10 +214,8 @@ install!((self, builder, _config),
}
};
Clippy, "clippy", Self::should_build(_config), only_hosts: true, {
if builder.ensure(dist::Clippy {
compiler: self.compiler,
target: self.target,
}).is_some() || Self::should_install(builder) {
builder.ensure(dist::Clippy { compiler: self.compiler, target: self.target });
if Self::should_install(builder) {
install_clippy(builder, self.compiler.stage, self.target);
} else {
builder.info(
Expand Down
6 changes: 2 additions & 4 deletions src/bootstrap/tool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -644,14 +644,12 @@ tool_extended!((self, builder),
Miri, miri, "src/tools/miri", "miri", {};
CargoMiri, miri, "src/tools/miri", "cargo-miri", {};
Rls, rls, "src/tools/rls", "rls", {
let clippy = builder.ensure(Clippy {
builder.ensure(Clippy {
compiler: self.compiler,
target: self.target,
extra_features: Vec::new(),
});
if clippy.is_some() {
self.extra_features.push("clippy".to_owned());
}
self.extra_features.push("clippy".to_owned());
};
Rustfmt, rustfmt, "src/tools/rustfmt", "rustfmt", {};
);
Expand Down
1 change: 0 additions & 1 deletion src/tools/clippy
Submodule clippy deleted from e170c8
6 changes: 6 additions & 0 deletions src/tools/clippy/.cargo/config
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
[alias]
uitest = "test --test compile-test"
dev = "run --package clippy_dev --bin clippy_dev --manifest-path clippy_dev/Cargo.toml --"

[build]
rustflags = ["-Zunstable-options"]
19 changes: 19 additions & 0 deletions src/tools/clippy/.editorconfig
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
# EditorConfig helps developers define and maintain consistent
# coding styles between different editors and IDEs
# editorconfig.org

root = true

[*]
end_of_line = lf
charset = utf-8
trim_trailing_whitespace = true
insert_final_newline = true
indent_style = space
indent_size = 4

[*.md]
trim_trailing_whitespace = false

[*.yml]
indent_size = 2
3 changes: 3 additions & 0 deletions src/tools/clippy/.gitattributes
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
* text=auto eol=lf
*.rs text eol=lf whitespace=tab-in-indent,trailing-space,tabwidth=4
*.fixed linguist-language=Rust
8 changes: 8 additions & 0 deletions src/tools/clippy/.github/ISSUE_TEMPLATE.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<!--
Hi there! Whether you've come to make a suggestion for a new lint, an improvement to an existing lint or to report a bug or a false positive in Clippy, you've come to the right place.

For bug reports and false positives, please include the output of `cargo clippy -V` in the report.

Thank you for using Clippy!

Write your comment below this line: -->
31 changes: 31 additions & 0 deletions src/tools/clippy/.github/PULL_REQUEST_TEMPLATE.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
Thank you for making Clippy better!

We're collecting our changelog from pull request descriptions.
If your PR only updates to the latest nightly, you can leave the
`changelog` entry as `none`. Otherwise, please write a short comment
explaining your change.

If your PR fixes an issue, you can add "fixes #issue_number" into this
PR description. This way the issue will be automatically closed when
your PR is merged.

If you added a new lint, here's a checklist for things that will be
checked during review or continuous integration.

- [ ] Followed [lint naming conventions][lint_naming]
- [ ] Added passing UI tests (including committed `.stderr` file)
- [ ] `cargo test` passes locally
- [ ] Executed `cargo dev update_lints`
- [ ] Added lint documentation
- [ ] Run `cargo dev fmt`

[lint_naming]: https://rust-lang.github.io/rfcs/0344-conventions-galore.html#lints

Note that you can skip the above if you are just opening a WIP PR in
order to get feedback.

Delete this line and everything above before opening your PR.

---

changelog: none
Loading