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
65 changes: 52 additions & 13 deletions crates/prek/src/cli/try_repo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,17 +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<PreparedRepo<'a>> {
let repo_path = Path::new(repo);
let is_local = repo_path.is_dir();
let runtime_source = if is_local {
Cow::Owned(
dunce::canonicalize(repo_path)?
.to_string_lossy()
.into_owned(),
)
} else {
Cow::Borrowed(repo)
};
let repo_path = Path::new(runtime_source.as_ref());

// If rev is provided, use it directly.
if let Some(rev) = rev {
return Ok((Cow::Borrowed(repo), rev.to_string()));
return Ok(PreparedRepo {
runtime_source,
display_source: is_local.then_some(repo),
rev: rev.to_string(),
});
}

// Get HEAD revision
Expand All @@ -111,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?
Expand All @@ -128,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((Cow::Borrowed(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>) -> String {
fn render_repo_config_toml(repo_path: &str, rev: &str, hooks: &[String]) -> String {
Comment thread
j178 marked this conversation as resolved.
let mut doc = DocumentMut::new();
let mut repo_table = toml_edit::Table::new();
repo_table["repo"] = toml_edit::value(repo_path);
Comment thread
j178 marked this conversation as resolved.
Expand Down Expand Up @@ -176,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()?)?;
Expand All @@ -196,16 +228,23 @@ pub(crate) async fn try_repo(
.map(|hook| hook.id)
.collect::<Vec<_>>();

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()
)?;
Comment thread
j178 marked this conversation as resolved.
writeln!(printer.stdout(), "{}", config_str.dimmed())?;
writeln!(printer.stdout(), "{}", display_config_str.dimmed())?;

crate::cli::run(
&store,
Expand Down
15 changes: 1 addition & 14 deletions crates/prek/src/git.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
use std::borrow::Cow;
use std::collections::HashSet;
use std::path::{Path, PathBuf};
use std::process::Stdio;
Expand Down Expand Up @@ -433,18 +432,6 @@ pub(crate) fn get_root() -> Result<PathBuf, Error> {
}

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")
Expand All @@ -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)
Comment thread
j178 marked this conversation as resolved.
.remove_git_envs()
.check(true)
.output()
Expand Down
5 changes: 4 additions & 1 deletion crates/prek/src/workspace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Comment thread
j178 marked this conversation as resolved.
}
}
}
Expand Down
45 changes: 43 additions & 2 deletions crates/prek/tests/try_repo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -255,15 +255,15 @@ 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#"
Comment thread
j178 marked this conversation as resolved.
success: true
exit_code: 0
----- 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" },
Expand Down Expand Up @@ -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(())
}
57 changes: 32 additions & 25 deletions crates/prek/tests/workspace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};

Expand Down Expand Up @@ -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 <https://github.com/j178/prek/issues/1065>
#[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<TestContext> {
let context = TestContext::new();
context.init_project();

Expand All @@ -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
Expand All @@ -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();

Expand Down Expand Up @@ -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 <https://github.com/j178/prek/issues/1065>
#[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#"
Expand All @@ -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(())
}
Loading