Skip to content

Commit

Permalink
Auto merge of #7361 - xFrednet:5394-expand-setup-command, r=flip1995
Browse files Browse the repository at this point in the history
Added `cargo dev setup git-hook` and updated `cargo dev setup intellij` including a `remove` command

This PR enables our dev tool to install a git hook that formats the code before each commit and also runs `update_lints` to make sure that everything is registered correctly. The script is located at `util/etc/pre-commit.sh`. I found it reasonable to locate it in the `util` folder and decided to add a `etc` in correlation to the main rust repo and to bring a bit of structure into it.

* The hook can be installed via: `cargo dev setup git-hook`
* And removed via: `cargo dev remove git-hook`

cc: #5394

The refactoring of `src/ide_setup.rs` to `src/setup/intellij.rs` is an extra commit to simplify the review.

---

Changes:
* Added `cargo dev setup git-hook` for formatting before every commit
* Added `cargo dev remove git-hook` to remove the hook again
* Added `cargo dev remove intellij` to remove rustc source path dependencies
* Changed `cargo dev ide_setup` to `cargo dev setup intellij`

changelog: none

This is only an internal change and therefore not worth an entry in the general change log.

---

Tested on:
* [x] Linux (by `@xFrednet)`
* [ ] Windows (All used commands run inside the git bash, so it's very likely to work as well `@xFrednet)`
* [ ] macOS
  • Loading branch information
bors committed Jun 25, 2021
2 parents b286b38 + 8e969cd commit a03dfb9
Show file tree
Hide file tree
Showing 10 changed files with 384 additions and 120 deletions.
2 changes: 1 addition & 1 deletion CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ To work around this, you need to have a copy of the [rustc-repo][rustc_repo] ava
`git clone https://github.com/rust-lang/rust/`.
Then you can run a `cargo dev` command to automatically make Clippy use the rustc-repo via path-dependencies
which `IntelliJ Rust` will be able to understand.
Run `cargo dev ide_setup --repo-path <repo-path>` where `<repo-path>` is a path to the rustc repo
Run `cargo dev setup intellij --repo-path <repo-path>` where `<repo-path>` is a path to the rustc repo
you just cloned.
The command will add path-dependencies pointing towards rustc-crates inside the rustc repo to
Clippys `Cargo.toml`s and should allow `IntelliJ Rust` to understand most of the types that Clippy uses.
Expand Down
2 changes: 1 addition & 1 deletion clippy_dev/src/fmt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ pub fn run(check: bool, verbose: bool) {
},
CliError::RaSetupActive => {
eprintln!(
"error: a local rustc repo is enabled as path dependency via `cargo dev ide_setup`.
"error: a local rustc repo is enabled as path dependency via `cargo dev setup intellij`.
Not formatting because that would format the local repo as well!
Please revert the changes to Cargo.tomls first."
);
Expand Down
103 changes: 0 additions & 103 deletions clippy_dev/src/ide_setup.rs

This file was deleted.

2 changes: 1 addition & 1 deletion clippy_dev/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,9 @@ use walkdir::WalkDir;

pub mod bless;
pub mod fmt;
pub mod ide_setup;
pub mod new_lint;
pub mod serve;
pub mod setup;
pub mod stderr_length_check;
pub mod update_lints;

Expand Down
66 changes: 53 additions & 13 deletions clippy_dev/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@
// warn on lints, that are included in `rust-lang/rust`s bootstrap
#![warn(rust_2018_idioms, unused_lifetimes)]

use clap::{App, Arg, ArgMatches, SubCommand};
use clippy_dev::{bless, fmt, ide_setup, new_lint, serve, stderr_length_check, update_lints};
use clap::{App, AppSettings, Arg, ArgMatches, SubCommand};
use clippy_dev::{bless, fmt, new_lint, serve, setup, stderr_length_check, update_lints};
fn main() {
let matches = get_clap_config();

Expand Down Expand Up @@ -36,7 +36,20 @@ fn main() {
("limit_stderr_length", _) => {
stderr_length_check::check();
},
("ide_setup", Some(matches)) => ide_setup::run(matches.value_of("rustc-repo-path")),
("setup", Some(sub_command)) => match sub_command.subcommand() {
("intellij", Some(matches)) => setup::intellij::setup_rustc_src(
matches
.value_of("rustc-repo-path")
.expect("this field is mandatory and therefore always valid"),
),
("git-hook", Some(matches)) => setup::git_hook::install_hook(matches.is_present("force-override")),
_ => {},
},
("remove", Some(sub_command)) => match sub_command.subcommand() {
("git-hook", Some(_)) => setup::git_hook::remove_hook(),
("intellij", Some(_)) => setup::intellij::remove_rustc_src(),
_ => {},
},
("serve", Some(matches)) => {
let port = matches.value_of("port").unwrap().parse().unwrap();
let lint = matches.value_of("lint");
Expand All @@ -48,6 +61,7 @@ fn main() {

fn get_clap_config<'a>() -> ArgMatches<'a> {
App::new("Clippy developer tooling")
.setting(AppSettings::ArgRequiredElseHelp)
.subcommand(
SubCommand::with_name("bless")
.about("bless the test output changes")
Expand Down Expand Up @@ -140,16 +154,42 @@ fn get_clap_config<'a>() -> ArgMatches<'a> {
.about("Ensures that stderr files do not grow longer than a certain amount of lines."),
)
.subcommand(
SubCommand::with_name("ide_setup")
.about("Alter dependencies so Intellij Rust can find rustc internals")
.arg(
Arg::with_name("rustc-repo-path")
.long("repo-path")
.short("r")
.help("The path to a rustc repo that will be used for setting the dependencies")
.takes_value(true)
.value_name("path")
.required(true),
SubCommand::with_name("setup")
.about("Support for setting up your personal development environment")
.setting(AppSettings::ArgRequiredElseHelp)
.subcommand(
SubCommand::with_name("intellij")
.about("Alter dependencies so Intellij Rust can find rustc internals")
.arg(
Arg::with_name("rustc-repo-path")
.long("repo-path")
.short("r")
.help("The path to a rustc repo that will be used for setting the dependencies")
.takes_value(true)
.value_name("path")
.required(true),
),
)
.subcommand(
SubCommand::with_name("git-hook")
.about("Add a pre-commit git hook that formats your code to make it look pretty")
.arg(
Arg::with_name("force-override")
.long("force-override")
.short("f")
.help("Forces the override of an existing git pre-commit hook")
.required(false),
),
),
)
.subcommand(
SubCommand::with_name("remove")
.about("Support for undoing changes done by the setup command")
.setting(AppSettings::ArgRequiredElseHelp)
.subcommand(SubCommand::with_name("git-hook").about("Remove any existing pre-commit git hook"))
.subcommand(
SubCommand::with_name("intellij")
.about("Removes rustc source paths added via `cargo dev setup intellij`"),
),
)
.subcommand(
Expand Down
79 changes: 79 additions & 0 deletions clippy_dev/src/setup/git_hook.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
use std::fs;
use std::path::Path;

/// Rusts setup uses `git rev-parse --git-common-dir` to get the root directory of the repo.
/// I've decided against this for the sake of simplicity and to make sure that it doesn't install
/// the hook if `clippy_dev` would be used in the rust tree. The hook also references this tool
/// for formatting and should therefor only be used in a normal clone of clippy
const REPO_GIT_DIR: &str = ".git";
const HOOK_SOURCE_FILE: &str = "util/etc/pre-commit.sh";
const HOOK_TARGET_FILE: &str = ".git/hooks/pre-commit";

pub fn install_hook(force_override: bool) {
if !check_precondition(force_override) {
return;
}

// So a little bit of a funny story. Git on unix requires the pre-commit file
// to have the `execute` permission to be set. The Rust functions for modifying
// these flags doesn't seem to work when executed with normal user permissions.
//
// However, there is a little hack that is also being used by Rust itself in their
// setup script. Git saves the `execute` flag when syncing files. This means
// that we can check in a file with execution permissions and the sync it to create
// a file with the flag set. We then copy this file here. The copy function will also
// include the `execute` permission.
match fs::copy(HOOK_SOURCE_FILE, HOOK_TARGET_FILE) {
Ok(_) => {
println!("info: the hook can be removed with `cargo dev remove git-hook`");
println!("git hook successfully installed");
},
Err(err) => eprintln!(
"error: unable to copy `{}` to `{}` ({})",
HOOK_SOURCE_FILE, HOOK_TARGET_FILE, err
),
}
}

fn check_precondition(force_override: bool) -> bool {
// Make sure that we can find the git repository
let git_path = Path::new(REPO_GIT_DIR);
if !git_path.exists() || !git_path.is_dir() {
eprintln!("error: clippy_dev was unable to find the `.git` directory");
return false;
}

// Make sure that we don't override an existing hook by accident
let path = Path::new(HOOK_TARGET_FILE);
if path.exists() {
if force_override {
return delete_git_hook_file(path);
}

eprintln!("error: there is already a pre-commit hook installed");
println!("info: use the `--force-override` flag to override the existing hook");
return false;
}

true
}

pub fn remove_hook() {
let path = Path::new(HOOK_TARGET_FILE);
if path.exists() {
if delete_git_hook_file(path) {
println!("git hook successfully removed");
}
} else {
println!("no pre-commit hook was found");
}
}

fn delete_git_hook_file(path: &Path) -> bool {
if let Err(err) = fs::remove_file(path) {
eprintln!("error: unable to delete existing pre-commit git hook ({})", err);
false
} else {
true
}
}
Loading

0 comments on commit a03dfb9

Please sign in to comment.