Skip to content
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

feat: improve behavior when config file is missing #53

Merged
merged 5 commits into from
Jul 11, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .vscode/cSpell.json
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
"rustup",
"sendemail",
"setvariable",
"shellcheck",
"shiba",
"swellaby",
"testresults",
Expand Down
2 changes: 1 addition & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "rusty-hook"
version = "0.8.4"
version = "0.9.0"
authors = ["Swellaby <[email protected]>"]
description = "git hook utility"
license = "MIT"
Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ Just add `rusty-hook` as a dev dependency in your Cargo.toml file:

```toml
[dev-dependencies]
rusty-hook = "0.8.4"
rusty-hook = "0.9.0"
```

## Initialize
Expand Down
16 changes: 8 additions & 8 deletions src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ verbose = true

const DEFAULT_CONFIG_FILE_NAME: &str = ".rusty-hook.toml";
const CONFIG_FILE_NAMES: [&str; 2] = [DEFAULT_CONFIG_FILE_NAME, "rusty-hook.toml"];
const NO_CONFIG_FILE_FOUND: &str = "No config file found";
pub const NO_CONFIG_FILE_FOUND: &str = "No config file found";
pub const MISSING_CONFIG_KEY: &str = "Missing config key";

fn find_config_file<F>(root_directory_path: &str, file_exists: F) -> Result<String, String>
Expand Down Expand Up @@ -106,14 +106,14 @@ where
G: Fn(&str) -> Result<bool, ()>,
{
let path = match find_config_file(root_directory_path, &file_exists) {
Ok(path) => path,
Err(_) => {
return Err(String::from(NO_CONFIG_FILE_FOUND));
Ok(path) => {
if path == NO_CONFIG_FILE_FOUND {
return Err(String::from(NO_CONFIG_FILE_FOUND));
} else {
path
}
}
};

if path == NO_CONFIG_FILE_FOUND {
return Err(String::from(NO_CONFIG_FILE_FOUND));
Err(_) => return Err(String::from(NO_CONFIG_FILE_FOUND)),
};

match read_file(&path) {
Expand Down
34 changes: 9 additions & 25 deletions src/git.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
pub const NO_CONFIG_FILE_FOUND_ERROR_CODE: i32 = 3;

pub fn get_root_directory_path<F>(run_command: F, target_directory: &str) -> Result<String, String>
where
F: Fn(&str, &str) -> Result<String, String>,
Expand All @@ -12,30 +14,7 @@ where
run_command("git rev-parse --git-path hooks", &root_directory)
}

const HOOK_FILE_TEMPLATE: &str = "#!/bin/sh
# rusty-hook
# version {{VERSION}}

hookName=$(basename \"$0\")
gitParams=\"$*\"

if ! command -v rusty-hook >/dev/null 2>&1; then
if [ -z \"${RUSTY_HOOK_SKIP_AUTO_INSTALL}\" ]; then
echo \"Finalizing rusty-hook configuration...\"
echo \"This may take a few seconds...\"
cargo install rusty-hook >/dev/null 2>&1
else
echo \"rusty-hook is not installed, and auto install is disabled\"
echo \"skipping $hookName hook\"
echo \"You can reinstall it using 'cargo install rusty-hook' or delete this hook\"
exit 0
fi
fi

# echo \"rusty-hook version: $(rusty-hook --version)\"
# echo \"hook file version: {{VERSION}}\"
rusty-hook run --hook \"$hookName\" \"$gitParams\"";

const HOOK_FILE_TEMPLATE: &str = include_str!("hook_script.sh");
const HOOK_NAMES: [&str; 19] = [
"applypatch-msg",
"pre-applypatch",
Expand Down Expand Up @@ -72,7 +51,12 @@ where
Err(_) => return Err(String::from("Failure determining git hooks directory")),
};
let version = env!("CARGO_PKG_VERSION");
let hook_file_contents = String::from(HOOK_FILE_TEMPLATE).replace("{{VERSION}}", version);
let exit_code = &NO_CONFIG_FILE_FOUND_ERROR_CODE.to_string();
let hook_file_contents = String::from(HOOK_FILE_TEMPLATE)
.replace("{{VERSION}}", version)
.replace("\n# shellcheck disable=SC2170,SC1083", "")
.replace("{{NO_CONFIG_FILE_EXIT_CODE}}", exit_code);

for hook in HOOK_NAMES.iter() {
if write_file(
&format!("{}/{}/{}", root_directory_path, hooks_directory, hook),
Expand Down
6 changes: 5 additions & 1 deletion src/git_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,11 @@ mod create_hook_files_tests {
let version = env!("CARGO_PKG_VERSION");
let root_dir = "/usr/repos/foo";
let git_hooks = ".git/hooks";
let exp_contents = &String::from(HOOK_FILE_TEMPLATE).replace("{{VERSION}}", version);
let exit_code = &NO_CONFIG_FILE_FOUND_ERROR_CODE.to_string();
let exp_contents = String::from(HOOK_FILE_TEMPLATE)
.replace("{{VERSION}}", version)
.replace("\n# shellcheck disable=SC2170,SC1083", "")
.replace("{{NO_CONFIG_FILE_EXIT_CODE}}", exit_code);
let run_command = |_cmd: &str, _dir: &str| Ok(String::from(git_hooks));
let write_file = |path: &str, contents: &str, make_executable: bool| {
let act_hook = &&path[(path.rfind('/').unwrap() + 1)..];
Expand Down
41 changes: 41 additions & 0 deletions src/hook_script.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
#!/bin/sh
# rusty-hook
# version {{VERSION}}

hookName=$(basename "$0")
gitParams="$*"

if ! command -v rusty-hook >/dev/null 2>&1; then
if [ -z "${RUSTY_HOOK_SKIP_AUTO_INSTALL}" ]; then
echo "Finalizing rusty-hook configuration..."
echo "This may take a few seconds..."
cargo install rusty-hook >/dev/null 2>&1
else
echo "rusty-hook is not installed, and auto install is disabled"
echo "skipping $hookName hook"
echo "You can reinstall it using 'cargo install rusty-hook' or delete this hook"
exit 0
fi
fi

rusty-hook run --hook "$hookName" "$gitParams"
rhExitCode=$?

if [ $rhExitCode -ne 0 ]; then
# shellcheck disable=SC2170,SC1083
if [ $rhExitCode -eq {{NO_CONFIG_FILE_EXIT_CODE}} ]; then
Copy link
Member Author

Choose a reason for hiding this comment

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

{{NO_CONFIG_FILE_EXIT_CODE}} is tokenized here so that rusty-hook can inject the correct value as part of writing all the git hook script files. Unsurprisingly, shellcheck doesn't like that, hence the above inline disables.

if [ "$hookName" = "pre-commit" ]; then
echo "rusty-hook git hooks are configured, but no config file was found"
echo "In order to use rusty-hook, your project must have a config file"
echo "See https://github.com/swellaby/rusty-hook#configure for more information"
echo
echo "If you were trying to remove rusty-hook, then you should also delete the git hook files to remove this warning"
echo
Copy link
Member Author

@calebcartwright calebcartwright Jul 10, 2019

Choose a reason for hiding this comment

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

We can update this message later on after #51 is completed to include a link to those docs on how to remove rusty-hook. I think it's more important to get this new behavior out though, so plan on tackling #51 (and probably #48) in a follow up PR.

fi
exit 0
else
echo "Configured hook command failed"
echo "$hookName hook rejected"
exit $rhExitCode
fi
fi
8 changes: 6 additions & 2 deletions src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,12 @@ fn run(args: Vec<String>) {
&closures::get_logger(),
&hook_name,
) {
eprintln!("{}", err);
exit(1);
if err == rusty_hook::NO_CONFIG_FILE_FOUND {
exit(rusty_hook::NO_CONFIG_FILE_FOUND_ERROR_CODE);
} else {
eprintln!("{}", err);
exit(1);
}
}
}

Expand Down
24 changes: 16 additions & 8 deletions src/rusty_hook.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@ mod config;
#[path = "git.rs"]
mod git;

pub use config::NO_CONFIG_FILE_FOUND;
pub use git::NO_CONFIG_FILE_FOUND_ERROR_CODE;

pub fn init_directory<F, G, H>(
run_command: F,
write_file: G,
Expand Down Expand Up @@ -62,7 +65,13 @@ where
let config_file_contents =
match config::get_config_file_contents(read_file, file_exists, &root_directory_path) {
Ok(contents) => contents,
Err(_) => return Err(String::from("Failed to locate or parse config file")),
Err(err) => {
if err == config::NO_CONFIG_FILE_FOUND {
return Err(String::from(config::NO_CONFIG_FILE_FOUND));
} else {
return Err(String::from("Failed to parse config file"));
}
}
};

let log_details = config::get_log_setting(&config_file_contents);
Expand All @@ -72,7 +81,7 @@ where
if err == config::MISSING_CONFIG_KEY {
return Ok(());
}
return Err(String::from("Invalid config file"));
return Err(String::from("Invalid rusty-hook config file"));
}
};

Expand All @@ -81,14 +90,13 @@ where
log(&format!("Running command: {}", script));
};

match run_command(&script, &root_directory_path) {
Ok(stdout) => {
if log_details {
log(&stdout);
}
match (run_command(&script, &root_directory_path), log_details) {
(Ok(stdout), true) => {
log(&stdout);
Ok(())
}
Err(stderr) => Err(stderr),
(Ok(_), false) => Ok(()),
(Err(stderr), _) => Err(stderr),
}
}

Expand Down
17 changes: 14 additions & 3 deletions src/rusty_hook_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,8 @@ mod run_tests {
}

#[test]
fn returns_error_when_config_contents_unloadable() {
let exp_err = "Failed to locate or parse config file";
fn returns_error_when_config_file_missing() {
let exp_err = config::NO_CONFIG_FILE_FOUND;
let run_command = |_cmd: &str, _dir: &str| Ok(String::from(""));
let read_file = |_file_path: &str| Err(());
let file_exists = |_path: &str| Ok(false);
Expand All @@ -89,6 +89,17 @@ mod run_tests {
assert_eq!(result, Err(String::from(exp_err)));
}

#[test]
fn returns_error_when_config_contents_unloadable() {
let exp_err = "Failed to parse config file";
let run_command = |_cmd: &str, _dir: &str| Ok(String::from(""));
let read_file = |_file_path: &str| Err(());
let file_exists = |_path: &str| Ok(true);
let log = |_path: &str| panic!("");
let result = run(run_command, file_exists, read_file, log, "");
assert_eq!(result, Err(String::from(exp_err)));
}

#[test]
fn returns_ok_when_hook_missing() {
let contents = "[hooks]
Expand All @@ -104,7 +115,7 @@ mod run_tests {

#[test]
fn returns_error_on_invalid_config() {
let exp_err = "Invalid config file";
let exp_err = "Invalid rusty-hook config file";
let contents = "abc";
let run_command = |_cmd: &str, _dir: &str| Ok(String::from(""));
let read_file = |_file_path: &str| Ok(String::from(contents));
Expand Down