Skip to content

Commit 2548800

Browse files
aclark4lifeCopilot
andcommitted
spec: clarify idempotency of sync vs patch apply
dbx spec sync: idempotent (always deletes+recopies test files) dbx spec patch apply: NOT idempotent (git apply -R fails if already applied) dbx spec sync --apply-patches: idempotent (sync resets first, then patches apply) Changes: - _apply_patches(): add dry-run check (git apply -R --check) before applying; if it fails, emit a clear error pointing to --apply-patches instead of a raw 'patch does not apply' git failure - spec status verify section: when stale specs have patches, recommend '--apply-patches' as the idempotent one-shot command with a note explaining why it's always safe to re-run - Patch annotation legend updated: 'use --apply-patches to sync + patch in one shot' Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 56d2451 commit 2548800

1 file changed

Lines changed: 50 additions & 5 deletions

File tree

src/dbx_python_cli/commands/spec.py

Lines changed: 50 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -126,13 +126,37 @@ def _show_patch_summary(driver_repo, verbose: bool = False) -> int:
126126

127127

128128
def _apply_patches(driver_repo, verbose: bool = False) -> bool:
129-
"""Run git apply -R on all patch files. Returns True on success."""
129+
"""Run git apply -R on all patch files. Returns True on success.
130+
131+
Checks applicability first and gives a clear error if patches appear to be
132+
already applied (i.e. the test files have already been modified).
133+
"""
130134
patch_dir = _get_patch_dir(driver_repo)
131135
patches = _list_patches(patch_dir)
132136
if not patches:
133137
typer.echo(" No patches to apply.")
134138
return True
135139

140+
# Dry-run first so we can give a helpful error instead of a raw git failure
141+
check_result = subprocess.run(
142+
["git", "apply", "-R", "--check", "--allow-empty", *[str(p) for p in patches]],
143+
cwd=str(driver_repo["path"]),
144+
check=False,
145+
capture_output=True,
146+
text=True,
147+
)
148+
if check_result.returncode != 0:
149+
typer.echo(
150+
"❌ Patches cannot be applied — they may already be applied.", err=True
151+
)
152+
typer.echo(
153+
" Run 'dbx spec sync <spec> --apply-patches' to reset and re-apply in one shot.",
154+
err=True,
155+
)
156+
if verbose:
157+
typer.echo(check_result.stderr.strip(), err=True)
158+
return False
159+
136160
cmd = [
137161
"git",
138162
"apply",
@@ -659,7 +683,7 @@ def _patches_for_spec(spec_name: str) -> list[str]:
659683
typer.echo(f"{prefix} dbx spec sync {name}{reason_str}{patch_str}")
660684
if any(_patches_for_spec(n) for n, _ in stale):
661685
typer.echo(
662-
"\n 🩹 = has an active patch — re-run 'dbx spec patch apply' after syncing"
686+
"\n 🩹 = has an active patch — use --apply-patches to sync + patch in one shot"
663687
)
664688
else:
665689
typer.echo(" ✅ All checked specs are up to date.")
@@ -681,12 +705,33 @@ def _patches_for_spec(spec_name: str) -> list[str]:
681705
step = 1
682706
if stale:
683707
spec_names = " ".join(name for name, _ in stale)
684-
typer.echo(f" {step}. Sync remaining specs:")
685-
typer.echo(f" dbx spec sync {spec_names}")
708+
any_patched = any(_patches_for_spec(n) for n, _ in stale)
709+
if any_patched and patch_count:
710+
# --apply-patches is idempotent: sync resets files, then patches apply fresh
711+
typer.echo(
712+
f" {step}. Sync and re-apply patches in one shot (recommended):"
713+
)
714+
typer.echo(f" dbx spec sync {spec_names} --apply-patches")
715+
typer.echo(
716+
" # equivalent to sync + 'dbx spec patch apply', but always safe to re-run"
717+
)
718+
else:
719+
typer.echo(f" {step}. Sync remaining specs:")
720+
typer.echo(f" dbx spec sync {spec_names}")
686721
step += 1
687-
if patch_count:
722+
if patch_count and not any_patched:
723+
typer.echo(f" {step}. Re-apply patches (removes unimplemented tests):")
724+
typer.echo(" dbx spec patch apply")
725+
typer.echo(
726+
" # ⚠ not idempotent — run sync first if patches are already applied"
727+
)
728+
step += 1
729+
elif patch_count:
688730
typer.echo(f" {step}. Re-apply patches (removes unimplemented tests):")
689731
typer.echo(" dbx spec patch apply")
732+
typer.echo(
733+
" # ⚠ not idempotent — run sync first if patches are already applied"
734+
)
690735
step += 1
691736
if branch_dirs:
692737
test_dirs = " ".join(f"test/{d}/" for d in branch_dirs[:5])

0 commit comments

Comments
 (0)