Unify deps.get and deps.update workflows

Author: José Valim

lib/mix/lib/mix/deps.ex

@@ -400,61 +400,6 @@ defmodule Mix.Deps do
 
   ## Helpers
 
-  @doc false
-  # Called by deps.get and deps.update
-  def finalize(all_deps, apps, lock, opts) do
-    deps = loaded_by_name(apps, all_deps)
-
-    # Do not attempt to compile dependencies that are not available.
-    # mix deps.check at the end will emit proper status in case they failed.
-    deps = Enum.filter(deps, &available?/1)
-
-    # Note we only retrieve the parent dependencies of the updated
-    # deps if all dependencies are available. This is because if a
-    # dependency is missing, it could be a children of the parent
-    # (aka a sibling) which would make parent compilation fail.
-    #
-    # If there is any other dependency that is not ok, we include
-    # it for compilation too, this is our best to try to solve the
-    # maximum we can at each deps.get and deps.update.
-    if Enum.all?(all_deps, &available?/1) do
-      deps = with_depending(deps, all_deps) ++
-             Enum.filter(all_deps, fn dep -> not ok?(dep) end)
-    end
-
-    apps = Enum.map(deps, &(&1.app)) |> Enum.uniq
-    Mix.Deps.Lock.write(lock)
-
-    unless opts[:no_compile] do
-      if apps != [] do
-        args = if opts[:quiet], do: ["--quiet"|apps], else: apps
-        Mix.Task.run("deps.compile", args)
-      end
-
-      unless opts[:no_deps_check] do
-        Mix.Task.run("deps.check", [])
-      end
-    end
-  end
-
-  defp with_depending(deps, all_deps) do
-    (deps ++ do_with_depending(deps, all_deps)) |> Enum.uniq(&(&1.app))
-  end
-
-  defp do_with_depending([], _all_deps) do
-    []
-  end
-
-  defp do_with_depending(deps, all_deps) do
-    dep_names = Enum.map(deps, fn dep -> dep.app end)
-
-    parents = Enum.filter all_deps, fn dep ->
-      Enum.any?(dep.deps, &(&1 in dep_names))
-    end
-
-    do_with_depending(parents, all_deps) ++ parents
-  end
-
   defp to_app_names(given) do
     Enum.map given, fn(app) ->
       if is_binary(app), do: binary_to_atom(app), else: app

lib/mix/lib/mix/deps/fetcher.ex

@@ -0,0 +1,119 @@
+# Module responsible for fetching (getting/updating)
+# dependencies from their sources.
+#
+# The new_lock and old_lock mechanism exists to signal
+# externally which dependencies need to be updated and
+# which ones do not.
+defmodule Mix.Deps.Fetcher do
+  @moduledoc false
+
+  import Mix.Deps, only: [format_dep: 1, check_lock: 2, out_of_date?: 1, available?: 1, ok?: 1]
+
+  @doc """
+  Fetches all dependencies.
+  """
+  def all(old_lock, new_lock, opts) do
+    do_finalize Mix.Deps.unloaded({ [], new_lock }, &do_fetch/2), old_lock, opts
+  end
+
+  @doc """
+  Fetches the dependencies with the given names and their children recursively.
+  """
+  def by_name(names, old_lock, new_lock, opts) do
+    do_finalize Mix.Deps.unloaded_by_name(names, { [], new_lock }, &do_fetch/2), old_lock, opts
+  end
+
+  defp do_fetch(dep, { acc, lock }) do
+    Mix.Dep[app: app, scm: scm, opts: opts] = dep = check_lock(dep, lock)
+
+    cond do
+      # Dependencies that cannot be fetched are always compiled afterwards
+      not scm.fetchable? ->
+        { dep, { [app|acc], lock } }
+
+      # If the dependency is not available or we have a lock mismatch
+      out_of_date?(dep) ->
+        new =
+          if scm.checked_out?(opts) do
+            Mix.shell.info "* Updating #{format_dep(dep)}"
+            scm.update(opts)
+          else
+            Mix.shell.info "* Getting #{format_dep(dep)}"
+            scm.checkout(opts)
+          end
+
+        if new do
+          { dep, { [app|acc], Keyword.put(lock, app, new) } }
+        else
+          { dep, { acc, lock } }
+        end
+
+      # The dependency is ok or has some other error
+      true ->
+        { dep, { acc, lock } }
+    end
+  end
+
+  defp do_finalize({ all_deps, { apps, new_lock } }, old_lock, opts) do
+    # Let's get the loaded versions of deps
+    deps = Mix.Deps.loaded_by_name(apps, all_deps)
+
+    # Do not attempt to compile dependencies that are not available.
+    # mix deps.check at the end will emit proper status in case they failed.
+    deps = Enum.filter(deps, &available?/1)
+
+    # Note we only retrieve the parent dependencies of the updated
+    # deps if all dependencies are available. This is because if a
+    # dependency is missing, it could be a children of the parent
+    # (aka a sibling) which would make parent compilation fail.
+    #
+    # If there is any other dependency that is not ok, we include
+    # it for compilation too, this is our best to try to solve the
+    # maximum we can at each deps.get and deps.update.
+    if Enum.all?(all_deps, &available?/1) do
+      deps = with_depending(deps, all_deps) ++
+             Enum.filter(all_deps, fn dep -> not ok?(dep) end)
+    end
+
+    # Merge the new lock on top of the old to guarantee we don't
+    # leave out things that could not be fetched and save it.
+    lock = Dict.merge(old_lock, new_lock)
+    Mix.Deps.Lock.write(lock)
+
+    do_compile(deps, opts)
+    apps
+  end
+
+  defp do_compile(deps, opts) do
+    apps = Enum.map(deps, &(&1.app)) |> Enum.uniq
+
+    unless opts[:no_compile] do
+      if apps != [] do
+        args = if opts[:quiet], do: ["--quiet"|apps], else: apps
+        Mix.Task.run("deps.compile", args)
+      end
+
+      unless opts[:no_deps_check] do
+        Mix.Task.run("deps.check", [])
+      end
+    end
+  end
+
+  defp with_depending(deps, all_deps) do
+    (deps ++ do_with_depending(deps, all_deps)) |> Enum.uniq(&(&1.app))
+  end
+
+  defp do_with_depending([], _all_deps) do
+    []
+  end
+
+  defp do_with_depending(deps, all_deps) do
+    dep_names = Enum.map(deps, fn dep -> dep.app end)
+
+    parents = Enum.filter all_deps, fn dep ->
+      Enum.any?(dep.deps, &(&1 in dep_names))
+    end
+
+    do_with_depending(parents, all_deps) ++ parents
+  end
+end

