-
Notifications
You must be signed in to change notification settings - Fork 0
feat(scripts): add submodule-init --advance for pin bumps #2447
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weβll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
f9b4979
21f2f07
69fc65b
0085257
44aed62
beb2980
3a1db71
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -371,7 +371,65 @@ report "empty-init: the failure NAMES the empty submodule (fails at init, not la | |
| out="$(cd "$c11/super" && "$helper" --check 2>&1)" && rc=0 || rc=$? | ||
| report "empty-init: --check still SKIPS a legitimately deinitialised submodule" \ | ||
| "$([[ $rc -eq 0 ]] && echo yes || echo no)" "rc=$rc $out" | ||
| # 12. --advance: move a populated checkout to a newer recorded pin WITHOUT | ||
| # `git submodule update` (which rewrites shared core.worktree). Hermetic | ||
| # fixture: bump the gitlink in the index while leaving the working tree on | ||
| # the old SHA, then advance and assert HEAD + isolation. | ||
| c12="$tmp/c12" | ||
| mk_super "$c12" | ||
| ( | ||
| cd "$c12/remote-sub" | ||
| echo next >file.txt | ||
| git add file.txt | ||
| git commit -q -m next | ||
| ) | ||
| new_sha="$(git -C "$c12/remote-sub" rev-parse HEAD)" | ||
| old_sha="$(git -C "$c12/super/sub" rev-parse HEAD)" | ||
| ( | ||
| cd "$c12/super" | ||
| # Record the new pin in the superproject without moving the working tree. | ||
| git update-index --cacheinfo "160000,$new_sha,sub" | ||
| git commit -q -m "bump sub" | ||
| ) | ||
| report "advance fixture: working tree still on old pin before --advance" \ | ||
| "$([[ "$(git -C "$c12/super/sub" rev-parse HEAD)" == "$old_sha" ]] && echo yes || echo no)" | ||
| # Make the new object reachable in the submodule (file:// remote). | ||
| git -C "$c12/super/sub" fetch -q origin | ||
| out="$(cd "$c12/super" && "$helper" --advance sub 2>&1)" && rc=0 || rc=$? | ||
| report "advance: exits 0" "$([[ $rc -eq 0 ]] && echo yes || echo no)" "$out" | ||
| report "advance: checkout moved to the recorded pin" \ | ||
| "$([[ "$(git -C "$c12/super/sub" rev-parse HEAD)" == "$new_sha" ]] && echo yes || echo no)" | ||
| report "advance: does not leave a shared core.worktree" \ | ||
| "$([[ -z "$(git config -f "$c12/super/.git/modules/sub/config" core.worktree 2>/dev/null || true)" ]] && echo yes || echo no)" | ||
| out="$(cd "$c12/super" && "$helper" --check 2>&1)" && rc=0 || rc=$? | ||
| report "advance: --check passes afterwards" "$([[ $rc -eq 0 ]] && echo yes || echo no)" "$out" | ||
|
Comment on lines
+396
to
+405
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. π Maintainability & Code Quality | π Major | β‘ Quick win Exercise the script-side target fetch. Lines 396-397 make Remove the fixture-side fetch. Assert that the target object is initially absent, then verify that π€ Prompt for AI AgentsSource: Learnings |
||
|
|
||
| # 13. --advance refuses a dirty working tree. | ||
| c13="$tmp/c13" | ||
| mk_super "$c13" | ||
| echo dirty >>"$c13/super/sub/file.txt" | ||
| out="$(cd "$c13/super" && "$helper" --advance sub 2>&1)" && rc=0 || rc=$? | ||
| report "advance dirty: exits non-zero" "$([[ $rc -ne 0 ]] && echo yes || echo no)" "$out" | ||
| report "advance dirty: names the dirty-tree refusal" \ | ||
| "$(grep -q 'dirty working tree' <<<"$out" && echo yes || echo no)" "$out" | ||
|
|
||
| # 14. --advance refuses a checkout that is ahead of the recorded pin. | ||
| c14="$tmp/c14" | ||
| mk_super "$c14" | ||
| pin="$(git -C "$c14/super/sub" rev-parse HEAD)" | ||
| ( | ||
| cd "$c14/super/sub" | ||
| echo local >extra.txt | ||
| git add extra.txt | ||
| git commit -q -m local-ahead | ||
| ) | ||
| # Superproject gitlink still points at the old pin; checkout is one commit ahead. | ||
| report "advance ahead fixture: gitlink still at old pin" \ | ||
| "$([[ "$(git -C "$c14/super" rev-parse HEAD:sub)" == "$pin" ]] && echo yes || echo no)" | ||
| out="$(cd "$c14/super" && "$helper" --advance sub 2>&1)" && rc=0 || rc=$? | ||
| report "advance ahead: exits non-zero" "$([[ $rc -ne 0 ]] && echo yes || echo no)" "$out" | ||
| report "advance ahead: names the ahead-of-pin refusal" \ | ||
| "$(grep -q 'ahead of the recorded pin' <<<"$out" && echo yes || echo no)" "$out" | ||
| if [[ $fail -ne 0 ]]; then | ||
| echo "submodule-init self-test: FAILURES above" >&2 | ||
| exit 1 | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -156,6 +156,27 @@ not isolated. The probe is non-destructive β it never modifies submodule conte | |
| other sessions' worktrees β but not strictly read-only: it adds and removes a throwaway probe | ||
| worktree to catch a dangling `core.worktree` a config read alone would miss. | ||
|
|
||
| ### Advancing a populated submodule to a new pin (`--advance`) | ||
|
|
||
| After a pin-bump PR merges and you `git pull` the superproject, an already-populated submodule | ||
| checkout stays on the **old** commit. Re-running `submodule-init.sh <path>` will not move it | ||
| (populated trees are repair-only). Plain `git submodule update -- <path>` *would* move it, but it | ||
| is the same family of command that writes shared `core.worktree` β do not use it here. | ||
|
|
||
| **Verified procedure** (hermetic fixture in `submodule-init.test.sh`, cases 6β8): | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. π Maintainability & Code Quality | π‘ Minor | β‘ Quick win Correct the test-case reference. The π€ Prompt for AI Agents |
||
|
|
||
| ```sh | ||
| # From the superproject, after pulling the pin bump. Refuses dirty / ahead-of-pin checkouts. | ||
| .claude/scripts/submodule-init.sh --advance <path> | ||
| .claude/scripts/submodule-init.sh --check # must pass afterwards | ||
| ``` | ||
|
|
||
| `--advance` reads the gitlink at `HEAD:<path>`, checks out that commit **directly** inside the | ||
| submodule (no `git submodule update`), then runs the usual repair + fail-closed probe. It refuses | ||
| when the working tree is dirty or when `HEAD` has commits not reachable from the new pin, so it | ||
| cannot discard uncommitted or unpushed work. Prefer this guarded flag over documenting the | ||
| hazardous plain-Git command. | ||
|
|
||
| What made it dangerous is that it fails **silently**: a `git worktree add` still succeeds, and the | ||
| worktree looks real. Three live linked worktrees β including **two belonging to the parallel sibling | ||
| agent** β were all resolving into the *shared main checkout*, i.e. actively colliding, with nothing | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ποΈ Data Integrity & Integration | π΄ Critical | β‘ Quick win
Repair isolation before Git uses the populated checkout.
Lines 304-341 run
git -C "$path"beforerepair "$path". If a stale sharedcore.worktreealready redirects this submodule,git checkout --detach "$target"can update another sessionβs worktree before this command repairs the configuration.Run
repairandprobeimmediately after the populated-checkout check. Do this beforestatus,rev-parse, orcheckout. Add a fixture with a pre-existing stalecore.worktreeand verify that--advancedoes not modify the redirected worktree.Proposed fix
is_populated "$path" || die "'$path' is not checked out here β run submodule-init.sh $path to populate it first" + repair "$path" + probe "$path" || die "repair did not restore isolation for '$path' β do not edit it" + if [ -n "$(git -C "$path" status --porcelain 2>/dev/null)" ]; thenπ Committable suggestion
π€ Prompt for AI Agents