Skip to content

gc --remove: remove copies with a submodule and copies with status noise only - #28

Merged
github-actions[bot] merged 3 commits into
mainfrom
improve/F-2-gc-remove-dpcli-head
Nov 1, 2026
Merged

github-actions[bot] merged 3 commits into
mainfrom
improve/F-2-gc-remove-dpcli-head

Conversation

@zolotarev

@zolotarev zolotarev commented Nov 1, 2026 •

Copy link
Copy Markdown
Owner

Friction — after a module was merged, gc --remove left every copy in place:

  • a copy with a populated submodule: git worktree remove refuses it without --force (working trees containing submodules cannot be moved or removed);
  • a copy whose status lists modified files while their content equals HEAD (a pre-commit hook stashed and restored them): reported as dirty copy.

Seen once in one project: 5 of 5 copies of a merged module were skipped; removed by hand.

Change (remove_copy)

  • tracked changes block removal only if git diff --quiet HEAD reports a difference (any non-zero exit, incl. errors, counts as dirty);
  • new submodule check: git status --porcelain and commits on no remote (rev-list --branches HEAD --not --remotes) in every initialized submodule block removal — the submodule repository is deleted with the copy; an uninitialized submodule has no checkout, so no work to lose;
  • after the checks, worktree remove --force always.

Review rework

  • test: a submodule commit recorded in the copy's branch but pushed nowhere blocks removal;
  • title narrowed to the case tested (staged vs worktree restored to HEAD); mode/CRLF-only changes are real diffs and still block;
  • tools/metrics.py --check passes (force 22); full suite 209 OK; VERSION 36, CHANGELOG line.

Sent by dpcli contribute. Anonymized: no project names, paths or quotes.

@zolotarev zolotarev added the feedback Friction reported from real use label Nov 1, 2026
@github-actions

github-actions Bot commented Nov 1, 2026

Copy link
Copy Markdown

VERDICT: rework

  • Data loss risk: lib/dpcli/commands/branches.py:~199 now always runs worktree remove --force. A copy's submodule git dir lives inside the worktree. A commit made in the submodule and recorded in the copy's branch passes both new checks (diff HEAD is clean, the submodule has no dirty files), and --force then deletes the only copy of that commit. Check that each submodule HEAD is reachable from a ref or remote outside the copy, or refuse. Add a test for it.
  • Friction evidence is thin: "Seen ×0 in 0 task(s)" and no open feedback issue. The body names a project ("mobile") and is in Russian, which breaks the English-only and anonymized rules. Rewrite the title and body in English with no project names.
  • git diff --quiet HEAD treats any non-zero exit (including git errors, 128) as dirty. That is the safe direction, but the "status noise" case is only covered for staged versus worktree. Add a test for the other noise sources the title mentions (formatter, mode or CRLF-only changes), or narrow the claim.
  • git submodule foreach --recursive "git status --porcelain" runs a shell string in every submodule. That is acceptable here since it is a constant, but it adds a process per submodule. Check it still works when a submodule is uninitialized (it is skipped, so any work there is invisible).
  • tools/budgets.json risky force counter: the new comment text mentions --force. Confirm the budget check still passes.
  • Process is otherwise fine: VERSION 36 bumped and CHANGELOG line added.

@github-actions github-actions Bot added the rework label Nov 1, 2026
@zolotarev

Copy link
Copy Markdown
Owner Author

+1 from another project: ×0 in 0 task(s).

Sent by dpcli contribute. Anonymized: no project names, paths or quotes.

@zolotarev zolotarev changed the title gc --remove: копии с сабмодулем .dpcli не удаляются, копии «грязные» от форматтера без отличий от HEAD gc --remove: remove copies with a submodule and copies with status noise only Nov 1, 2026
@github-actions

github-actions Bot commented Nov 1, 2026

Copy link
Copy Markdown

VERDICT: rework

  • Real friction (feedback label, reproduced in PR body), VERSION 36 + CHANGELOG present, change is small, tests added for the status-noise and submodule cases.
  • Fail-open: lib/dpcli/commands/branches.py:192 ignores the returncode of submodule foreach. If it fails (git error, a submodule whose status/rev-list errors), stdout is empty, the check passes, and worktree remove --force deletes the submodule repository with possibly unpushed work. Treat a non-zero exit as blocking, like the diff --quiet check does (e.g. return "cannot check submodules: ").
  • Add a test for that path (e.g. a submodule with a broken checkout) so the fail-closed behavior is pinned.
  • Minor: stashes in a submodule are not checked and are lost with --force; either check git stash list in the foreach or note the limit in the CHANGELOG.

@zolotarev

Copy link
Copy Markdown
Owner Author

Rework adeb6ce: submodule foreach non-zero exit now blocks removal (cannot check submodules: …), pinned by a test with a broken submodule gitdir; git stash list added to the check, with a test. Full suite 209 OK, tools/metrics.py --check passes.

@github-actions

github-actions Bot commented Nov 1, 2026

Copy link
Copy Markdown

VERDICT: accept

  • Real friction (feedback label, 5 of 5 copies skipped); the change is small and confined to remove_copy (branches.py:184).
  • Safety holds: tracked changes are judged by git diff --quiet HEAD (non-zero or error means dirty), and untracked files still block as before. Submodule status, stash and commits on no remote block removal. A failed submodule check blocks too. --force runs only after those checks.
  • Tests cover the status-noise case, a submodule with a work file, a stash, an unpushed submodule commit, a failing check, and the clean removal (tests/test_fixes.py:214+).
  • VERSION is 36 with a CHANGELOG line. English only, anonymized, no new dependencies.
  • Nit: the # checked above; --force comment is longer than it needs to be. Not blocking.
  • I did not run the test suite myself. I'm relying on the stated 209 OK.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feedback Friction reported from real use

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant