From 3c885334d9d3db402b096406ac2de7ad4c334337 Mon Sep 17 00:00:00 2001 From: Jo <10510431+j178@users.noreply.github.com> Date: Fri, 10 Jul 2026 04:00:51 +0800 Subject: [PATCH 1/2] Fix try-repo local path resolution --- crates/prek/src/cli/try_repo.rs | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/crates/prek/src/cli/try_repo.rs b/crates/prek/src/cli/try_repo.rs index fbae87814..f39712058 100644 --- a/crates/prek/src/cli/try_repo.rs +++ b/crates/prek/src/cli/try_repo.rs @@ -11,6 +11,7 @@ use toml_edit::{Array, ArrayOfTables, DocumentMut, InlineTable, Item, Value}; use crate::cli::run::Selectors; use crate::cli::{ExitStatus, RunOptions, flag}; use crate::config::{self, Stage}; +use crate::fs::PathClean; use crate::git; use crate::git::GIT_ROOT; use crate::printer::Printer; @@ -97,10 +98,23 @@ async fn prepare_repo_and_rev<'a>( ) -> Result<(Cow<'a, str>, String)> { let repo_path = Path::new(repo); let is_local = repo_path.is_dir(); + // The generated config lives in the scratch directory, where relative repo paths would be + // resolved against the wrong base. Preserve the CLI argument's meaning before writing it. + let repo = if is_local { + Cow::Owned( + std::path::absolute(repo_path)? + .clean() + .to_string_lossy() + .into_owned(), + ) + } else { + Cow::Borrowed(repo) + }; + let repo_path = Path::new(repo.as_ref()); // If rev is provided, use it directly. if let Some(rev) = rev { - return Ok((Cow::Borrowed(repo), rev.to_string())); + return Ok((repo, rev.to_string())); } // Get HEAD revision @@ -130,7 +144,7 @@ async fn prepare_repo_and_rev<'a>( let head_rev = get_head_rev(&shadow).await?; Ok((Cow::Owned(shadow.to_string_lossy().into_owned()), head_rev)) } else { - Ok((Cow::Borrowed(repo), head_rev)) + Ok((repo, head_rev)) } } From 9ede9385cde652c0a23425df8ab30cdac772b2e6 Mon Sep 17 00:00:00 2001 From: Jo <10510431+j178@users.noreply.github.com> Date: Fri, 10 Jul 2026 04:00:55 +0800 Subject: [PATCH 2/2] Add try-repo local path regression tests --- crates/prek/src/cli/try_repo.rs | 65 +++++++++++++++++++++++---------- crates/prek/src/git.rs | 15 +------- crates/prek/src/workspace.rs | 5 ++- crates/prek/tests/try_repo.rs | 45 ++++++++++++++++++++++- crates/prek/tests/workspace.rs | 57 ++++++++++++++++------------- 5 files changed, 125 insertions(+), 62 deletions(-) diff --git a/crates/prek/src/cli/try_repo.rs b/crates/prek/src/cli/try_repo.rs index f39712058..e1ca20b2f 100644 --- a/crates/prek/src/cli/try_repo.rs +++ b/crates/prek/src/cli/try_repo.rs @@ -11,7 +11,6 @@ use toml_edit::{Array, ArrayOfTables, DocumentMut, InlineTable, Item, Value}; use crate::cli::run::Selectors; use crate::cli::{ExitStatus, RunOptions, flag}; use crate::config::{self, Stage}; -use crate::fs::PathClean; use crate::git; use crate::git::GIT_ROOT; use crate::printer::Printer; @@ -91,30 +90,37 @@ async fn clone_and_commit(repo_path: &Path, head_rev: &str, tmp_dir: &Path) -> R Ok(shadow) } -async fn prepare_repo_and_rev<'a>( +struct PreparedRepo<'a> { + runtime_source: Cow<'a, str>, + display_source: Option<&'a str>, + rev: String, +} + +async fn prepare_repo<'a>( repo: &'a str, - rev: Option<&'a str>, - tmp_dir: &'a Path, -) -> Result<(Cow<'a, str>, String)> { + rev: Option<&str>, + tmp_dir: &Path, +) -> Result> { let repo_path = Path::new(repo); let is_local = repo_path.is_dir(); - // The generated config lives in the scratch directory, where relative repo paths would be - // resolved against the wrong base. Preserve the CLI argument's meaning before writing it. - let repo = if is_local { + let runtime_source = if is_local { Cow::Owned( - std::path::absolute(repo_path)? - .clean() + dunce::canonicalize(repo_path)? .to_string_lossy() .into_owned(), ) } else { Cow::Borrowed(repo) }; - let repo_path = Path::new(repo.as_ref()); + let repo_path = Path::new(runtime_source.as_ref()); // If rev is provided, use it directly. if let Some(rev) = rev { - return Ok((repo, rev.to_string())); + return Ok(PreparedRepo { + runtime_source, + display_source: is_local.then_some(repo), + rev: rev.to_string(), + }); } // Get HEAD revision @@ -125,7 +131,7 @@ async fn prepare_repo_and_rev<'a>( let head_rev = git::git_cmd()? .arg("ls-remote") .arg("--exit-code") - .arg(repo) + .arg(runtime_source.as_ref()) .arg("HEAD") .output() .await? @@ -142,13 +148,21 @@ async fn prepare_repo_and_rev<'a>( warn_user!("Creating temporary repo with uncommitted changes..."); let shadow = clone_and_commit(repo_path, &head_rev, tmp_dir).await?; let head_rev = get_head_rev(&shadow).await?; - Ok((Cow::Owned(shadow.to_string_lossy().into_owned()), head_rev)) + Ok(PreparedRepo { + runtime_source: Cow::Owned(shadow.to_string_lossy().into_owned()), + display_source: None, + rev: head_rev, + }) } else { - Ok((repo, head_rev)) + Ok(PreparedRepo { + runtime_source, + display_source: is_local.then_some(repo), + rev: head_rev, + }) } } -fn render_repo_config_toml(repo_path: &str, rev: &str, hooks: Vec) -> String { +fn render_repo_config_toml(repo_path: &str, rev: &str, hooks: &[String]) -> String { let mut doc = DocumentMut::new(); let mut repo_table = toml_edit::Table::new(); repo_table["repo"] = toml_edit::value(repo_path); @@ -190,12 +204,16 @@ pub(crate) async fn try_repo( let store = Store::from_settings()?; let tmp_dir = TempDir::with_prefix_in("try-repo-", store.scratch_path())?; - let (repo_path, rev) = prepare_repo_and_rev(&repo, rev.as_deref(), tmp_dir.path()) + let prepared = prepare_repo(&repo, rev.as_deref(), tmp_dir.path()) .await .context("Failed to determine repository and revision")?; let store = Store::from_path(tmp_dir.path()).init()?; - let repo_config = config::RemoteRepo::new(repo_path.to_string(), rev.clone(), vec![]); + let repo_config = config::RemoteRepo::new( + prepared.runtime_source.to_string(), + prepared.rev.clone(), + vec![], + ); let repo_clone_path = store.clone_repo(&repo_config, None).await?; let selectors = Selectors::load(&run_args.includes, &run_args.skips, GIT_ROOT.as_ref()?)?; @@ -210,16 +228,23 @@ pub(crate) async fn try_repo( .map(|hook| hook.id) .collect::>(); - let config_str = render_repo_config_toml(&repo_path, &rev, hooks); + // The scratch config needs the resolved source, while the displayed config should preserve + // the user's path so it remains meaningful when copied into their project. + let config_str = render_repo_config_toml(&prepared.runtime_source, &prepared.rev, &hooks); let config_file = tmp_dir.path().join(PREK_TOML); fs_err::tokio::write(&config_file, &config_str).await?; + let display_config_str = match prepared.display_source { + Some(source) => Cow::Owned(render_repo_config_toml(source, &prepared.rev, &hooks)), + None => Cow::Borrowed(config_str.as_str()), + }; + writeln!( printer.stdout(), "{}", format!("Using generated `{PREK_TOML}`:").cyan().bold() )?; - writeln!(printer.stdout(), "{}", config_str.dimmed())?; + writeln!(printer.stdout(), "{}", display_config_str.dimmed())?; crate::cli::run( &store, diff --git a/crates/prek/src/git.rs b/crates/prek/src/git.rs index 49d1b5bb2..596495c90 100644 --- a/crates/prek/src/git.rs +++ b/crates/prek/src/git.rs @@ -1,4 +1,3 @@ -use std::borrow::Cow; use std::collections::HashSet; use std::path::{Path, PathBuf}; use std::process::Stdio; @@ -433,18 +432,6 @@ pub(crate) fn get_root() -> Result { } pub(crate) async fn init_repo(url: &str, path: &Path) -> Result<(), Error> { - let url = if Path::new(url).is_dir() { - // If the URL is a local path, convert it to an absolute path - Cow::Owned( - std::path::absolute(url)? - .clean() - .to_string_lossy() - .to_string(), - ) - } else { - Cow::Borrowed(url) - }; - git_cmd()? // Unset `extensions.objectFormat` if set, just follow what hash the remote uses. .arg("-c") @@ -462,7 +449,7 @@ pub(crate) async fn init_repo(url: &str, path: &Path) -> Result<(), Error> { .arg("remote") .arg("add") .arg("origin") - .arg(&*url) + .arg(url) .remove_git_envs() .check(true) .output() diff --git a/crates/prek/src/workspace.rs b/crates/prek/src/workspace.rs index 9bd4b3c5f..bea1aaa7d 100644 --- a/crates/prek/src/workspace.rs +++ b/crates/prek/src/workspace.rs @@ -281,7 +281,10 @@ impl Project { { let resolved = config_dir.join(repo_path); if resolved.is_dir() { - remote.repo = resolved.to_string_lossy().into_owned(); + remote.repo = dunce::canonicalize(resolved) + .map_err(config::Error::from)? + .to_string_lossy() + .into_owned(); } } } diff --git a/crates/prek/tests/try_repo.rs b/crates/prek/tests/try_repo.rs index 9585af091..c4f3260fe 100644 --- a/crates/prek/tests/try_repo.rs +++ b/crates/prek/tests/try_repo.rs @@ -255,7 +255,7 @@ fn try_repo_specific_rev() -> Result<()> { ("'", "\""), ]); - cmd_snapshot!(filters, context.try_repo().arg(&repo_path) + cmd_snapshot!(filters, context.try_repo().arg("../home/test-repos/try-repo-specific-rev") .arg("--ref") .arg(&initial_rev), @r#" success: true @@ -263,7 +263,7 @@ fn try_repo_specific_rev() -> Result<()> { ----- stdout ----- Using generated `prek.toml`: [[repos]] - repo = "[HOME]/test-repos/try-repo-specific-rev" + repo = "../home/test-repos/try-repo-specific-rev" rev = "[COMMIT_SHA]" hooks = [ { id = "test-hook" }, @@ -370,3 +370,44 @@ fn try_repo_relative_path() -> Result<()> { Ok(()) } + +#[test] +fn try_repo_dot_path() -> Result<()> { + let context = TestContext::new(); + let repo_path = create_hook_repo(&context, "try-repo-dot")?; + + ChildPath::new(&repo_path) + .child("test.txt") + .write_str("test")?; + git_cmd(&repo_path).arg("add").arg(".").assert().success(); + git_cmd(&repo_path) + .arg("commit") + .arg("-m") + .arg("Add test file") + .assert() + .success(); + + let mut filters = context.filters(); + filters.extend([(r"[a-f0-9]{40}", "[COMMIT_SHA]")]); + + cmd_snapshot!(filters, context.try_repo().current_dir(&repo_path).arg(".").arg("--all-files"), @r#" + success: true + exit_code: 0 + ----- stdout ----- + Using generated `prek.toml`: + [[repos]] + repo = "." + rev = "[COMMIT_SHA]" + hooks = [ + { id = "test-hook" }, + { id = "another-hook" }, + ] + + Test Hook................................................................Passed + Another Hook.............................................................Passed + + ----- stderr ----- + "#); + + Ok(()) +} diff --git a/crates/prek/tests/workspace.rs b/crates/prek/tests/workspace.rs index 04b12a34e..2624ac675 100644 --- a/crates/prek/tests/workspace.rs +++ b/crates/prek/tests/workspace.rs @@ -5,6 +5,7 @@ use assert_cmd::assert::OutputAssertExt; use assert_fs::fixture::{FileWriteStr, PathChild, PathCreateDir}; use indoc::indoc; use prek_consts::env_vars::EnvVars; +use prek_consts::{PRE_COMMIT_CONFIG_YAML, PRE_COMMIT_HOOKS_YAML}; use crate::common::{TestContext, cmd_snapshot, git_cmd}; @@ -1535,15 +1536,7 @@ fn orphan_projects() -> Result<()> { Ok(()) } -/// Test that relative repo paths in subproject configs resolve from the config -/// file's directory, not from the process's current working directory. -/// -/// Regression test for -#[test] -fn relative_repo_path_resolution() -> Result<()> { - use assert_fs::fixture::PathCreateDir; - use prek_consts::{PRE_COMMIT_CONFIG_YAML, PRE_COMMIT_HOOKS_YAML}; - +fn setup_relative_repo_path_project() -> Result { let context = TestContext::new(); context.init_project(); @@ -1553,21 +1546,6 @@ fn relative_repo_path_resolution() -> Result<()> { git_cmd(&hook_repo).args(["init"]).assert().success(); - git_cmd(&hook_repo) - .args(["config", "user.name", "Test"]) - .assert() - .success(); - - git_cmd(&hook_repo) - .args(["config", "user.email", "test@test.com"]) - .assert() - .success(); - - git_cmd(&hook_repo) - .args(["config", "core.autocrlf", "false"]) - .assert() - .success(); - hook_repo.child(PRE_COMMIT_HOOKS_YAML).write_str(indoc! {r" - id: test-hook name: Test Hook @@ -1579,7 +1557,7 @@ fn relative_repo_path_resolution() -> Result<()> { git_cmd(&hook_repo).args(["add", "."]).assert().success(); git_cmd(&hook_repo) - .args(["commit", "--no-si", "-m", "Initial commit"]) + .args(["commit", "-m", "Initial commit"]) .assert() .success(); @@ -1619,6 +1597,17 @@ fn relative_repo_path_resolution() -> Result<()> { context.git_add("."); + Ok(context) +} + +/// Test that relative repo paths in subproject configs resolve from the config +/// file's directory, not from the process's current working directory. +/// +/// Regression test for +#[test] +fn relative_repo_path_resolution() -> Result<()> { + let context = setup_relative_repo_path_project()?; + // Run from the root directory - the relative path ../hook-repo should resolve // from subproject/.pre-commit-config.yaml's location, not from CWD cmd_snapshot!(context.filters(), context.run(), @r#" @@ -1635,3 +1624,21 @@ fn relative_repo_path_resolution() -> Result<()> { Ok(()) } + +#[test] +fn relative_repo_path_resolution_with_explicit_relative_config() -> Result<()> { + let context = setup_relative_repo_path_project()?; + + cmd_snapshot!(context.filters(), context.run() + .arg("--config") + .arg("subproject/.pre-commit-config.yaml"), @r#" + success: true + exit_code: 0 + ----- stdout ----- + Test Hook................................................................Passed + + ----- stderr ----- + "#); + + Ok(()) +}