Merge #6708

6708: Fix only_update_to_newer_versions regression r=greysteil a=theflow

This is my attempt to fix #6529

### What was the end-user problem that led to this PR?

Running `bundle update` with `BUNDLE_ONLY_UPDATE_TO_NEWER_VERSIONS: "true"` resulted in a gem getting downgraded to a really old version in a certain edge case. Ironically it wouldn't get downgraded when `BUNDLE_ONLY_UPDATE_TO_NEWER_VERSIONS` was set to false.

### What was your diagnosis of the problem?

My diagnosis was that https://github.com/bundler/bundler/commit/47256d20cb05ebc724ee67173094682153b6b4aa tried to solve the problem of still allowing manual downgrades in the Gemfile while `only_update_to_newer_versions` is true. But introduced a regression that prevented the `additional_base_requirements_for_resolve` method to work as intended:

This is the relevant change from that commit that tries to avoid adding the `>=` requirement if the  requirement in the Gemfile is different than the requirement in the lockfile (as far as I understand it):

```ruby
next requirements if @locked_deps[name] != dependencies_by_name[name]
```

I identified two problems
 1. `dependencies_by_name[name]` returns an array of `Bundler::Dependency`, where as 
`@locked_deps[name]` just returns a single `Bundler::Dependency`. Comparing the two will always be false.
 1. `@locked_deps` is always empty in case of `bundle update`. See: https://github.com/bundler/bundler/blob/3d9e6167a7df9ca89a030dfe95c7cdff293e74a9/lib/bundler/definition.rb#L95

### What is your fix for the problem, implemented in this PR?

My fixes:

 1. Make sure `dependencies_by_name` is a hash with `Bundler::Dependency` as values
 1. Fetch the `@locked_gems.dependencies` again instead of using `@locked_deps`
 1. The existing test worked for me with and without the `only_update_to_newer_versions` set to true, I replaced it with a reproduction of the edge case I was investigating (this is as minimal as I could make it)
 1. I've added a test for the manual downgrading case.

### Why did you choose this fix out of the possible options?

This is the only way I could make these cases work. It's possible there are other edge cases I don't understand.

Co-authored-by: Florian Munz <surf@theflow.de>
(cherry picked from commit 8501b1e3608579acf53a4978b62c0d8891d23005)

2 files changed, 47 insertions, 13 deletions

diff --git a/lib/bundler/definition.rb b/lib/bundler/definition.rb
index 89ae1ce926..1585fc3125 100644
--- a/lib/bundler/definition.rb
+++ b/lib/bundler/definition.rb
@@ -975,10 +975,10 @@ module Bundler
 
     def additional_base_requirements_for_resolve
       return [] unless @locked_gems && Bundler.feature_flag.only_update_to_newer_versions?
-      dependencies_by_name = dependencies.group_by(&:name)
+      dependencies_by_name = dependencies.inject({}) {|memo, dep| memo.update(dep.name => dep) }
       @locked_gems.specs.reduce({}) do |requirements, locked_spec|
         name = locked_spec.name
-        next requirements if @locked_deps[name] != dependencies_by_name[name]
+        next requirements if @locked_gems.dependencies[name] != dependencies_by_name[name]
         dep = Gem::Dependency.new(name, ">= #{locked_spec.version}")
         requirements[name] = DepProxy.new(dep, locked_spec.platform)
         requirements
diff --git a/spec/commands/update_spec.rb b/spec/commands/update_spec.rb
index 675e8574f1..e8f5275118 100644
--- a/spec/commands/update_spec.rb
+++ b/spec/commands/update_spec.rb
@@ -121,33 +121,67 @@ RSpec.describe "bundle update" do
       before do
         bundle! "config only_update_to_newer_versions true"
       end
+
       it "does not go to an older version" do
         build_repo4 do
-          build_gem "a" do |s|
-            s.add_dependency "b"
-            s.add_dependency "c"
+          build_gem "tilt", "2.0.8"
+          build_gem "slim", "3.0.9" do |s|
+            s.add_dependency "tilt", [">= 1.3.3", "< 2.1"]
+          end
+          build_gem "slim_lint", "0.16.1" do |s|
+            s.add_dependency "slim", [">= 3.0", "< 5.0"]
+          end
+          build_gem "slim-rails", "0.2.1" do |s|
+            s.add_dependency "slim", ">= 0.9.2"
+          end
+          build_gem "slim-rails", "3.1.3" do |s|
+            s.add_dependency "slim", "~> 3.0"
           end
-          build_gem "b"
-          build_gem "c"
-          build_gem "c", "2.0"
         end
 
         install_gemfile! <<-G
           source "file:#{gem_repo4}"
-          gem "a"
+          gem "slim-rails"
+          gem "slim_lint"
         G
 
-        expect(the_bundle).to include_gems("a 1.0", "b 1.0", "c 2.0")
+        expect(the_bundle).to include_gems("slim 3.0.9", "slim-rails 3.1.3", "slim_lint 0.16.1")
 
         update_repo4 do
-          build_gem "b", "2.0" do |s|
-            s.add_dependency "c", "< 2"
+          build_gem "slim", "4.0.0" do |s|
+            s.add_dependency "tilt", [">= 2.0.6", "< 2.1"]
           end
         end
 
         bundle! "update", :all => bundle_update_requires_all?
 
-        expect(the_bundle).to include_gems("a 1.0", "b 1.0", "c 2.0")
+        expect(the_bundle).to include_gems("slim 3.0.9", "slim-rails 3.1.3", "slim_lint 0.16.1")
+      end
+
+      it "should still downgrade if forced by the Gemfile" do
+        build_repo4 do
+          build_gem "a"
+          build_gem "b", "1.0"
+          build_gem "b", "2.0"
+        end
+
+        install_gemfile! <<-G
+          source "file:#{gem_repo4}"
+          gem "a"
+          gem "b"
+        G
+
+        expect(the_bundle).to include_gems("a 1.0", "b 2.0")
+
+        gemfile <<-G
+          source "file://#{gem_repo4}"
+          gem "a"
+          gem "b", "1.0"
+        G
+
+        bundle! "update b"
+
+        expect(the_bundle).to include_gems("a 1.0", "b 1.0")
       end
     end
   end