Dependencies convergence properly relies on requirements

Previously Mix required dependencies to have exactly the same
requirements but now we just check all requirements satisfy the
current version.

Author: José Valim

lib/mix/lib/mix/deps.ex

@@ -221,16 +221,24 @@ defmodule Mix.Deps do
   def format_status(Mix.Dep[status: :nolock]),
     do: "the dependency is not locked"
 
+  def format_status(Mix.Dep[app: app, status: { :divergedreq, other }] = dep) do
+    "the dependency #{app} defined\n" <>
+    "#{dep_status(dep)}" <>
+    "\n  does not match the requirement specified\n" <>
+    "#{dep_status(other)}" <>
+    "\n  Ensure they match or specify one of the above in your #{inspect Mix.Project.get} deps and set `override: true`"
+  end
+
   def format_status(Mix.Dep[app: app, status: { :diverged, other }] = dep) do
     "different specs were given for the #{inspect app} app:\n" <>
     "#{dep_status(dep)}#{dep_status(other)}" <>
-    "\n  Ensure they match or specify one in your #{inspect Mix.Project.get} deps and set `override: true`"
+    "\n  Ensure they match or specify one of the above in your #{inspect Mix.Project.get} deps and set `override: true`"
   end
 
   def format_status(Mix.Dep[app: app, status: { :overriden, other }] = dep) do
     "the dependency #{app} in #{Path.relative_to_cwd(dep.from)} is overriding a child dependency:\n" <>
     "#{dep_status(dep)}#{dep_status(other)}" <>
-    "\n  Ensure they match or specify one in your #{inspect Mix.Project.get} deps and set `override: true`"
+    "\n  Ensure they match or specify one of the above in your #{inspect Mix.Project.get} deps and set `override: true`"
   end
 
   def format_status(Mix.Dep[status: { :unavailable, _ }]),