lib/mix/lib/mix/scm/path.ex

@@ -22,9 +22,9 @@ defmodule Mix.SCM.Path do
         path = "../#{app}"
 
         opts
-          |> Keyword.put(:dest, Path.expand(path))
-          |> Keyword.put_new(:path, path)
-          |> Keyword.put_new(:env, Mix.env)
+        |> Keyword.put(:dest, Path.expand(path))
+        |> Keyword.put_new(:path, path)
+        |> Keyword.put_new(:env, Mix.env)
       true ->
         nil
     end

lib/mix/lib/mix/tasks/deps.get.ex

@@ -15,64 +15,19 @@ defmodule Mix.Tasks.Deps.Get do
 
   """
 
-  import Mix.Deps, only: [unloaded: 2, unloaded_by_name: 3, format_dep: 1,
-                          check_lock: 2, out_of_date?: 1]
-
   def run(args) do
     Mix.Project.get! # Require the project to be available
     { opts, rest, _ } = OptionParser.parse(args, switches: [no_compile: :boolean, quiet: :boolean])
 
-    acc =
+    apps =
       if rest != [] do
-        unloaded_by_name(rest, init, &deps_getter/2)
+        Mix.Deps.Fetcher.by_name(rest, [], Mix.Deps.Lock.read, opts)
       else
-        unloaded(init, &deps_getter/2)
+        Mix.Deps.Fetcher.all([], Mix.Deps.Lock.read, opts)
       end
 
-    finalize_get(acc, opts)
-  end
-
-  defp init do
-    { [], Mix.Deps.Lock.read }
-  end
-
-  defp finalize_get({ all_deps, { apps, lock } }, opts) do
-    Mix.Deps.finalize(all_deps, apps, lock, opts)
-
     if apps == [] && !opts[:quiet] do
       Mix.shell.info "All dependencies up to date"
     end
   end
-
-  defp deps_getter(dep, { acc, lock }) do
-    shell = Mix.shell
-    Mix.Dep[app: app, scm: scm, opts: opts] = dep = check_lock(dep, lock)
-
-    cond do
-      # Dependencies that cannot be fetched are always compiled afterwards
-      not scm.fetchable? ->
-        { dep, { [app|acc], lock } }
-
-      # If the dependency is not available or we have a lock mismatch
-      out_of_date?(dep) ->
-        shell.info "* Getting #{format_dep(dep)}"
-
-        new =
-          if scm.checked_out?(opts) do
-            scm.update(opts)
-          else
-            scm.checkout(opts)
-          end
-
-        if new do
-          { dep, { [app|acc], Keyword.put(lock, app, new) } }
-        else
-          { dep, { acc, lock } }
-        end
-
-      # The dependency is ok or has some other error
-      true ->
-        { dep, { acc, lock } }
-    end
-  end
 end

lib/mix/lib/mix/tasks/deps.update.ex

@@ -20,48 +20,25 @@ defmodule Mix.Tasks.Deps.Update do
 
   """
 
