Skip to content

fix(writer): reject commits with unchanged trees#379

Open
KSHITIZ6341 wants to merge 1 commit into
grafana:mainfrom
KSHITIZ6341:fix/unchanged-tree-commit
Open

fix(writer): reject commits with unchanged trees#379
KSHITIZ6341 wants to merge 1 commit into
grafana:mainfrom
KSHITIZ6341:fix/unchanged-tree-commit

Conversation

@KSHITIZ6341

Copy link
Copy Markdown

What

  • Return ErrNothingToCommit when staged operations rebuild the same root tree as the parent commit.
  • Add a regression test for updating an existing blob with identical content.

Why

PackfileWriter.HasObjects() reports whether blob or tree objects were staged, not whether the resulting repository tree changed. Re-staging identical content therefore rebuilt the same root tree but still allowed Commit() to create an empty commit, which could then produce a no-op receive-pack request.

Good catch by @jdstrand; the reproduction clearly isolated the distinction between staged packfile objects and an unchanged commit tree.

Fixes #378

How

After pending trees are built, Commit() compares the rebuilt root tree hash with the parent commit's tree hash before creating a commit object. A match returns the existing ErrNothingToCommit error. Commits whose final tree differs from the parent continue through the existing path unchanged.

The signer unit-test fixtures now use a distinct parent tree so they continue to represent a real changed-tree commit.

Remarks

Behavioral effect: callers now receive ErrNothingToCommit for semantically unchanged staged updates, including an identical UpdateBlob, instead of receiving a new commit. There is no public API change and changed-tree commits are unaffected.

Checks performed:

  • go test -race -parallel 6 --short -count=1 across all packages except protocol/signing
  • go test . -run '^TestStagedWriter_Commit_(UnchangedTree|SignerError|SignerInvoked)$' -count=1
  • make lint
  • make lint-staticcheck
  • git diff --check

make test-unit could not complete locally because the host does not have the gpg and gpgsm executables required by protocol/signing; all other packages passed with the race detector. make test-integration could not start its Gitea fixture because no local Docker provider is available, so 0 of 321 integration specs ran locally.

Author Checklist

  • Tests added/updated.
  • Documentation is not required for this internal behavior correction.
  • Changes have been tested locally.

@cla-assistant

cla-assistant Bot commented Jul 11, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@cla-assistant

cla-assistant Bot commented Jul 11, 2026

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@KSHITIZ6341
KSHITIZ6341 marked this pull request as ready for review July 12, 2026 13:01
@KSHITIZ6341
KSHITIZ6341 requested a review from a team as a code owner July 12, 2026 13:01
@KSHITIZ6341
KSHITIZ6341 requested review from floriecai and konsalex and removed request for a team July 12, 2026 13:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Push() sends a no-op receive-pack when re-staging unchanged content — Commit() lacks a tree-vs-parent guard

1 participant