Skip to content

Commit 52e3834

Browse files
committed
setup: Add git-init guidance for non-git hook installs
Detect non-git repositories before hook setup work so `sce setup --hooks` fails with actionable remediation instead of surfacing raw git errors.
1 parent ba3b2e5 commit 52e3834

File tree

6 files changed

+260
-13
lines changed

6 files changed

+260
-13
lines changed

cli/src/app.rs

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -613,6 +613,19 @@ fn dispatch(command: &Command) -> Result<String, ClassifiedError> {
613613
.context("Failed to determine current directory")
614614
.map_err(|error| ClassifiedError::runtime(error.to_string()))?;
615615

616+
let preflight_hooks_repository = if request.install_hooks {
617+
let repository_root = request
618+
.hooks_repo_path
619+
.as_deref()
620+
.unwrap_or(current_dir.as_path());
621+
Some(
622+
services::setup::prepare_setup_hooks_repository(repository_root)
623+
.map_err(|error| ClassifiedError::runtime(error.to_string()))?,
624+
)
625+
} else {
626+
None
627+
};
628+
616629
let mut sections = Vec::new();
617630

618631
if let Some(mode) = request.config_mode {
@@ -636,10 +649,9 @@ fn dispatch(command: &Command) -> Result<String, ClassifiedError> {
636649
}
637650

638651
if request.install_hooks {
639-
let repository_root = request
640-
.hooks_repo_path
652+
let repository_root = preflight_hooks_repository
641653
.as_deref()
642-
.unwrap_or(current_dir.as_path());
654+
.expect("hook repository preflight should exist when install_hooks is true");
643655
let hooks_message = services::setup::run_setup_hooks(repository_root)
644656
.map_err(|error| ClassifiedError::runtime(error.to_string()))?;
645657
sections.push(hooks_message);

cli/src/services/setup.rs

Lines changed: 41 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -181,12 +181,16 @@ pub fn run_setup_for_mode(repository_root: &Path, mode: SetupMode) -> Result<Str
181181
}
182182

183183
pub fn run_setup_hooks(repository_root: &Path) -> Result<String> {
184-
let normalized_repository_root = normalize_user_repository_path(repository_root)?;
185-
let outcome = install_required_git_hooks(&normalized_repository_root)
184+
let outcome = install_required_git_hooks(repository_root)
186185
.context("Hook setup failed while installing required git hooks")?;
187186
Ok(format_required_hook_install_success_message(&outcome))
188187
}
189188

189+
pub fn prepare_setup_hooks_repository(repository_root: &Path) -> Result<PathBuf> {
190+
let normalized_repository_root = normalize_user_repository_path(repository_root)?;
191+
resolve_git_repository_root(&normalized_repository_root)
192+
}
193+
190194
fn normalize_user_repository_path(repository_root: &Path) -> Result<PathBuf> {
191195
if repository_root.as_os_str().is_empty() {
192196
bail!("Option '--repo' must not be empty. Try: pass a path to an existing git repository.");
@@ -325,17 +329,31 @@ pub struct RequiredHooksInstallOutcome {
325329
}
326330

327331
pub fn install_required_git_hooks(repository_root: &Path) -> Result<RequiredHooksInstallOutcome> {
328-
install_required_git_hooks_with_rename(repository_root, |from, to| fs::rename(from, to))
332+
let resolved_repository_root = prepare_setup_hooks_repository(repository_root)?;
333+
install_required_git_hooks_in_resolved_repository(&resolved_repository_root, |from, to| {
334+
fs::rename(from, to)
335+
})
329336
}
330337

338+
#[cfg_attr(not(test), allow(dead_code))]
331339
fn install_required_git_hooks_with_rename<F>(
332340
repository_root: &Path,
333341
mut rename_fn: F,
334342
) -> Result<RequiredHooksInstallOutcome>
335343
where
336344
F: FnMut(&Path, &Path) -> io::Result<()>,
337345
{
338-
let resolved_repository_root = resolve_git_repository_root(repository_root)?;
346+
let resolved_repository_root = prepare_setup_hooks_repository(repository_root)?;
347+
install_required_git_hooks_in_resolved_repository(&resolved_repository_root, &mut rename_fn)
348+
}
349+
350+
fn install_required_git_hooks_in_resolved_repository<F>(
351+
resolved_repository_root: &Path,
352+
mut rename_fn: F,
353+
) -> Result<RequiredHooksInstallOutcome>
354+
where
355+
F: FnMut(&Path, &Path) -> io::Result<()>,
356+
{
339357
ensure_directory_is_writable(&resolved_repository_root, "repository root")?;
340358
let hooks_directory = resolve_git_hooks_directory(&resolved_repository_root)?;
341359
fs::create_dir_all(&hooks_directory).with_context(|| {
@@ -354,7 +372,7 @@ where
354372
}
355373

356374
Ok(RequiredHooksInstallOutcome {
357-
repository_root: resolved_repository_root,
375+
repository_root: resolved_repository_root.to_path_buf(),
358376
hooks_directory,
359377
hook_results,
360378
})
@@ -515,10 +533,27 @@ fn resolve_git_repository_root(repository_root: &Path) -> Result<PathBuf> {
515533
repository_root,
516534
&["rev-parse", "--show-toplevel"],
517535
"Failed to resolve repository root. Ensure '--repo' points to an accessible git repository.",
518-
)?;
536+
)
537+
.map_err(|error| map_setup_non_git_repository_error(repository_root, error))?;
519538
Ok(PathBuf::from(repository_root_output))
520539
}
521540

541+
fn map_setup_non_git_repository_error(
542+
repository_root: &Path,
543+
error: anyhow::Error,
544+
) -> anyhow::Error {
545+
let message = error.to_string();
546+
if message.contains("not a git repository") {
547+
anyhow::anyhow!(
548+
"Directory '{}' is not a git repository. Try: run 'git init' in '{}', then rerun 'sce setup'.",
549+
repository_root.display(),
550+
repository_root.display()
551+
)
552+
} else {
553+
error
554+
}
555+
}
556+
522557
fn resolve_git_hooks_directory(repository_root: &Path) -> Result<PathBuf> {
523558
let hooks_directory_output = run_git_command_in_directory(
524559
repository_root,

cli/src/services/setup/tests.rs

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -115,9 +115,8 @@ fn run_setup_hooks_reports_per_hook_statuses() -> Result<()> {
115115
fn run_setup_hooks_rejects_missing_repo_path() {
116116
let missing_path = PathBuf::from("/definitely/missing/sce-test-repo");
117117
let error = run_setup_hooks(&missing_path).expect_err("missing repo path should fail");
118-
assert!(error
119-
.to_string()
120-
.contains("Failed to resolve repository path"));
118+
let message = format!("{error:#}");
119+
assert!(message.contains("Failed to resolve repository path"));
121120
}
122121

123122
#[test]
@@ -127,7 +126,21 @@ fn run_setup_hooks_rejects_file_repo_path() -> Result<()> {
127126
fs::write(&file_path, b"not a repo")?;
128127

129128
let error = run_setup_hooks(&file_path).expect_err("file path should fail");
130-
assert!(error.to_string().contains("is not a directory"));
129+
let message = format!("{error:#}");
130+
assert!(message.contains("is not a directory"));
131+
132+
Ok(())
133+
}
134+
135+
#[test]
136+
fn run_setup_hooks_rejects_non_git_directory_with_git_init_guidance() -> Result<()> {
137+
let temp = TestTempDir::new("sce-setup-hook-install-tests")?;
138+
139+
let error = run_setup_hooks(temp.path()).expect_err("non-git directory should fail");
140+
let message = format!("{error:#}");
141+
assert!(message.contains("is not a git repository"));
142+
assert!(message.contains("git init"));
143+
assert!(message.contains("rerun 'sce setup'"));
131144

132145
Ok(())
133146
}

cli/tests/setup_integration.rs

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -365,6 +365,77 @@ fn setup_fail_repo_missing() -> TestResult<()> {
365365
Ok(())
366366
}
367367

368+
#[test]
369+
fn setup_fail_hooks_in_non_git_current_directory() -> TestResult<()> {
370+
let harness = SetupIntegrationHarness::new("sce-setup-failure-contracts")?;
371+
372+
let result = harness.run_sce(["setup", "--hooks"])?;
373+
374+
assert_eq!(
375+
result.status.code(),
376+
Some(4),
377+
"setup --hooks in a non-git directory should exit with runtime_failure code 4\nstdout:\n{}\nstderr:\n{}",
378+
result.stdout,
379+
result.stderr
380+
);
381+
382+
assert!(
383+
result.stderr.contains("is not a git repository"),
384+
"stderr should explain the current directory is not a git repository\nstderr:\n{}",
385+
result.stderr
386+
);
387+
assert!(
388+
result.stderr.contains("git init"),
389+
"stderr should include git init guidance\nstderr:\n{}",
390+
result.stderr
391+
);
392+
assert!(
393+
result.stderr.contains("rerun 'sce setup'"),
394+
"stderr should tell the user to rerun sce setup\nstderr:\n{}",
395+
result.stderr
396+
);
397+
398+
Ok(())
399+
}
400+
401+
#[test]
402+
fn setup_fail_hooks_repo_flag_in_non_git_directory() -> TestResult<()> {
403+
let harness = SetupIntegrationHarness::new("sce-setup-failure-contracts")?;
404+
405+
let result = harness.run_sce([
406+
"setup",
407+
"--hooks",
408+
"--repo",
409+
harness.repo_root().to_str().unwrap(),
410+
])?;
411+
412+
assert_eq!(
413+
result.status.code(),
414+
Some(4),
415+
"setup --hooks --repo <non-git-dir> should exit with runtime_failure code 4\nstdout:\n{}\nstderr:\n{}",
416+
result.stdout,
417+
result.stderr
418+
);
419+
420+
assert!(
421+
result.stderr.contains("is not a git repository"),
422+
"stderr should explain the supplied path is not a git repository\nstderr:\n{}",
423+
result.stderr
424+
);
425+
assert!(
426+
result.stderr.contains("git init"),
427+
"stderr should include git init guidance\nstderr:\n{}",
428+
result.stderr
429+
);
430+
assert!(
431+
result.stderr.contains("rerun 'sce setup'"),
432+
"stderr should tell the user to rerun sce setup\nstderr:\n{}",
433+
result.stderr
434+
);
435+
436+
Ok(())
437+
}
438+
368439
#[test]
369440
fn setup_fail_repo_without_hooks() -> TestResult<()> {
370441
let harness = SetupIntegrationHarness::new("sce-setup-failure-contracts")?;

context/plans/sce-cli-ux-runtime-fixes.md

Lines changed: 115 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

context/sce/setup-githooks-cli-ux.md

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)