Skip to content

feat(gitter): Stop trying to make fetch happen#5299

Merged
Ly-Joey merged 4 commits intogoogle:masterfrom
Ly-Joey:feat-gitter-fallback-clone
May 6, 2026
Merged

feat(gitter): Stop trying to make fetch happen#5299
Ly-Joey merged 4 commits intogoogle:masterfrom
Ly-Joey:feat-gitter-fallback-clone

Conversation

@Ly-Joey
Copy link
Copy Markdown
Contributor

@Ly-Joey Ly-Joey commented May 5, 2026

Gitter will attempt to solve a few common git errors (index.lock file exists, and refname conflict) when git fetch and reset fail.
If that doesn't fix the git fetch, fallback to a fresh reclone instead of redoing fetch on a broken repo next time.

Comment thread go/cmd/gitter/gitter.go Fixed
Comment thread go/cmd/gitter/gitter.go Dismissed
@jess-lowe
Copy link
Copy Markdown
Contributor

fetch

@Ly-Joey Ly-Joey requested review from another-rex and tobyhawker May 5, 2026 05:22
Comment thread go/cmd/gitter/gitter.go Outdated
Comment on lines +429 to +450
if isIndexLockError(err) {
logger.WarnContext(ctx, "index.lock exists, attempting to remove")
indexLockPath := filepath.Join(repoPath, ".git", "index.lock")
if err := os.Remove(indexLockPath); err != nil {
logger.ErrorContext(ctx, "failed to remove index.lock", slog.Any("err", err))
return false
}

return true
}

// Refname conflict, likely name conflict between local and remote refs
// We can try removing stale remote-tracking branches and retry
if isRefConflictError(err) {
logger.WarnContext(ctx, "ref conflict detected, running git remote prune origin")
if err := runCmd(ctx, repoPath, nil, "git", "remote", "prune", "origin"); err != nil {
logger.ErrorContext(ctx, "failed to prune origin", slog.Any("err", err))
return false
}

return true
}
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.

I think this is even being too clever, now that we have graceful shutdowns, how often do these issues actually come up?

I'm leaning towards just always reclone at the first sign of trouble.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The index.lock fix has been in gitter since #4816 so I don't really have stats for it.

For the second case (refname conflict). We’ve seen this issue in 7 repos over the past week in our test env.

It happens when, for example:

  1. A repo starts off with a fix branch
  2. Gitter clones / fetches at this point (we now have a local ref fix).
  3. Afterwards, the branch can be renamed upstream to fix/some-other-name. Similarly they could have deleted the fix branch and started some more branches under the fix/ prefix.
  4. When gitter fetches again, the refname conflict will happen as git doesn't differentiate between directory and files. So the local fix ref name will be in conflict with every branch upstream that starts with fix/.

That said, this is not a hill I'm dying on, so I'm happy to change to just using reclone as the fallback.

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.

Let's keep this second case. For the first case I think let's reclone, and log it so we know how often this happens.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done :)

@Ly-Joey Ly-Joey requested a review from another-rex May 6, 2026 05:07
Copy link
Copy Markdown
Contributor

@another-rex another-rex left a comment

Choose a reason for hiding this comment

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

LGTM

@Ly-Joey Ly-Joey merged commit be6eec8 into google:master May 6, 2026
21 checks passed
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.

4 participants