Do not run converger if dependencies have diverged

The remote converger should not worry about the dep
status code in case they are invalid. We have also
changed mix deps.get/deps.update to raise at the end
in case any of the dependencies diverged.

Closes hexpm/hex#169

Author: José Valim

lib/mix/lib/mix/dep.ex

@@ -281,12 +281,17 @@ defmodule Mix.Dep do
 
   Available dependencies are the ones that can be loaded.
   """
-  def available?(%Mix.Dep{status: {:unavailable, _}}),  do: false
-  def available?(%Mix.Dep{status: {:overridden, _}}),   do: false
-  def available?(%Mix.Dep{status: {:diverged, _}}),     do: false
-  def available?(%Mix.Dep{status: {:divergedreq, _}}),  do: false
-  def available?(%Mix.Dep{status: {:divergedonly, _}}), do: false
-  def available?(%Mix.Dep{}), do: true
+  def available?(%Mix.Dep{status: {:unavailable, _}}), do: false
+  def available?(dep), do: not diverged?(dep)
+
+  @doc """
+  Checks if a dependency has diverged.
+  """
+  def diverged?(%Mix.Dep{status: {:overridden, _}}),   do: true
+  def diverged?(%Mix.Dep{status: {:diverged, _}}),     do: true
+  def diverged?(%Mix.Dep{status: {:divergedreq, _}}),  do: true
+  def diverged?(%Mix.Dep{status: {:divergedonly, _}}), do: true
+  def diverged?(%Mix.Dep{}), do: false
 
   @doc """
   Formats a dependency for printing.

lib/mix/lib/mix/dep/converger.ex

@@ -69,7 +69,7 @@ defmodule Mix.Dep.Converger do
     # iteration.
     {deps, rest, lock} =
       all(main, only, [], current, callback, acc, lock, fn dep ->
-        if (converger = Mix.RemoteConverger.get) && converger.remote?(dep) do
+        if (remote = Mix.RemoteConverger.get) && remote.remote?(dep) do
           {:loaded, dep}
         else
           {:unloaded, dep, nil}
@@ -79,10 +79,11 @@ defmodule Mix.Dep.Converger do
     # Filter deps per environment once more. If the filtered
     # dependencies had no conflicts, they are removed now.
     {deps, _} = Mix.Dep.Loader.partition_by_env(deps, opts)
+    diverged? = Enum.any?(deps, &Mix.Dep.diverged?/1)
 
     # Run remote converger if one is available and rerun Mix's
     # converger with the new information
-    if remote = Mix.RemoteConverger.get do
+    if not diverged? && (remote = Mix.RemoteConverger.get) do
       # If there is a lock, it means we are doing a get/update
       # and we need to hit the remote converger which do external
       # requests and what not. In case of deps.check, deps and so

lib/mix/lib/mix/dep/fetcher.ex

@@ -105,8 +105,11 @@ defmodule Mix.Dep.Fetcher do
     # leave out things that could not be fetched and save it.
     lock = Map.merge(old_lock, new_lock)
     Mix.Dep.Lock.write(lock)
-
     mark_as_fetched(deps)
+
+    # See if any of the deps diverged and abort.
+    show_diverged!(Enum.filter(all_deps, &Mix.Dep.diverged?/1))
+
     {apps, all_deps}
   end
 
@@ -144,4 +147,17 @@ defmodule Mix.Dep.Fetcher do
       if is_binary(app), do: String.to_atom(app), else: app
     end)
   end
+
+  defp show_diverged!([]), do: :ok
+  defp show_diverged!(deps) do
+    shell = Mix.shell
+    shell.error "Dependencies have diverged:"
+
+    Enum.each deps, fn(dep) ->
+      shell.error "* #{Mix.Dep.format_dep dep}"
+      shell.error "  #{Mix.Dep.format_status dep}"
+    end
+
+    Mix.raise "Can't continue due to errors on dependencies"
+  end
 end

lib/mix/lib/mix/dep/umbrella.ex

@@ -14,7 +14,7 @@ defmodule Mix.Dep.Umbrella do
       paths
       |> extract_umbrella
       |> filter_umbrella(config[:apps])
-      |> to_umbrella_dep(build)
+      |> to_umbrella_dep(build, Path.absname("mix.exs"))
     else
       []
     end
