Skip to content

Commit cbddd43

Browse files
setup: Skip backup creation in git-backed repositories
Add SetupBackupPolicy enum with CreateAndRestoreBackups and GitBackedRepository variants. When installing config assets or required git hooks in a git-backed repository, skip backup creation and emit git-recovery guidance on failure instead of rollback-based recovery. This avoids unnecessary backup clutter in version-controlled projects while preserving safety for non-git contexts. Co-authored-by: SCE <sce@crocoder.dev>
1 parent 956982f commit cbddd43

14 files changed

Lines changed: 737 additions & 59 deletions

cli/src/services/auth_command.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ pub fn run_logout(format: AuthFormat) -> Result<String> {
101101
let guidance = auth_state_path_guidance(
102102
"verify file permissions for the auth state directory and rerun 'sce auth logout'",
103103
);
104-
anyhow!(format!("{} Try: {}", error, guidance))
104+
anyhow!(format!("{error} Try: {guidance}"))
105105
})?;
106106
render_logout_result(deleted, format)
107107
}

cli/src/services/setup.rs

Lines changed: 231 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,12 @@ pub enum SetupMode {
108108
NonInteractive(SetupTarget),
109109
}
110110

111+
#[derive(Clone, Copy, Debug, Eq, PartialEq)]
112+
enum SetupBackupPolicy {
113+
CreateAndRestoreBackups,
114+
GitBackedRepository,
115+
}
116+
111117
#[derive(Clone, Copy, Debug, Eq, PartialEq)]
112118
pub enum SetupDispatch {
113119
Proceed(SetupMode),
@@ -274,6 +280,11 @@ fn format_setup_install_success_message(outcome: &SetupInstallOutcome) -> String
274280
backup_root.display()
275281
))
276282
)),
283+
None if result.skipped_backup_in_git_backed_repo => lines.push(format!(
284+
" {}: {}",
285+
label("backup:"),
286+
value("not created (git-backed repository)")
287+
)),
277288
None => lines.push(format!(
278289
" {}: {}",
279290
label("backup:"),
@@ -322,7 +333,22 @@ fn format_required_hook_install_success_message(outcome: &RequiredHooksInstallOu
322333
label("backup:"),
323334
value(&format!("'{}'", backup_path.display()))
324335
)),
325-
None => lines.push(format!(" {}: {}", label("backup:"), value("not needed"))),
336+
None if result.skipped_backup_in_git_backed_repo => lines.push(format!(
337+
" {}: {}",
338+
label("backup:"),
339+
value("not created (git-backed repository)")
340+
)),
341+
None => lines.push(format!(
342+
" {}: {}",
343+
label("backup:"),
344+
value(match result.status {
345+
RequiredHookInstallStatus::Installed => "not needed (no existing hook)",
346+
RequiredHookInstallStatus::Skipped => {
347+
"not needed (hook already matched canonical state)"
348+
}
349+
RequiredHookInstallStatus::Updated => "not needed",
350+
})
351+
)),
326352
}
327353
}
328354

@@ -350,6 +376,7 @@ pub struct SetupInstallTargetResult {
350376
pub target: SetupTarget,
351377
pub destination_root: PathBuf,
352378
pub backup_root: Option<PathBuf>,
379+
pub skipped_backup_in_git_backed_repo: bool,
353380
pub installed_file_count: usize,
354381
}
355382

@@ -371,6 +398,7 @@ pub struct RequiredHookInstallResult {
371398
pub hook_path: PathBuf,
372399
pub status: RequiredHookInstallStatus,
373400
pub backup_path: Option<PathBuf>,
401+
pub skipped_backup_in_git_backed_repo: bool,
374402
}
375403

