Follow renamed default branch in git sources - #9813
Open
hsbt wants to merge 15 commits into
Open
Conversation
When a git source pins no branch, Bundler fetches whatever branch the cached clone's HEAD points at. Renaming the remote's default branch made that fetch fail, and the failure was swallowed as a network error, so the lockfile stayed silently pinned to the old revision. Resolve the remote's current default branch on that failure and repoint the cached clone. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Rename the branch without naming the old one so the spec does not depend on what `build_git` picks as its default branch. Drop the comment that repeated the rename rationale already stated on the recovery helper. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The recovery reassigned the `command` and `command_with_no_credentials` locals that the enclosing retry block closes over. When `git symbolic-ref` then failed, the retry re-ran the recovery fetch instead of the original one, that fetch succeeded because the ref was already there, and the block returned success with HEAD still on the deleted branch, so a stale revision was written with nothing reported. The same reassignment made a failed recovery report the recovery command next to the old branch name. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Repointing the cached clone's HEAD changes which branch every project sharing that cache tracks, so it should not happen silently. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Every other git invocation here puts the Gemfile-controlled URI after `--` so git cannot read it as an option, and `git ls-remote` accepts `--upload-pack`, which runs a command. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Dropping the guard on the recovery path left every git spec green, so nothing stopped an explicit `:branch` from being silently swapped for whatever branch the remote now points HEAD at. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`git_null` drops stderr, so an unsupported `--symref`, an auth failure and a network error all collapsed into the same silent nil and the recovery just did not happen with nothing to explain it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`refspec = refspec()` only reaches the method because of the parentheses. Dropping them binds the half-defined local instead, and `.compact` then turns the missing refspec into a fetch that quietly updates nothing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The `&&` form put a boolean and a branch name in the same local, so any later `nil?` check would have missed the false case. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A failed `git symbolic-ref` raised through the retry block, which then redid the whole recovery on each attempt: four passes, twelve network round trips and nine seconds of backoff for a local lock conflict. Treat it as best effort instead and report the original fetch failure, which also stops the cache being left with the new branch fetched but HEAD still on the old one. The recovery fetch now says why it failed too. The message it prints on success no longer claims the branch was renamed, which it cannot observe. A deleted branch, a cache seeded by another project's `branch:`, and a detached cache HEAD all reach the same line and none of them is a rename. It also warns rather than informs now, since it changes which commit gets installed and `--quiet` drops info. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Deleting the warning or the guard that keeps a pinned branch from following left both specs green, so neither the message nor the cached clone's HEAD was actually covered. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The added block ran to about 10% comment lines against roughly 4% for the file, and part of it narrated the method body instead of explaining it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A remote may advertise a ref name that is not valid UTF-8, and matching that as text raises ArgumentError, which is not a GitError, so it was retried four times and then escaped as a backtrace. Match the bytes instead. The rescue around the repoint was narrower than the comment above it claimed, and the debug lines carried the whole command, which still holds credentials that the redaction does not reach. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The glob returned nil when it matched nothing and the subprocess helper skips chdir for a nil directory, so the assertion would have run git against the rubygems checkout itself. Deriving the path the way the source does makes a miss an outright failure, and reading the file keeps the assertion from displacing the bundle command that `err` reports on. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Dropping `commit.nil?` from the guard left every spec green, so nothing stopped a source whose locked revision the remote had dropped from silently repointing the shared cache at the default branch instead. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A git source that pins no branch, tag or ref follows whatever branch the cached bare clone's
HEADpoints at. When the remote renames its default branch, that fetch fails withcouldn't find remote ref, andBundler::Source::Git#fetchswallows it as a network problem. The user seesUsing cached git data because of network errorswhile the lockfile stays pinned to the old revision, sobundle updatenever picks up new commits. Recovering means deleting the cache by hand.On that specific failure, and only when no branch, tag, ref or locked revision was requested, Bundler now asks the remote for its current default branch with
git ls-remote --symref, fetches it, and repoints the cached clone'sHEAD. Unaffected sources pay no extra round trip because the lookup runs only after a fetch has already failed.Fixes #5810.