Skip to content

fix(git): complete ref update after fetch for both bare and normal repositories#5144

Merged
markphelps merged 6 commits into
v2from
rd/go-git-grafted
Feb 5, 2026
Merged

fix(git): complete ref update after fetch for both bare and normal repositories#5144
markphelps merged 6 commits into
v2from
rd/go-git-grafted

Conversation

@erka

@erka erka commented Dec 7, 2025

Copy link
Copy Markdown
Contributor

The Fetch method was not persisting fetched references to refs/heads after
pulling from the remote. When using depth=1 (shallow fetches), the shallow
boundary wasn't maintained and refs/heads/ were never created. For normal repos
with full history, updatedRefs were collected but never written to storage.

  • For shallow repositories: Create refs/heads/ and maintain shallow boundary
  • For normal repositories: Create refs/heads/ from updatedRefs
  • Ensure both code paths properly persist fetched references to local storage

@erka
erka requested a review from a team as a code owner December 7, 2025 22:39
@dosubot dosubot Bot added the size:M This PR changes 30-99 lines, ignoring generated files. label Dec 7, 2025
@dosubot

dosubot Bot commented Dec 7, 2025

Copy link
Copy Markdown

Related Documentation

Checked 2 published document(s) in 1 knowledge base(s). No updates required.

How did I do? Any feedback?  Join Discord

@codecov

codecov Bot commented Dec 7, 2025

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 70.00000% with 12 lines in your changes missing coverage. Please review.
✅ Project coverage is 60.55%. Comparing base (17ed13d) to head (170fbea).
⚠️ Report is 1 commits behind head on v2.

Files with missing lines Patch % Lines
internal/storage/git/repository.go 70.00% 7 Missing and 5 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##               v2    #5144      +/-   ##
==========================================
+ Coverage   60.53%   60.55%   +0.02%     
==========================================
  Files         138      138              
  Lines       13540    13578      +38     
==========================================
+ Hits         8196     8222      +26     
- Misses       4653     4660       +7     
- Partials      691      696       +5     
Flag Coverage Δ
integrationtests 34.65% <47.50%> (+0.03%) ⬆️
unittests 51.95% <70.00%> (+0.14%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a critical bug where the Fetch method was not properly persisting fetched references to refs/heads/ after pulling from remote repositories. The fix handles both bare and normal repositories correctly, ensuring that shallow fetches maintain their boundaries and all repository types properly create local branch references.

Key changes:

  • Conditionally applies depth=1 only to bare repositories (not normal repos with working trees)
  • Adds logic to create/update refs/heads/ references from fetched remote refs
  • Implements branch pruning to remove local refs when branches are deleted on remote
  • Maintains shallow boundaries for depth=1 fetches by tracking shallow commit hashes

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +444 to +503
// Get all existing local branch refs
allLocalRefs, err := r.Storer.IterReferences()
if err != nil {
return fmt.Errorf("failed to get local refs: %w", err)
}

existingHeads := make(map[string]struct{})
if err := allLocalRefs.ForEach(func(ref *plumbing.Reference) error {
if ref.Name().IsBranch() {
branchName := strings.TrimPrefix(ref.Name().String(), "refs/heads/")
existingHeads[branchName] = struct{}{}
}
return nil
}); err != nil {
return fmt.Errorf("failed to iterate local refs: %w", err)
}

// Update refs for branches that exist on remote
for name, hash := range updatedRefs {
ref := plumbing.NewHashReference(plumbing.NewBranchReferenceName(name), hash)
if err := r.Storer.SetReference(ref); err != nil {
return fmt.Errorf("failed to set reference %s: %w", name, err)
}
delete(existingHeads, name)
}

// Remove local refs for branches that no longer exist on remote
for branchName := range existingHeads {
refName := plumbing.NewBranchReferenceName(branchName)
// Check if this branch is one we're tracking
isTracked := false
for _, head := range heads {
if refMatch(branchName, head) {
isTracked = true
break
}
}
if isTracked {
if err := r.Storer.RemoveReference(refName); err != nil {
return fmt.Errorf("failed to remove reference %s: %w", branchName, err)
}
}
}

if opts.Depth == 1 {
// When performing a depth=1 fetch, maintain the shallow boundary in the object store.
// For shallow clones, we track which commits are shallow to prevent operations that
// require complete commit history from proceeding.
shallows, err := r.Storer.Shallow()
if err != nil {
return fmt.Errorf("failed to get shallows: %w", err)
}
shallows = slices.AppendSeq(shallows, maps.Values(updatedRefs))
slices.SortFunc(shallows, func(a plumbing.Hash, b plumbing.Hash) int {
return a.Compare(b.Bytes())
})
if err := r.Storer.SetShallow(slices.Compact(shallows)); err != nil {
return fmt.Errorf("failed to set shallows: %w", err)
}
}

Copilot AI Dec 8, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new reference management logic (creating refs/heads/, pruning removed branches, and maintaining shallow boundaries) lacks test coverage. Consider adding tests that verify:

  1. Refs are correctly created in refs/heads/ after fetch
  2. Removed branches are properly pruned when they no longer exist on remote
  3. Shallow boundaries are maintained correctly for depth=1 fetches
  4. Behavior differences between bare and normal repositories

Copilot uses AI. Check for mistakes.
@dosubot dosubot Bot added size:L This PR changes 100-499 lines, ignoring generated files. and removed size:M This PR changes 30-99 lines, ignoring generated files. labels Dec 8, 2025
erka added 2 commits December 9, 2025 23:14
…positories

The Fetch method was not persisting fetched references to refs/heads after
pulling from the remote. When using depth=1 (shallow fetches), the shallow
boundary wasn't maintained and refs/heads/ were never created. For normal repos
with full history, updatedRefs were collected but never written to storage.

- For shallow repositories: Create refs/heads/ and maintain shallow boundary
- For normal repositories: Create refs/heads/ from updatedRefs
- Ensure both code paths properly persist fetched references to local storage

Signed-off-by: Roman Dmytrenko <[email protected]>
Signed-off-by: Roman Dmytrenko <[email protected]>
@erka
erka force-pushed the rd/go-git-grafted branch from d214b61 to b6da8ac Compare December 9, 2025 23:14
@markphelps
markphelps merged commit 8ac6346 into v2 Feb 5, 2026
29 checks passed
@markphelps
markphelps deleted the rd/go-git-grafted branch February 5, 2026 13:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L This PR changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants