Skip to content

Patch() reports a submodule with only uncommitted changes as a "Subproject commit …-dirty" hunk that Apply().ToIndex() stages with exit 0 while changing nothing #173

Description

@matt-edmondson

What's wrong

#124 pinned --submodule=short in GitPatchBuilder (GitIntegration/Builders/GitPatchBuilder.cs:184-188) so that a moved submodule appears as a stageable gitlink hunk. Submodule dirtiness is not pinned. When a submodule's working tree has modified or untracked files but its HEAD hasn't moved, git diff --submodule=short still emits an entry with a hunk:

diff --git a/sub b/sub
--- a/sub
+++ b/sub
@@ -1 +1 @@
-Subproject commit e372f884ad809ed1306fe6781f2c805e31b5376a
+Subproject commit e372f884ad809ed1306fe6781f2c805e31b5376a-dirty

GitPatchParser reads this as an ordinary Modified GitFilePatch with one hunk. Feeding that hunk through file.PatchFor(file.Hunks) and repository.Apply(text).ToIndex() exits 0 and stages nothing: the index gitlink is unchanged and git status still shows m sub.

This was verified with git 2.43, using the exact flags Patch() emits (--no-ext-diff --src-prefix=a/ --dst-prefix=b/ --submodule=short -U3 --no-renames) followed by git apply --cached --whitespace=nowarn.

If the submodule has moved and is dirty, the +…-dirty hunk does stage the new commit. Only the dirty-but-not-moved case produces a hunk that applies to nothing.

Why it matters

The patch verbs exist for hunk-level staging in a source-control view. In that view, this submodule shows a stageable hunk forever: "Stage hunk" reports success, the hunk is still there on refresh, and nothing explains why. Checked() doesn't catch it, because git apply --check also succeeds.

Suggested fix / acceptance criteria

  • Add --ignore-submodules=dirty to GitPatchBuilder's vector next to --submodule=short, so a patch contains only gitlink changes that Apply can stage. Note in the code comment that this overrides diff.ignoreSubmodules / submodule.<name>.ignore, as the other pins do. Status() and Diff() keep reporting the dirty submodule.
  • Alternatively, keep the entry but mark it as not stageable (e.g. a flag on GitFilePatch with empty Hunks). Either way, never hand out a hunk that applies to nothing.
  • Integration tests:
    • Dirty a submodule's working tree without moving its HEAD; Patch() returns no stageable hunk for it.
    • Move and dirty a submodule; the gitlink hunk is still reported and round-trips through Apply().ToIndex().

Activity

  1. matt-edmondson commented on Oct 5, 2026

    @matt-edmondson
    ContributorAuthor

    Triage


    Generated by Claude Code

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions