Merge pull request #1082 from ericmj/mix-umbrella-appfile2

Clean up mix dependency retrieving and converging

Author: José Valim

lib/mix/lib/mix/deps.ex

@@ -43,9 +43,7 @@ defmodule Mix.Deps do
   @doc """
   Returns all direct child dependencies.
   """
-  def children do
-    Mix.Deps.Project.all
-  end
+  defdelegate children(), to: Mix.Deps.Retriever
 
   @doc """
   Returns all dependencies depending on given dependencies.
@@ -162,7 +160,7 @@ defmodule Mix.Deps do
   @doc """
   Updates the dependency inside the given project.
   """
-  defdelegate update(dep), to: Mix.Deps.Project
+  defdelegate update(dep), to: Mix.Deps.Retriever
 
   @doc """
   Check if a dependency is ok.
@@ -234,14 +232,14 @@ defmodule Mix.Deps do
   end
 
   @doc """
-  Returns if dependency is a mix project.
+  Returns true if dependency is a mix project.
   """
   def mix?(dep) do
     dep.project != nil
   end
 
   @doc """
-  Returns if dependency is a rebar project.
+  Returns true if dependency is a rebar project.
   """
   def rebar?(dep) do
     Enum.any? ["rebar.config", "rebar.config.script"], fn file ->
@@ -250,7 +248,7 @@ defmodule Mix.Deps do
   end
 
   @doc """
-  Returns if dependency is a make project.
+  Returns true if dependency is a make project.
   """
   def make?(dep) do
     File.regular? Path.join(dep.opts[:dest], "Makefile")

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

@@ -4,8 +4,6 @@
 defmodule Mix.Deps.Converger do
   @moduledoc false
 
-  import Mix.Deps, only: [available?: 1]
-
   @doc """
   Clear up the mixfile cache.
   """
@@ -20,11 +18,10 @@ defmodule Mix.Deps.Converger do
   an updated depedency in case some processing is done.
   """
   def all(rest, callback) do
-    main   = Enum.reverse Mix.Deps.Project.all
     config = [ deps_path: Path.expand(Mix.project[:deps_path]),
                root_lockfile: Path.expand(Mix.project[:lockfile]) ]
-    { deps, rest } = all(main, [], [], main, config, callback, rest)
-    { nest_deps(deps, config), rest }
+    { main, rest } = Mix.Deps.Retriever.all(rest, config,callback)
+    { all(Enum.reverse(main), [], [], main), rest }
   end
 
   # We traverse the tree of dependencies in a breadth-
@@ -66,27 +63,25 @@ defmodule Mix.Deps.Converger do
   # Now, since `d` was specified in a parent project, no
   # exception is going to be raised since d is considered
   # to be the authorative source.
-  defp all([dep|t], acc, upper_breadths, current_breadths, config, callback, rest) do
+  defp all([dep|t], acc, upper_breadths, current_breadths) do
     cond do
       contains_dep?(upper_breadths, dep) ->
-        all(t, acc, upper_breadths, current_breadths, config, callback, rest)
+        all(t, acc, upper_breadths, current_breadths)
       match?({ diverged_acc, true }, diverged_dep?(acc, dep)) ->
-        all(t, diverged_acc, upper_breadths, current_breadths, config, callback, rest)
+        all(t, diverged_acc, upper_breadths, current_breadths)
       true ->
-        { dep, rest } = callback.(dep, rest)
-
-        if available?(dep) and mixfile?(dep) do
-          { project, deps } = nested_deps(dep, config)
-          { acc, rest } = all(t, [dep.project(project)|acc], upper_breadths, current_breadths, config, callback, rest)
-          all(deps, acc, current_breadths, deps ++ current_breadths, config, callback, rest)
+        deps = dep.deps
+        if deps != [] do
+          acc = all(t, [dep|acc], upper_breadths, current_breadths)
+          all(deps, acc, current_breadths, deps ++ current_breadths)
         else
-          all(t, [dep|acc], upper_breadths, current_breadths, config, callback, rest)
+          all(t, [dep|acc], upper_breadths, current_breadths)
         end
     end
   end
 
-  defp all([], acc, _upper, _current, _config, _callback, rest) do
-    { acc, rest }
+  defp all([], acc, _upper, _current) do
+    acc
   end
 
   # Does the list contain the given dependency?
@@ -111,40 +106,4 @@ defmodule Mix.Deps.Converger do
       end
     end
   end
-
-  defp mixfile?(dep) do
-    File.regular?(Path.join dep.opts[:dest], "mix.exs")
-  end
-
-  # Sets the `deps` field to the child dependencies of all
-  # given dependencies and does so recursively.
-  defp nest_deps(deps, config) do
-    Enum.map deps, fn dep ->
-      nest_deps(dep, deps, config)
-    end
-  end
-
-  defp nest_deps(dep, deps, config) do
-    if available?(dep) and mixfile?(dep) do
-      nested_apps = nested_names(dep, config)
-      sub_deps = Enum.filter(deps, fn dep -> dep.app in nested_apps end)
-      dep.deps Enum.map(sub_deps, nest_deps(&1, deps, config))
-    else
-      dep
-    end
-  end
-
-  defp nested_names(dep, post_config) do
-    Mix.Deps.in_dependency dep, post_config, fn _ ->
-      Mix.Deps.Project.all_names
-    end
-  end
-
-  # The dependency contains a Mixfile, so let's
-  # load it and retrieve its nested dependencies.
-  defp nested_deps(dep, post_config) do
-    Mix.Deps.in_dependency dep, post_config, fn project ->
-      { project, Enum.reverse Mix.Deps.Project.all }
-    end
-  end
 end

lib/mix/lib/mix/deps/project.ex renamed to lib/mix/lib/mix/deps/retriever.ex

@@ -1,7 +1,7 @@
-# This module is responsible for interacting
-# with dependencies of a given project. This
+# This module is responsible for retrieving
+# dependencies of a given project. This
 # module and its functions are private to Mix.
-defmodule Mix.Deps.Project do
+defmodule Mix.Deps.Retriever do
   @moduledoc false
 
   @doc """
@@ -13,18 +13,56 @@ defmodule Mix.Deps.Project do
   This function raises an exception in case the developer
   provides a dependency in the wrong format.
   """
-  def all do
-    deps = Mix.project[:deps] || []
-    scms = Mix.SCM.available
-    Enum.map deps, with_scm_and_status(&1, scms)
+  def all(post_config // []) do
+    { deps, _ } = all(nil, post_config, fn(dep, acc) -> { dep, acc } end)
+    deps
+  end
+
+  @doc """
+  Like `all/0` but takes a callback that is invoked for
+  each dependency and must return an updated depedency
+  in case some processing is done.
+  """
+  def all(rest, post_config // [], callback) do
+    Enum.map_reduce children, rest, fn (dep, rest) ->
+      { dep, rest } = callback.(dep, rest)
+
+      if Mix.Deps.available?(dep) and Mix.Deps.mix?(dep) do
+        { dep, rest } = Mix.Deps.in_dependency dep, post_config, fn _ ->
+          { deps, rest } = all(rest, callback)
+          { dep.deps(deps), rest }
+        end
+      end
+
+      { dep, rest }
+    end
   end
 
   @doc """
-  Returns all application names of current project's dependencies.
+  Gets all direct children for the current Mix.Project
+  as a `Mix.Dep` record. Unlike with `all` the `deps`
+  field is not populated.
   """
-  def all_names do
+  def children() do
     deps = Mix.project[:deps] || []
-    Enum.map(deps, elem(&1, 0))
+    scms = Mix.SCM.available
+
+    Enum.map deps, fn dep ->
+      dep = with_scm_and_status(dep, scms)
+
+      # Set properties if dependency is a mix project
+      if Mix.Deps.available?(dep) and mixfile?(dep) do
+        dep = Mix.Deps.in_dependency dep, fn project ->
+          if match?({ :noappfile, _ }, dep.status) and Mix.Project.umbrella? do
+            dep = dep.update_opts(Keyword.put(&1, :app, false))
+                     .status({ :ok, nil })
+          end
+          dep.project(project)
+        end
+      end
+
+      dep
+    end
   end
 
   @doc """
@@ -106,4 +144,8 @@ defmodule Mix.Deps.Project do
   defp vsn_match?(nil, _actual), do: true
   defp vsn_match?(expected, actual) when is_binary(expected), do: actual == expected
   defp vsn_match?(expected, actual) when is_regex(expected),  do: actual =~ expected
+
+  defp mixfile?(dep) do
+    File.regular?(Path.join dep.opts[:dest], "mix.exs")
+  end
 end

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

@@ -43,4 +43,4 @@ defmodule Mix.Tasks.Deps.Check do
 
   defp ok?(Mix.Dep[status: { :ok, _ }]), do: true
   defp ok?(_),                           do: false
-end
+end

lib/mix/test/fixtures/umbrella_dep/mix.exs

@@ -7,6 +7,6 @@ defmodule UmbrellaDep.Mixfile do
   end
 
   defp deps do
-    [ { :umbrella, path: "deps/umbrella", app: false } ]
+    [ { :umbrella, path: "deps/umbrella" } ]
   end
 end