@@ -280,6 +288,7 @@ defmodule Mix.Deps do
   """
   def available?(Mix.Dep[status: { :overriden, _ }]),    do: false
   def available?(Mix.Dep[status: { :diverged, _ }]),     do: false
+  def available?(Mix.Dep[status: { :divergedreq, _ }]),  do: false
   def available?(Mix.Dep[status: { :elixirreq, _ }]),    do: false
   def available?(Mix.Dep[status: { :unavailable, _ }]),  do: false
   def available?(Mix.Dep[]), do: true

lib/mix/lib/mix/deps/converger.ex

@@ -132,8 +132,8 @@ defmodule Mix.Deps.Converger do
           Mix.Dep[app: other_app, opts: other_opts] = other
 
           cond do
-            app == other_app && (other_opts[:override] || converge?(dep, other)) ->
-              { other, acc }
+            app == other_app && (other_opts[:override] || converge?(other, dep)) ->
+              { with_matching_req(other, dep), acc }
             app == other_app ->
               { other.status({ :overriden, dep }), acc }
             true ->
@@ -159,8 +159,8 @@ defmodule Mix.Deps.Converger do
         cond do
           app != other_app ->
             { other, match }
-          converge?(dep, other) ->
-            { other, true }
+          converge?(other, dep) ->
+            { with_matching_req(other, dep), true }
           true ->
             { other.status({ :diverged, dep }), true }
         end
@@ -169,10 +169,22 @@ defmodule Mix.Deps.Converger do
     if match, do: acc
   end
 
-  defp converge?(Mix.Dep[scm: scm, requirement: req, opts: opts1],
-                 Mix.Dep[scm: scm, requirement: req, opts: opts2]) do
+  defp converge?(Mix.Dep[scm: scm, opts: opts1], Mix.Dep[scm: scm, opts: opts2]) do
     scm.equal?(opts1, opts2)
   end
 
   defp converge?(_, _), do: false
+
+  def with_matching_req(Mix.Dep[] = other, Mix.Dep[] = dep) do
+    case other.status do
+      { :ok, vsn } when not nil?(vsn) ->
+        if Mix.Deps.Retriever.vsn_match?(dep.requirement, vsn) do
+          other
+        else
+          other.status({ :divergedreq, dep })
+        end
+      _ ->
+        other
+    end
+  end
 end

lib/mix/lib/mix/deps/retriever.ex

@@ -46,6 +46,16 @@ defmodule Mix.Deps.Retriever do
     end)
   end
 
+  @doc """
+  Checks if a requirement from a dependency matches
+  the given version.
+  """
+  def vsn_match?(nil, _actual), do: true
+  def vsn_match?(req, actual) when is_regex(req), do: actual =~ req
+  def vsn_match?(req, actual) when is_binary(req) do
+    Version.match?(actual, req)
+  end
+
   ## Helpers
 
   defp to_dep(tuple, scms, from, manager // nil) do
@@ -193,12 +203,6 @@ defmodule Mix.Deps.Retriever do
     end
   end
 
-  defp vsn_match?(nil, _actual), do: true
-  defp vsn_match?(req, actual) when is_regex(req), do: actual =~ req
-  defp vsn_match?(req, actual) when is_binary(req) do
-    Version.match?(actual, req)
-  end
-
   defp old_elixir_lock do
     old_vsn = Mix.Deps.Lock.elixir_vsn
     if old_vsn && old_vsn != System.version do

lib/mix/test/mix/deps_test.exs

@@ -82,6 +82,28 @@ defmodule Mix.DepsTest do
     Mix.Project.pop
   end
 
+  defmodule NestedDepsApp do
+    def project do
+      [
+        app: :raw_sample,
+        version: "0.1.0",
+        deps: [
+          { :deps_repo, "0.1.0", path: "custom/deps_repo" }
+        ]
+      ]
+    end
+  end
+
+  test "nested deps come first" do
+    Mix.Project.push NestedDepsApp
+
+    in_fixture "deps_status", fn ->
+      assert Enum.map(Mix.Deps.fetched, &(&1.app)) == [:git_repo, :deps_repo]
+    end
+  after
+    Mix.Project.pop
+  end
+
   defmodule ConvergedDepsApp do
     def project do
       [
@@ -95,11 +117,11 @@ defmodule Mix.DepsTest do
     end
   end
 
-  test "correctly order overriden deps" do
+  test "correctly order converged deps" do
     Mix.Project.push ConvergedDepsApp
 
     in_fixture "deps_status", fn ->
-      assert [:git_repo, :deps_repo] == Enum.map(Mix.Deps.fetched, &(&1.app))
+      assert Enum.map(Mix.Deps.fetched, &(&1.app)) == [:git_repo, :deps_repo]
     end
   after
     Mix.Project.pop

lib/mix/test/mix/tasks/deps_test.exs

@@ -142,7 +142,7 @@ defmodule Mix.Tasks.DepsTest do
     Mix.Project.pop
   end
 
-  test "check list of dependencies and their status with failure" do
+  test "check slist of dependencies and their status with failure" do
     Mix.Project.push OutOfDateDepsApp
 
     in_fixture "deps_status", fn ->
@@ -156,7 +156,7 @@ defmodule Mix.Tasks.DepsTest do
     Mix.Project.pop
   end
 
-  test "check list of dependencies and their status on failure" do
+  test "checks list of dependencies and their status on failure" do
     Mix.Project.push DepsApp
 
     in_fixture "deps_status", fn ->
@@ -228,7 +228,7 @@ defmodule Mix.Tasks.DepsTest do
     end
   end
 
-  test "by default sets deps env to prod" do
+  test "sets deps env to prod by default" do
     Mix.Project.push DepsEnvApp
 
     in_fixture "deps_status", fn ->
@@ -284,7 +284,7 @@ defmodule Mix.Tasks.DepsTest do
         version: "0.1.0",
         deps: [
           { :deps_repo, "0.1.0", path: "custom/deps_repo" },
-          { :git_repo, "0.1.0", git: MixTest.Case.fixture_path("git_repo") }
+          { :git_repo, ">= 0.1", git: MixTest.Case.fixture_path("git_repo") }
         ]
       ]
     end
@@ -345,10 +345,6 @@ defmodule Mix.Tasks.DepsTest do
       assert_received { :mix_shell, :info, [^message] }
       refute_received { :mix_shell, :info, ["* Compiling deps_repo"] }
       assert_received { :mix_shell, :info, ["Generated git_repo.app"] }
-
-      Mix.Tasks.Deps.Update.run ["--all"]
-      assert_received { :mix_shell, :info, ["* Updating deps_repo (custom/deps_repo)"] }
-      refute_received { :mix_shell, :info, ["* Compiling deps_repo"] }
     end
   after
     Mix.Project.pop
@@ -374,6 +370,42 @@ defmodule Mix.Tasks.DepsTest do
     Mix.Project.pop
   end
 
+  test "fails on diverged dependencies by requirement" do
+    Mix.Project.push ConvergedDepsApp
+
+    in_fixture "deps_status", fn ->
+      File.write!("custom/deps_repo/mix.exs", """)
+      defmodule DepsRepo do
+        use Mix.Project
+
+        def project do
+          [
+            app: :deps_repo,
+            version: "0.1.0",
+            deps: [
+              { :git_repo, "0.2.0", git: MixTest.Case.fixture_path("git_repo") }
+            ]
+          ]
+        end
+      end
+      """
+
+      assert_raise Mix.Error, fn ->
+        Mix.Tasks.Deps.Get.run []
+      end
+
+      receive do
+        { :mix_shell, :error, ["  the dependency git_repo defined" <> _ = msg] } ->
+          assert msg =~ "In custom/deps_repo/mix.exs:"
+          assert msg =~ "{:git_repo, \"0.2.0\", [git: #{inspect fixture_path("git_repo")}]}"
+      after
+        0 -> flunk "expected diverged req error message"
+      end
+    end
+  after
+    Mix.Project.pop
+  end
+
   test "works with converged dependencies" do
     Mix.Project.push ConvergedDepsApp
 
@@ -430,7 +462,7 @@ defmodule Mix.Tasks.DepsTest do
     Mix.Project.pop
   end
 
-  test "converged dependencies will error if not overriding" do
+  test "converged dependencies errors if not overriding" do
     Mix.Project.push NonOverridenDepsApp
 
     in_fixture "deps_status", fn ->
@@ -451,19 +483,7 @@ defmodule Mix.Tasks.DepsTest do
     Mix.Project.pop
   end
 
-  test "converged dependencies are properly ordered" do
-    Mix.Project.push NestedDepsApp
-
-    in_fixture "deps_status", fn ->
-      # Nested dependencies need to come first. They are
-      # listed first, compiled first, etc.
-      assert [Mix.Dep[app: :git_repo], Mix.Dep[app: :deps_repo]] = Mix.Deps.fetched
-    end
-  after
-    Mix.Project.pop
-  end
-
-  test "update parent dependencies" do
+  test "updates parent dependencies" do
     Mix.Project.push NestedDepsApp
 
     in_fixture "deps_status", fn ->
@@ -479,7 +499,7 @@ defmodule Mix.Tasks.DepsTest do
     Mix.Project.pop
   end
 
-  test "check if dependencies are using old elixir" do
+  test "checks if dependencies are using old elixir version" do
     Mix.Project.push SuccessfulDepsApp
 
     in_fixture "deps_status", fn ->
@@ -513,7 +533,7 @@ defmodule Mix.Tasks.DepsTest do
     end
   end
 
-  test "dont compile deps" do
+  test "does not compile deps that have explicit flag" do
     Mix.Project.push NonCompilingDeps
 
     in_fixture "deps_status", fn ->
@@ -539,7 +559,7 @@ defmodule Mix.Tasks.DepsTest do
     end
   end
 
-  test "converts duplicated deps at the same level" do
+  test "converges duplicated deps at the same level" do
     Mix.Project.push DupDeps
 
     in_fixture "deps_status", fn ->