376404
#[derive(Clone, Debug, Eq, PartialEq)]
@@ -382,9 +410,12 @@ pub struct RequiredHooksInstallOutcome {
382410

383411
pub fn install_required_git_hooks(repository_root: &Path) -> Result<RequiredHooksInstallOutcome> {
384412
let resolved_repository_root = prepare_setup_hooks_repository(repository_root)?;
385-
install_required_git_hooks_in_resolved_repository(&resolved_repository_root, |from, to| {
386-
fs::rename(from, to)
387-
})
413+
let backup_policy = resolve_setup_backup_policy(&resolved_repository_root);
414+
install_required_git_hooks_in_resolved_repository(
415+
&resolved_repository_root,
416+
backup_policy,
417+
|from, to| fs::rename(from, to),
418+
)
388419
}
389420

390421
#[allow(dead_code)]
@@ -396,11 +427,17 @@ where
396427
F: FnMut(&Path, &Path) -> io::Result<()>,
397428
{
398429
let resolved_repository_root = prepare_setup_hooks_repository(repository_root)?;
399-
install_required_git_hooks_in_resolved_repository(&resolved_repository_root, &mut rename_fn)
430+
let backup_policy = resolve_setup_backup_policy(&resolved_repository_root);
431+
install_required_git_hooks_in_resolved_repository(
432+
&resolved_repository_root,
433+
backup_policy,
434+
&mut rename_fn,
435+
)
400436
}
401437

402438
fn install_required_git_hooks_in_resolved_repository<F>(
403439
resolved_repository_root: &Path,
440+
backup_policy: SetupBackupPolicy,
404441
mut rename_fn: F,
405442
) -> Result<RequiredHooksInstallOutcome>
406443
where
@@ -418,8 +455,12 @@ where
418455

419456
let mut hook_results = Vec::new();
420457
for hook_asset in iter_required_hook_assets() {
421-
let hook_result =
422-
install_single_required_hook_with_rename(&hooks_directory, hook_asset, &mut rename_fn)?;
458+
let hook_result = install_single_required_hook_with_rename(
459+
&hooks_directory,
460+
hook_asset,
461+
backup_policy,
462+
&mut rename_fn,
463+
)?;
423464
hook_results.push(hook_result);
424465
}
425466

@@ -433,6 +474,7 @@ where
433474
fn install_single_required_hook_with_rename<F>(
434475
hooks_directory: &Path,
435476
hook_asset: &EmbeddedAsset,
477+
backup_policy: SetupBackupPolicy,
436478
rename_fn: &mut F,
437479
) -> Result<RequiredHookInstallResult>
438480
where
@@ -457,6 +499,7 @@ where
457499
hook_path,
458500
status: RequiredHookInstallStatus::Skipped,
459501
backup_path: None,
502+
skipped_backup_in_git_backed_repo: false,
460503
});
461504
}
462505
} else if existing_metadata.is_some() {
@@ -489,9 +532,19 @@ where
489532
hook_path,
490533
status: RequiredHookInstallStatus::Installed,
491534
backup_path: None,
535+
skipped_backup_in_git_backed_repo: false,
492536
});
493537
}
494538

