Skip to content

Added cargo dev setup git-hook and updated cargo dev setup intellij including a remove command #7361

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

Merged
merged 8 commits into from
Jun 25, 2021
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