Skip to content
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
6 changes: 3 additions & 3 deletions crates/prek/src/cli/hook_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -344,15 +344,15 @@ async fn parse_pre_push_info(remote_name: &str, stdin: &[u8]) -> Result<Option<P

// New remote ref, missing old remote object, or rebased force-push: find the
// commits reachable from the local tip that this remote cannot already reach.
let ancestors = git::get_ancestors_not_in_remote(local_sha, remote_name).await?;
let ancestors = git::ancestors_not_in_remote(local_sha, remote_name).await?;
if ancestors.is_empty() {
// The local tip is already reachable from the remote, so this line does
// not introduce files that need pre-push checks.
continue;
}

let first_ancestor = &ancestors[0];
let roots = git::get_root_commits(local_sha).await?;
let roots = git::root_commits(local_sha).await?;

if roots.contains(first_ancestor) {
// The first commit being pushed is a root commit. There is no parent to
Expand All @@ -369,7 +369,7 @@ async fn parse_pre_push_info(remote_name: &str, stdin: &[u8]) -> Result<Option<P
// Use the parent of the first remote-unknown commit as the diff base. For
// rebased force-pushes, this usually resolves to the updated default branch
// base, matching the files a pull request would show.
if let Some(source) = git::get_parent_commit(first_ancestor).await? {
if let Some(source) = git::parent_commit(first_ancestor).await? {
return Ok(Some(PushInfo {
from_ref: Some(source),
to_ref: Some(local_sha.to_string()),
Expand Down
8 changes: 4 additions & 4 deletions crates/prek/src/cli/install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,17 +83,17 @@ pub(crate) async fn install(
);
}

let hooks_path = git::get_git_common_dir().await?.join("hooks");
let hooks_path = git::common_dir().await?.join("hooks");
warn_user!(
"`core.hooksPath` is configured outside this repository. Installing Git shims to `{}` because `--force` was used.",
hooks_path.user_display().cyan()
);
hooks_path
} else {
git::get_git_hooks_dir().await?
git::hooks_dir().await?
};

let hook_mode = git::get_shared_repository_file_mode(0o755)
let hook_mode = git::shared_repository_file_mode(0o755)
.await
.unwrap_or(0o755);

Expand Down Expand Up @@ -427,7 +427,7 @@ pub(crate) async fn uninstall(
let hooks_path = if let Some(dir) = git_dir {
dir.join("hooks")
} else {
git::get_git_hooks_dir().await?
git::hooks_dir().await?
};

let types: Vec<HookType> = if all {
Expand Down
6 changes: 3 additions & 3 deletions crates/prek/src/cli/run/filter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -667,7 +667,7 @@ async fn collect_files_for_selection(
) -> Result<Vec<PathBuf>> {
match selection {
FileSelection::Diff { from_ref, to_ref } => {
let files = git::get_changed_files(&from_ref, &to_ref, workspace_root).await?;
let files = git::changed_files(&from_ref, &to_ref, workspace_root).await?;
debug!(
"Files changed between {} and {}: {}",
from_ref,
Expand All @@ -688,12 +688,12 @@ async fn collect_files_for_selection(
}
FileSelection::Default => {
if git::is_in_merge_conflict().await? {
let files = git::get_conflicted_files(workspace_root).await?;
let files = git::conflicted_files(workspace_root).await?;
debug!("Conflicted files: {}", files.len());
return Ok(files);
}

let files = git::get_staged_files(workspace_root).await?;
let files = git::staged_files(workspace_root).await?;
debug!("Staged files: {}", files.len());
Ok(files)
}
Expand Down
2 changes: 1 addition & 1 deletion crates/prek/src/cli/try_repo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ async fn clone_and_commit(repo_path: &Path, head_rev: &str, tmp_dir: &Path) -> R
let index_path = shadow.join(".git/index");
let objects_path = shadow.join(".git/objects");

let staged_files = git::get_staged_files(repo_path).await?;
let staged_files = git::staged_files(repo_path).await?;
if !staged_files.is_empty() {
git::git_cmd()?
.arg("add")
Expand Down
72 changes: 39 additions & 33 deletions crates/prek/src/git.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ fn git_work_tree() -> Option<&'static Path> {
}

pub(crate) static GIT_ROOT: LazyLock<Result<PathBuf, Error>> = LazyLock::new(|| {
get_root()
root()
.map(|root| dunce::canonicalize(&root).unwrap_or(root))
.inspect(|root| {
debug!("Git root: {}", root.display());
Expand Down Expand Up @@ -172,7 +172,7 @@ pub(crate) async fn intent_to_add_files(root: &Path) -> Result<Vec<PathBuf>, Err
Ok(zsplit(&output.stdout)?)
}

pub(crate) async fn get_added_files(root: &Path) -> Result<Vec<PathBuf>, Error> {
pub(crate) async fn staged_added_files(root: &Path) -> Result<Vec<PathBuf>, Error> {
let output = git_cmd()?
.current_dir(root)
.arg("diff")
Expand All @@ -190,7 +190,7 @@ pub(crate) async fn get_added_files(root: &Path) -> Result<Vec<PathBuf>, Error>
Ok(zsplit(&output.stdout)?)
}

pub(crate) async fn get_changed_files(
pub(crate) async fn changed_files(
old: &str,
new: &str,
root: &Path,
Expand Down Expand Up @@ -248,7 +248,7 @@ where
Ok(zsplit(&output.stdout)?)
}

pub(crate) async fn get_git_dir() -> Result<PathBuf, Error> {
pub(crate) async fn git_dir() -> Result<PathBuf, Error> {
let output = git_cmd()?
.arg("rev-parse")
.arg("--git-dir")
Expand All @@ -260,23 +260,23 @@ pub(crate) async fn get_git_dir() -> Result<PathBuf, Error> {
))
}

pub(crate) async fn get_git_common_dir() -> Result<PathBuf, Error> {
pub(crate) async fn common_dir() -> Result<PathBuf, Error> {
let output = git_cmd()?
.arg("rev-parse")
.arg("--git-common-dir")
.check(true)
.output()
.await?;
if output.stdout.trim_ascii().is_empty() {
Ok(get_git_dir().await?)
Ok(git_dir().await?)
} else {
Ok(PathBuf::from(
String::from_utf8_lossy(&output.stdout).trim_ascii(),
))
}
}

pub(crate) async fn get_git_hooks_dir() -> Result<PathBuf, Error> {
pub(crate) async fn hooks_dir() -> Result<PathBuf, Error> {
// Ask Git for the effective hooks directory instead of reconstructing it
// ourselves. That lets Git apply the full precedence chain for
// `core.hooksPath`, including local/worktree config, linked worktrees, bare
Expand All @@ -290,7 +290,7 @@ pub(crate) async fn get_git_hooks_dir() -> Result<PathBuf, Error> {
.output()
.await?;
let hooks_dir = if output.stdout.trim_ascii().is_empty() {
get_git_common_dir().await?.join("hooks")
common_dir().await?.join("hooks")
} else {
PathBuf::from(String::from_utf8_lossy(&output.stdout).trim_ascii())
};
Expand All @@ -308,7 +308,7 @@ pub(crate) async fn get_git_hooks_dir() -> Result<PathBuf, Error> {
}
}

pub(crate) async fn get_staged_files(root: &Path) -> Result<Vec<PathBuf>, Error> {
pub(crate) async fn staged_files(root: &Path) -> Result<Vec<PathBuf>, Error> {
let output = git_cmd()?
.current_dir(root)
.arg("diff")
Expand Down Expand Up @@ -365,11 +365,11 @@ pub(crate) async fn has_diff(rev: &str, path: &Path) -> Result<bool> {
}

pub(crate) async fn is_in_merge_conflict() -> Result<bool, Error> {
let git_dir = get_git_dir().await?;
let git_dir = git_dir().await?;
Ok(git_dir.join("MERGE_HEAD").try_exists()? && git_dir.join("MERGE_MSG").try_exists()?)
}

pub(crate) async fn get_conflicted_files(root: &Path) -> Result<Vec<PathBuf>, Error> {
pub(crate) async fn conflicted_files(root: &Path) -> Result<Vec<PathBuf>, Error> {
let tree = git_cmd()?.arg("write-tree").check(true).output().await?;

let output = git_cmd()?
Expand All @@ -396,7 +396,7 @@ pub(crate) async fn get_conflicted_files(root: &Path) -> Result<Vec<PathBuf>, Er
}

async fn parse_merge_msg_for_conflicts() -> Result<Vec<PathBuf>, Error> {
let git_dir = get_git_dir().await?;
let git_dir = git_dir().await?;
let merge_msg = git_dir.join("MERGE_MSG");
let content = fs_err::tokio::read_to_string(&merge_msg).await?;
let conflicts = content
Expand Down Expand Up @@ -471,9 +471,9 @@ pub(crate) async fn write_tree() -> Result<String, Error> {
.to_string())
}

/// Get the path of the top-level directory of the working tree.
/// Return the path of the top-level directory of the working tree.
#[instrument(level = "trace")]
pub(crate) fn get_root() -> Result<PathBuf, Error> {
pub(crate) fn root() -> Result<PathBuf, Error> {
let git = GIT.as_ref().map_err(|&e| Error::GitNotFound(e))?;
let mut cmd = Command::new(git);
let output = apply_git_work_tree(&mut cmd)
Expand Down Expand Up @@ -709,7 +709,7 @@ pub(crate) async fn clone_repo(
clone_repo_attempt(rev, path, terminal_prompt).await
}

async fn get_config_value(scope: Option<&str>, key: &str) -> Result<Option<Vec<u8>>, Error> {
async fn config_value(scope: Option<&str>, key: &str) -> Result<Option<Vec<u8>>, Error> {
let mut cmd = git_cmd()?;
cmd.arg("config").arg("--includes");
if let Some(scope) = scope {
Expand All @@ -729,11 +729,11 @@ async fn has_config_value(scope: Option<&str>, key: &str) -> Result<bool, Error>
// An empty config value still counts as configured and can affect Git's
// path resolution, e.g. `core.hooksPath=` makes `--git-path hooks`
// resolve to the current directory.
Ok(get_config_value(scope, key).await?.is_some())
Ok(config_value(scope, key).await?.is_some())
}

async fn config_value_is_empty(scope: Option<&str>, key: &str) -> Result<bool, Error> {
Ok(get_config_value(scope, key)
Ok(config_value(scope, key)
.await?
.as_deref()
.is_some_and(|value| value.strip_suffix(b"\0").unwrap_or(value).is_empty()))
Expand All @@ -752,7 +752,7 @@ pub(crate) async fn has_repo_hooks_path_set() -> Result<bool, Error> {
///
/// This mirrors the relevant parts of Git's `git_config_perm` in `setup.c`
/// and `calc_shared_perm` in `path.c`.
fn shared_repository_file_mode(value: &str, mode: u32) -> Option<u32> {
fn apply_shared_repository_file_mode(value: &str, mode: u32) -> Option<u32> {
const PERM_GROUP: u32 = 0o660;
const PERM_EVERYBODY: u32 = 0o664;

Expand Down Expand Up @@ -797,7 +797,7 @@ fn shared_repository_file_mode(value: &str, mode: u32) -> Option<u32> {
}

/// Resolve the file mode implied by `core.sharedRepository` for a newly created file.
pub(crate) async fn get_shared_repository_file_mode(mode: u32) -> Result<u32> {
pub(crate) async fn shared_repository_file_mode(mode: u32) -> Result<u32> {
let output = git_cmd()?
.arg("config")
.arg("--get")
Expand All @@ -807,13 +807,13 @@ pub(crate) async fn get_shared_repository_file_mode(mode: u32) -> Result<u32> {
.await?;
if output.status.success() {
let value = str::from_utf8(&output.stdout)?;
Ok(shared_repository_file_mode(value, mode).unwrap_or(mode))
Ok(apply_shared_repository_file_mode(value, mode).unwrap_or(mode))
} else {
Ok(mode)
}
}

pub(crate) async fn get_lfs_files(
pub(crate) async fn lfs_files(
current_dir: &Path,
paths: &[&Path],
) -> Result<FxHashSet<PathBuf>, Error> {
Expand Down Expand Up @@ -911,8 +911,8 @@ pub(crate) async fn is_ancestor(ancestor: &str, commit: &str) -> Result<bool, Er
Ok(false)
}

/// Get commits that are ancestors of the given commit but not in the specified remote
pub(crate) async fn get_ancestors_not_in_remote(
/// Return commits that are ancestors of the given commit but not in the specified remote.
pub(crate) async fn ancestors_not_in_remote(
local_sha: &str,
remote_name: &str,
) -> Result<Vec<String>, Error> {
Expand All @@ -933,8 +933,8 @@ pub(crate) async fn get_ancestors_not_in_remote(
.collect())
}

/// Get root commits (commits with no parents) for the given commit
pub(crate) async fn get_root_commits(local_sha: &str) -> Result<FxHashSet<String>, Error> {
/// Return root commits (commits with no parents) for the given commit.
pub(crate) async fn root_commits(local_sha: &str) -> Result<FxHashSet<String>, Error> {
let output = git_cmd()?
.arg("rev-list")
.arg("--max-parents=0")
Expand All @@ -949,8 +949,8 @@ pub(crate) async fn get_root_commits(local_sha: &str) -> Result<FxHashSet<String
.collect())
}

/// Get the parent commit of the given commit
pub(crate) async fn get_parent_commit(commit: &str) -> Result<Option<String>, Error> {
/// Return the parent commit of the given commit.
pub(crate) async fn parent_commit(commit: &str) -> Result<Option<String>, Error> {
let output = git_cmd()?
.arg("rev-parse")
.arg(format!("{commit}^"))
Expand Down Expand Up @@ -997,7 +997,7 @@ mod tests {
#[cfg(unix)]
use super::zsplit;
use super::{
Error, GIT, TerminalPrompt, full_clone, init_repo, shared_repository_file_mode,
Error, GIT, TerminalPrompt, apply_shared_repository_file_mode, full_clone, init_repo,
should_update_submodules, update_submodules,
};
use assert_cmd::assert::OutputAssertExt;
Expand Down Expand Up @@ -1107,27 +1107,33 @@ mod tests {
#[test]
fn shared_repository_group_mode_matches_git_behavior() {
for value in ["group", "true", "yes", "on", "1"] {
assert_eq!(shared_repository_file_mode(value, 0o755), Some(0o775));
assert_eq!(apply_shared_repository_file_mode(value, 0o755), Some(0o775));
}
}

#[test]
fn shared_repository_everybody_mode_matches_git_behavior() {
for value in ["all", "world", "everybody", "2"] {
assert_eq!(shared_repository_file_mode(value, 0o755), Some(0o775));
assert_eq!(apply_shared_repository_file_mode(value, 0o755), Some(0o775));
}
}

#[test]
fn shared_repository_octal_mode_matches_git_behavior() {
assert_eq!(shared_repository_file_mode("0640", 0o644), Some(0o640));
assert_eq!(shared_repository_file_mode("0640", 0o755), Some(0o750));
assert_eq!(
apply_shared_repository_file_mode("0640", 0o644),
Some(0o640)
);
assert_eq!(
apply_shared_repository_file_mode("0640", 0o755),
Some(0o750)
);
}

#[test]
fn shared_repository_umask_or_invalid_values_do_not_override_mode() {
for value in ["", "umask", "false", "no", "off", "0", "invalid", "0400"] {
assert_eq!(shared_repository_file_mode(value, 0o755), None);
assert_eq!(apply_shared_repository_file_mode(value, 0o755), None);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use std::path::{Path, PathBuf};
use clap::Parser;
use rustc_hash::FxHashSet;

use crate::git::{get_added_files, get_lfs_files};
use crate::git::{lfs_files, staged_added_files};
use crate::hook::Hook;
use crate::hooks::HookOutput;
use crate::hooks::pre_commit_hooks::{hook_filenames, parse_hook_args};
Expand Down Expand Up @@ -35,7 +35,7 @@ pub(crate) async fn run(hook: &Hook, filenames: &[&Path]) -> anyhow::Result<Hook
let filenames = if args.enforce_all {
filenames
} else {
let added_files = get_added_files(hook.work_dir())
let added_files = staged_added_files(hook.work_dir())
.await?
.into_iter()
.collect::<FxHashSet<_>>();
Expand All @@ -53,7 +53,7 @@ pub(crate) async fn run(hook: &Hook, filenames: &[&Path]) -> anyhow::Result<Hook

// Builtin hooks receive project-relative filenames, so git attribute lookups need to run
// from the project root for nested `.gitattributes` files to apply.
let lfs_files = get_lfs_files(hook.work_dir(), filenames).await?;
let lfs_files = lfs_files(hook.work_dir(), filenames).await?;

let filenames = filenames
.iter()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ pub(crate) async fn run(hook: &Hook, filenames: &[&Path]) -> Result<HookOutput>
}

// Get relevant files (filenames + added files) and include their parent directories.
let added = git::get_added_files(work_dir).await?;
let added = git::staged_added_files(work_dir).await?;
let mut relevant_files_with_dirs: FxHashSet<&Path> = FxHashSet::default();
for filename in &filenames {
insert_path_and_parents(&mut relevant_files_with_dirs, filename);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use anyhow::Result;
use clap::Parser;
use tokio::io::AsyncBufReadExt;

use crate::git::get_git_dir;
use crate::git::git_dir;
use crate::hook::Hook;
use crate::hooks::HookOutput;
use crate::hooks::pre_commit_hooks::{hook_filenames, parse_hook_args};
Expand Down Expand Up @@ -48,7 +48,7 @@ pub(crate) async fn run(hook: &Hook, filenames: &[&Path]) -> Result<HookOutput>

async fn is_in_merge() -> Result<bool> {
// Change directory temporarily or ensure we're in the right directory
let git_dir = get_git_dir().await?;
let git_dir = git_dir().await?;

// Check if MERGE_MSG exists
let merge_msg_exists = git_dir.join("MERGE_MSG").exists();
Expand Down
Loading
Loading