Skip to content

Conversation

@erka
Copy link
Contributor

@erka erka commented Dec 7, 2025

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
Copy link

dosubot bot commented Dec 7, 2025

Related Documentation

1 document(s) may need updating based on files changed in this PR:

flipt

How did I do? Any feedback?  Join Discord

@codecov
Copy link

codecov bot commented Dec 7, 2025

Codecov Report

❌ Patch coverage is 70.00000% with 12 lines in your changes missing coverage. Please review.
✅ Project coverage is 60.60%. Comparing base (5c24303) to head (b6da8ac).

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.60%   +0.06%     
==========================================
  Files         138      138              
  Lines       13507    13545      +38     
==========================================
+ Hits         8177     8209      +32     
- Misses       4641     4643       +2     
- Partials      689      693       +4     
Flag Coverage Δ
integrationtests 34.77% <50.00%> (+0.09%) ⬆️
unittests 51.96% <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.

Copy link
Contributor

Copilot AI left a comment

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)
}
}
Copy link

Copilot AI Dec 8, 2025

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
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.

2 participants