-  import Mix.Deps, only: [unloaded: 2, unloaded_by_name: 3, available?: 1, format_dep: 1]
-
   def run(args) do
     Mix.Project.get! # Require the project to be available
     { opts, rest, _ } = OptionParser.parse(args, switches: [no_compile: :boolean, all: :boolean])
 
     cond do
       opts[:all] ->
-        acc = unloaded(init, &deps_updater/2)
+        Mix.Deps.Fetcher.all(Mix.Deps.Lock.read, [], opts)
       rest != [] ->
-        acc = unloaded_by_name(rest, init, &deps_updater/2)
+        { old, new } = Dict.split(Mix.Deps.Lock.read, to_app_names(rest))
+        Mix.Deps.Fetcher.by_name(rest, old, new, opts)
       true ->
         raise Mix.Error, message: "mix deps.update expects dependencies as arguments or " <>
                                   "the --all option to update all dependencies"
     end
-
-    finalize_update(acc, opts)
-  end
-
-  defp init do
-    { [], Mix.Deps.Lock.read }
   end
 
-  defp finalize_update({ all_deps, { apps, lock } }, opts) do
-    Mix.Deps.finalize(all_deps, apps, lock, opts)
-  end
-
-  defp deps_updater(dep, { acc, lock }) do
-    if available?(dep) do
-      Mix.Dep[app: app, scm: scm, opts: opts] = dep
-      Mix.shell.info "* Updating #{format_dep(dep)}"
-
-      lock =
-        if latest = scm.update(opts) do
-          Keyword.put(lock, app, latest)
-        else
-          lock
-        end
-
-      { dep, { [app|acc], lock } }
-    else
-      { dep, { acc, lock } }
+  defp to_app_names(given) do
+    Enum.map given, fn(app) ->
+      if is_binary(app), do: binary_to_atom(app), else: app
     end
   end
 end

lib/mix/test/mix/tasks/deps.git_test.exs

@@ -241,7 +241,7 @@ defmodule Mix.Tasks.DepsGitTest do
       assert File.exists?("deps/git_repo/lib/git_repo.ex")
       assert File.read!("mix.lock") =~ last
 
-      message = "* Getting git_repo (#{fixture_path("git_repo")})"
+      message = "* Updating git_repo (#{fixture_path("git_repo")})"
       assert_received { :mix_shell, :info, [^message] }
 
       # Check we got no error

lib/mix/test/mix/tasks/deps.path_test.exs

@@ -15,12 +15,11 @@ defmodule Mix.Tasks.DepsPathTest do
     end
   end
 
-  test "updates and cleans path repos with compilation" do
+  test "compiles path repos on update" do
     Mix.Project.push DepsApp
 
     in_fixture "deps_status", fn ->
       Mix.Tasks.Deps.Update.run ["--all"]
-      assert_received { :mix_shell, :info, ["* Updating raw_repo (custom/raw_repo)"] }
       assert_received { :mix_shell, :info, ["Compiled lib/raw_repo.ex"] }
       assert_received { :mix_shell, :info, ["Generated raw_repo.app"] }
       assert File.exists?("_build/shared/lib/raw_repo/ebin/Elixir.RawRepo.beam")

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

@@ -378,7 +378,6 @@ defmodule Mix.Tasks.DepsTest do
       Mix.Task.clear
 
       Mix.Tasks.Deps.Update.run ["--all"]
-      assert_received { :mix_shell, :info, ["* Updating deps_repo (custom/deps_repo)"] }
       assert_received { :mix_shell, :info, ["* Compiling deps_repo"] }
     end
   end
@@ -491,8 +490,6 @@ defmodule Mix.Tasks.DepsTest do
       Mix.Task.clear
       Mix.Tasks.Deps.Update.run ["--all"]
 
-      message = "* Updating deps_repo (custom/deps_repo)"
-      assert_received { :mix_shell, :info, [^message] }
       message = "* Updating git_repo (#{fixture_path("git_repo")})"
       assert_received { :mix_shell, :info, [^message] }
 
@@ -518,8 +515,6 @@ defmodule Mix.Tasks.DepsTest do
       Mix.Task.clear
       Mix.Tasks.Deps.Update.run ["--all"]
 
-      message = "* Updating bad_deps_repo (custom/bad_deps_repo)"
-      assert_received { :mix_shell, :info, [^message] }
       message = "* Updating git_repo (#{fixture_path("git_repo")})"
       assert_received { :mix_shell, :info, [^message] }
     end