Fix bundle outdated crashing when both ref and branch specified for a git gem in Gemfile
#6959
Conversation
pboling
force-pushed
the
fix-git-proxy-for-branch-and-ref
branch
2 times, most recently
from
22d7f4c
to
dbc1ce6
Compare
| let(:revision) { nil } | ||
| let(:git_source) { nil } | ||
| let(:clone_result) { double(Process::Status, :success? => true) } | ||
| let(:base_clone_args) { ["clone", "--bare", "--no-hardlinks", "--quiet", "--no-tags", "--depth", "1", "--single-branch"] } | ||
| subject { described_class.new(path, uri, ref, revision, git_source) } |
There was a problem hiding this comment.
Note that the signature for GitProxy.new is not followed here, sending 'HEAD' as ref instead of the actual expected 3rd argument of options as a Hash. This is the case for all tests in this file, clearly indicating that these tests did zero testing of any options. It was working only because of ducky handling:
>> opt = "HEAD"
=> "HEAD"
>> opt["tag"]
=> nil
There was a problem hiding this comment.
This is great. Thank you for this. I had made a quick pass at flipping the order, saw that not one single test failed, and decided I would have to come back to it. Thanks for taking the time to increase the test coverage and make more than zero tests cover this behavior, and for raising the error when the nonsensical is given.
|
By the way, I'm fine with the multiple unrelated commits. We just won't squash this when we merge it so the commit messages can stay coupled to their changes even though they are not themselves related. It would also be fine to make separate PRs with each of them. |
pboling
force-pushed
the
fix-git-proxy-for-branch-and-ref
branch
2 times, most recently
from
91213e3
to
0f47ea6
Compare
- Specs for GitProxy were incorrect and insufficient - Specs are now correct and less insufficient
pboling
force-pushed
the
fix-git-proxy-for-branch-and-ref
branch
from
0f47ea6
to
2851e05
Compare
|
Rebased on latest upstream/master |
deivid-rodriguez
changed the title
bundle outdatedcrashing when both ref and branch specified for a git gem in Gemfile
deivid-rodriguez
changed the title
bundle outdatedcrashing when both ref and branch specified for a git gem in Gemfilebundle outdated crashing when both ref and branch specified for a git gem in Gemfile
(cherry picked from commit c2c0966)
(cherry picked from commit c2c0966)
Fixes #6951 to honor
ref>branch. This in turn fixesbundle outdatedfor Gemfiles with dependencies specifying bothbranchandref.Also:
reforbranchare specified withtag, as that's an ambiguous state.What was the end-user or developer problem that led to this PR?
branchandrefon a git source, some bundler commands break (e.g.bundle outdatedsee bundle outdated uses incorrect git command when branch and ref specified #6951)branchhad higher precedence thanref, which is incorrect.tagwas in betweenbranchandrefin precedence, which doesn't make any sense, and in fact,tagshould never be specified withbranchorref, as it creates an ambiguous reference that has until now been arbitrarily decided by the code in bundler (should it be the tag, or the branch/ref?).What is your fix for the problem, implemented in this PR?
This implementation raises an error, and I expect it can affect users who unwittingly specified ambiguous references in their Gemfile, but for whom
bundlecommands currently work. As such this might be considered a breaking change, but it is also definitely a bugfix. In many cases this would only affect users for whom the bundle / git commands are already broken, such as the case in #6951.Make sure the following tasks are checked