539+
if backup_policy == SetupBackupPolicy::GitBackedRepository {
540+
return update_git_backed_required_hook_with_rename(
541+
hook_asset,
542+
hook_path,
543+
&hook_staging_path,
544+
rename_fn,
545+
);
546+
}
547+
495548
let backup_path = next_backup_path(&hook_path)?;
496549
rename_fn(&hook_path, &backup_path).with_context(|| {
497550
format!(
@@ -529,6 +582,43 @@ where
529582
hook_path,
530583
status: RequiredHookInstallStatus::Updated,
531584
backup_path: Some(backup_path),
585+
skipped_backup_in_git_backed_repo: false,
586+
})
587+
}
588+
589+
fn update_git_backed_required_hook_with_rename<F>(
590+
hook_asset: &EmbeddedAsset,
591+
hook_path: PathBuf,
592+
hook_staging_path: &Path,
593+
rename_fn: &mut F,
594+
) -> Result<RequiredHookInstallResult>
595+
where
596+
F: FnMut(&Path, &Path) -> io::Result<()>,
597+
{
598+
remove_existing_install_target(&hook_path).with_context(|| {
599+
format!(
600+
"Failed to replace existing hook '{}' without creating a backup",
601+
hook_path.display()
602+
)
603+
})?;
604+
605+
if let Err(error) = rename_fn(hook_staging_path, &hook_path).with_context(|| {
606+
format!(
607+
"Failed to update required hook '{}' at '{}'",
608+
hook_asset.relative_path,
609+
hook_path.display()
610+
)
611+
}) {
612+
cleanup_path_if_exists(hook_staging_path);
613+
return Err(error.context(git_backed_hook_install_recovery_guidance(&hook_path)));
614+
}
615+
616+
Ok(RequiredHookInstallResult {
617+
hook_name: hook_asset.relative_path.to_string(),
618+
hook_path,
619+
status: RequiredHookInstallStatus::Updated,
620+
backup_path: None,
621+
skipped_backup_in_git_backed_repo: true,
532622
})
533623
}
534624

@@ -701,15 +791,20 @@ pub fn install_embedded_setup_assets(
701791
repository_root: &Path,
702792
target: SetupTarget,
703793
) -> Result<SetupInstallOutcome> {
704-
install_embedded_setup_assets_with_rename(repository_root, target, |from, to| {
705-
fs::rename(from, to)
706-
})
794+
let backup_policy = resolve_setup_backup_policy(repository_root);
795+
install_embedded_setup_assets_with_rename(
796+
repository_root,
797+
target,
798+
|from, to| fs::rename(from, to),
799+
backup_policy,
800+
)
707801
}
708802

709803
fn install_embedded_setup_assets_with_rename<F>(
710804
repository_root: &Path,
711805
target: SetupTarget,
712806
mut rename_fn: F,
807+
backup_policy: SetupBackupPolicy,
713808
) -> Result<SetupInstallOutcome>
714809
where
715810
F: FnMut(&Path, &Path) -> io::Result<()>,
@@ -726,6 +821,7 @@ where
726821
repository_root,
727822
concrete_target,
728823
&assets,
824+
backup_policy,
729825
&mut rename_fn,
730826
)?;
731827
target_results.push(result);
@@ -738,6 +834,7 @@ fn install_assets_for_concrete_target_with_rename<F>(
738834
repository_root: &Path,
739835
target: SetupTarget,
740836
assets: &[&'static EmbeddedAsset],
837+
backup_policy: SetupBackupPolicy,
741838
rename_fn: &mut F,
742839
) -> Result<SetupInstallTargetResult>
743840
where
@@ -751,6 +848,16 @@ where
751848
return Err(error);
752849
}
753850

851+
if backup_policy == SetupBackupPolicy::GitBackedRepository {
852+
return install_assets_for_git_backed_target_with_rename(
853+
target,
854+
destination_root,
855+
&staging_root,
856+
assets.len(),
857+
rename_fn,
858+
);
859+
}
860+
754861
let mut backup_root = None;
755862

756863
if destination_root.exists() {
@@ -794,10 +901,98 @@ where
794901
target,
795902
destination_root,
796903
backup_root,
904+
skipped_backup_in_git_backed_repo: false,
797905
installed_file_count: assets.len(),
798906
})
799907
}
800908

909+
fn install_assets_for_git_backed_target_with_rename<F>(
910+
target: SetupTarget,
911+
destination_root: PathBuf,
912+
staging_root: &Path,
913+
installed_file_count: usize,
914+
rename_fn: &mut F,
915+
) -> Result<SetupInstallTargetResult>
916+
where
917+
F: FnMut(&Path, &Path) -> io::Result<()>,
918+
{
919+
if destination_root.exists() {
920+
remove_existing_install_target(&destination_root).with_context(|| {
921+
format!(
922+
"Failed to replace existing setup target '{}' without creating a backup",
923+
destination_root.display()
924+
)
925+
})?;
926+
}
927+
928+
if let Err(error) = rename_fn(staging_root, &destination_root).with_context(|| {
929+
format!(
930+
"Failed to swap staged install '{}' into destination '{}'",
931+
staging_root.display(),
932+
destination_root.display()
933+
)
934+
}) {
935+
cleanup_path_if_exists(staging_root);
936+
return Err(error.context(git_backed_setup_install_recovery_guidance(
937+
target,
938+
&destination_root,
939+
)));
940+
}
941+
942+
Ok(SetupInstallTargetResult {
943+
target,
944+
destination_root,
945+
backup_root: None,
946+
skipped_backup_in_git_backed_repo: true,
947+
installed_file_count,
948+
})
949+
}
950+
951+
fn remove_existing_install_target(destination_root: &Path) -> Result<()> {
952+
let metadata = fs::metadata(destination_root).with_context(|| {
953+
format!(
954+
"Failed to inspect existing setup target '{}'",
955+
destination_root.display()
956+
)
957+
})?;
958+
959+
if metadata.is_dir() {
960+
fs::remove_dir_all(destination_root).with_context(|| {
961+
format!(
962+
"Failed to remove existing setup target directory '{}'",
963+
destination_root.display()
964+
)
965+
})?;
966+
} else {
967+
fs::remove_file(destination_root).with_context(|| {
968+
format!(
969+
"Failed to remove existing setup target file '{}'",
970+
destination_root.display()
971+
)
972+
})?;
973+
}
974+
975+
Ok(())
976+
}
977+
978+
fn git_backed_setup_install_recovery_guidance(
979+
target: SetupTarget,
980+
destination_root: &Path,
981+
) -> String {
982+
format!(
983+
"Git-backed setup for {} does not create backups. Recover '{}' from git state if needed.",
984+
setup_target_label(target),
985+
destination_root.display()
986+
)
987+
}
988+
989+
fn git_backed_hook_install_recovery_guidance(hook_path: &Path) -> String {
990+
format!(
991+
"Git-backed hook setup does not create backups. Recover '{}' from git state if needed.",
992+
hook_path.display()
993+
)
994+
}
995+
801996
fn write_assets_to_staging(staging_root: &Path, assets: &[&'static EmbeddedAsset]) -> Result<()> {
802997
for asset in assets {
803998
validate_embedded_relative_path(asset.relative_path)?;
@@ -892,6 +1087,32 @@ fn next_backup_path(destination_root: &Path) -> Result<PathBuf> {
8921087
unreachable!("backup suffix iterator is unbounded")
8931088
}
8941089

1090+
fn resolve_setup_backup_policy(repository_root: &Path) -> SetupBackupPolicy {
1091+
resolve_setup_backup_policy_with_probe(repository_root, is_git_backed_repository)
1092+
}
1093+
1094+
fn resolve_setup_backup_policy_with_probe<F>(
1095+
repository_root: &Path,
1096+
git_backed_repository_probe: F,
1097+
) -> SetupBackupPolicy
1098+
where
1099+
F: FnOnce(&Path) -> bool,
1100+
{
1101+
if git_backed_repository_probe(repository_root) {
1102+
SetupBackupPolicy::GitBackedRepository
1103+
} else {
1104+
SetupBackupPolicy::CreateAndRestoreBackups
1105+
}
1106+
}
1107+
1108+
fn is_git_backed_repository(repository_root: &Path) -> bool {
1109+
Command::new("git")
1110+
.args(["rev-parse", "--show-toplevel"])
1111+
.current_dir(repository_root)
1112+
.output()
1113+
.is_ok_and(|output| output.status.success())
1114+
}
1115+
8951116
fn target_install_directory_name(target: SetupTarget) -> &'static str {
8961117
match target {
8971118
SetupTarget::OpenCode => ".opencode",

0 commit comments

Comments
 (0)