@@ -48,7 +48,7 @@ defmodule Mix.Dep.Umbrella do
     for {app, _} = pair <- pairs, app in apps, do: pair
   end
 
-  defp to_umbrella_dep(paths, build) do
+  defp to_umbrella_dep(paths, build, from) do
     Enum.map paths, fn({app, path}) ->
       opts = [path: path, dest: Path.expand(path), from_umbrella: true,
               env: Mix.env, build: Path.join([build, "lib", Atom.to_string(app)])]
@@ -58,6 +58,7 @@ defmodule Mix.Dep.Umbrella do
         requirement: nil,
         manager: :mix,
         status: {:ok, nil},
+        from: from,
         opts: opts}
     end
   end

lib/mix/lib/mix/tasks/deps.check.ex

@@ -70,7 +70,6 @@ defmodule Mix.Tasks.Deps.Check do
     end
   end
 
-
   defp partition([dep|deps], not_ok, compile) do
     cond do
       from_umbrella?(dep)      -> partition(deps, not_ok, compile)

lib/mix/test/mix/dep_test.exs

@@ -208,6 +208,41 @@ defmodule Mix.DepTest do
     Mix.RemoteConverger.register(nil)
   end
 
+  defmodule RaiseRemoteConverger do
+    @behaviour Mix.RemoteConverger
+
+    def remote?(_app), do: false
+
+    def converge(_deps, lock) do
+      Process.put(:remote_converger, true)
+      lock
+    end
+
+    def deps(_deps, _lock) do
+      []
+    end
+  end
+
+  test "remote converger is not invoked if deps diverge" do
+    deps = [{:deps_repo, "0.1.0", path: "custom/deps_repo"},
+            {:git_repo, "0.2.0", git: MixTest.Case.fixture_path("git_repo"), only: :test}]
+
+    with_deps deps, fn ->
+      Mix.RemoteConverger.register(RaiseRemoteConverger)
+
+      in_fixture "deps_status", fn ->
+        assert_raise Mix.Error, fn ->
+          Mix.Tasks.Deps.Get.run([])
+        end
+
+        assert_received {:mix_shell, :error, ["Dependencies have diverged:"]}
+        refute Process.get(:remote_converger)
+      end
+    end
+  after
+    Mix.RemoteConverger.register(nil)
+  end
+
   ## Only handling
 
   test "only extract deps matching environment" do

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

@@ -344,6 +344,27 @@ defmodule Mix.Tasks.DepsTest do
     end
   end
 
+  test "fails on diverged dependencies on get/update" do
+    Mix.Project.push ConflictDepsApp
+
+    in_fixture "deps_status", fn ->
+      assert_raise Mix.Error, fn ->
+        Mix.Tasks.Deps.Check.run []
+      end
+      assert_received {:mix_shell, :error, ["  the dependency git_repo in mix.exs is overriding a child dependency" <> _]}
+
+      assert_raise Mix.Error, fn ->
+        Mix.Tasks.Deps.Get.run []
+      end
+      assert_received {:mix_shell, :error, ["  the dependency git_repo in mix.exs is overriding a child dependency" <> _]}
+
+      assert_raise Mix.Error, fn ->
+        Mix.Tasks.Deps.Update.run ["--all"]
+      end
+      assert_received {:mix_shell, :error, ["  the dependency git_repo in mix.exs is overriding a child dependency" <> _]}
+    end
+  end
+
   test "fails on diverged dependencies on check" do
     Mix.Project.push DivergedDepsApp
 
@@ -536,7 +557,6 @@ defmodule Mix.Tasks.DepsTest do
         app: :raw_sample,
         version: "0.1.0",
         deps: [
-          {:deps_repo, "0.1.0", path: "custom/deps_repo", compile: false},
           {:git_repo, "0.1.0", git: MixTest.Case.fixture_path("git_repo"), compile: false}
         ]
       ]
@@ -548,7 +568,6 @@ defmodule Mix.Tasks.DepsTest do
 
     in_fixture "deps_status", fn ->
       Mix.Tasks.Deps.Compile.run []
-      refute_received {:mix_shell, :info, ["==> deps_repo"]}
       refute_received {:mix_shell, :info, ["==> git_repo"]}
     end
   end