From a26744bf5644d71cd637ae3e09a2a80fb9701e39 Mon Sep 17 00:00:00 2001
From: Keith Salisbury <keithsalisbury@gmail.com>
Date: Fri, 22 Jun 2018 01:12:50 +0100
Subject: [PATCH] Update Mix Deps Converger

 - Add test for deps diverged from remote converger (@ericmj)
 - Add scm to the deps cache
 - Add code comments to help explain reasoning
---
 lib/mix/lib/mix/dep/converger.ex |  8 ++++---
 lib/mix/test/mix/dep_test.exs    | 40 +++++++++++++++++++++++++++++++-
 2 files changed, 44 insertions(+), 4 deletions(-)

diff --git a/lib/mix/lib/mix/dep/converger.ex b/lib/mix/lib/mix/dep/converger.ex
index 0992b24b77..2d46465cae 100644
--- a/lib/mix/lib/mix/dep/converger.ex
+++ b/lib/mix/lib/mix/dep/converger.ex
@@ -94,17 +94,19 @@ defmodule Mix.Dep.Converger do
       # on, there is no lock, so we won't hit this branch.
       lock = if lock_given?, do: remote.converge(deps, lock), else: lock
 
-      deps =
+      # Build a cache using both dep_name and dep_scm to ensure we only
+      # return :loaded for deps which have been loaded from the same source.
+      cache =
         deps
         |> Enum.reject(&remote.remote?(&1))
-        |> Enum.into(%{}, &{&1.app, &1})
+        |> Enum.into(%{}, &{{&1.app, &1.scm}, &1})
 
       # In case no lock was given, we will use the local lock
       # which is potentially stale. So remote.deps/2 needs to always
       # check if the data it finds in the lock is actually valid.
       {deps, rest, lock} =
         all(main, apps, callback, rest, lock, env, fn dep ->
-          if cached = deps[dep.app] do
+          if cached = cache[{dep.app, dep.scm}] do
             {:loaded, cached}
           else
             {:unloaded, dep, remote.deps(dep, lock)}
diff --git a/lib/mix/test/mix/dep_test.exs b/lib/mix/test/mix/dep_test.exs
index c9c7e58de0..93abd43a84 100644
--- a/lib/mix/test/mix/dep_test.exs
+++ b/lib/mix/test/mix/dep_test.exs
@@ -292,7 +292,7 @@ defmodule Mix.DepTest do
     end)
   end
 
-  ## Remove converger
+  ## Remote converger
 
   defmodule IdentityRemoteConverger do
     @behaviour Mix.RemoteConverger
@@ -330,6 +330,44 @@ defmodule Mix.DepTest do
     Mix.RemoteConverger.register(nil)
   end
 
+  defmodule DivergingRemoteConverger do
+    @behaviour Mix.RemoteConverger
+
+    def remote?(%Mix.Dep{app: :deps_repo, scm: Mix.SCM.Path}), do: true
+    def remote?(%Mix.Dep{app: :git_repo, scm: Mix.SCM.Path}), do: true
+    def remote?(%Mix.Dep{}), do: false
+    def deps(%Mix.Dep{app: :deps_repo}, _lock), do: [{:git_repo, path: "custom/git_repo"}]
+    def deps(%Mix.Dep{app: :git_repo}, _lock), do: []
+    def post_converge, do: :ok
+
+    def converge(_deps, lock) do
+      lock
+      |> Map.put(:deps_repo, :custom)
+      |> Map.put(:git_repo, :custom)
+    end
+  end
+
+  test "converger detects diverged deps from remote converger" do
+    deps = [
+      {:deps_on_git_repo, "0.2.0", git: MixTest.Case.fixture_path("deps_on_git_repo")},
+      {:deps_repo, "0.1.0", path: "custom/deps_repo"}
+    ]
+
+    with_deps(deps, fn ->
+      Mix.RemoteConverger.register(DivergingRemoteConverger)
+
+      in_fixture("deps_status", fn ->
+        assert_raise Mix.Error, fn ->
+          Mix.Tasks.Deps.Get.run([])
+        end
+
+        assert_received {:mix_shell, :error, ["Dependencies have diverged:"]}
+      end)
+    end)
+  after
+    Mix.RemoteConverger.register(nil)
+  end
+
   test "pass dependencies to remote converger in defined order" do
     deps = [
       {:ok, "0.1.0", path: "deps/ok"},