Load all umbrella children from the fetcher, closes #4069

Author: José Valim

lib/mix/lib/mix/dep.ex

@@ -72,7 +72,9 @@ defmodule Mix.Dep do
   This function raises an exception if any of the dependencies
   provided in the project are in the wrong format.
   """
-  defdelegate children(), to: Mix.Dep.Loader
+  def children do
+    Mix.Dep.Loader.children(Mix.env)
+  end
 
   @doc """
   Returns loaded dependencies recursively as a `Mix.Dep` struct.
@@ -83,7 +85,7 @@ defmodule Mix.Dep do
   provided in the project are in the wrong format.
   """
   def loaded(opts) do
-    Mix.Dep.Converger.converge(nil, nil, opts, &{&1, &2, &3}) |> elem(0)
+    Mix.Dep.Converger.converge(children(), nil, nil, opts, &{&1, &2, &3}) |> elem(0)
   end
 
   @doc """

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

@@ -43,13 +43,12 @@ defmodule Mix.Dep.Converger do
 
   See `Mix.Dep.Loader.children/1` for options.
   """
-  def converge(acc, lock, opts, callback) do
-    {deps, acc, lock} = all(acc, lock, opts, callback)
+  def converge(deps, acc, lock, opts, callback) do
+    {deps, acc, lock} = all(deps, acc, lock, opts, callback)
     {topsort(deps), acc, lock}
   end
 
-  defp all(acc, lock, opts, callback) do
-    deps = Mix.Dep.Loader.children()
+  defp all(deps, acc, lock, opts, callback) do
     deps = Enum.map(deps, &(%{&1 | top_level: true}))
     lock_given? = !!lock
 

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

@@ -13,7 +13,8 @@ defmodule Mix.Dep.Fetcher do
   Fetches all dependencies.
   """
   def all(old_lock, new_lock, opts) do
-    result = Mix.Dep.Converger.converge([], new_lock, opts, &do_fetch/3)
+    deps = Mix.Dep.Loader.children(nil) # We do not use environments on the fetcher
+    result = Mix.Dep.Converger.converge(deps, [], new_lock, opts, &do_fetch/3)
     {apps, _deps} = do_finalize(result, old_lock, opts)
     apps
   end
@@ -23,7 +24,8 @@ defmodule Mix.Dep.Fetcher do
   """
   def by_name(names, old_lock, new_lock, opts) do
     fetcher = fetch_by_name(names, new_lock)
-    result = Mix.Dep.Converger.converge([], new_lock, opts, fetcher)
+    deps = Mix.Dep.Loader.children(nil) # We do not use environments on the fetcher
+    result = Mix.Dep.Converger.converge(deps, [], new_lock, opts, fetcher)
     {apps, deps} = do_finalize(result, old_lock, opts)
 
     # Check if all given dependencies are loaded or fail

lib/mix/lib/mix/dep/loader.ex

@@ -14,8 +14,8 @@ defmodule Mix.Dep.Loader do
   By default, it will filter all dependencies that does not match
   current environment, behaviour can be overridden via options.
   """
-  def children() do
-    mix_children([]) ++ Mix.Dep.Umbrella.unloaded
+  def children(env) do
+    mix_children([]) ++ Mix.Dep.Umbrella.unloaded(env)
   end
 
   @doc """
@@ -236,7 +236,10 @@ defmodule Mix.Dep.Loader do
         opts = Keyword.put_new(opts, :app, false)
       end
 
-      deps = mix_children(env: opts[:env] || :prod) ++ Mix.Dep.Umbrella.unloaded
+      # env may be nil and we should respect it
+      env  = Keyword.get(opts, :env, :prod)
+      deps = mix_children(env: env) ++ Mix.Dep.Umbrella.unloaded(env)
+
       {%{dep | opts: opts}, deps}
     end)
   end

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

@@ -3,8 +3,13 @@ defmodule Mix.Dep.Umbrella do
 
   @doc """
   Gets all umbrella dependencies in unloaded format.
+
+  The environment is required and usually is set to
+  `Mix.env` (i.e. the same environment as the parent
+  project). However, it may be nil when invoked by
+  the fetcher so all deps are fetched recursively.
   """
-  def unloaded do
+  def unloaded(env) do
     config = Mix.Project.config
 
     if apps_path = config[:apps_path] do
@@ -14,7 +19,7 @@ defmodule Mix.Dep.Umbrella do
       paths
       |> extract_umbrella
       |> filter_umbrella(config[:apps])
-      |> to_umbrella_dep(build, Path.absname("mix.exs"))
+      |> to_umbrella_dep(build, Path.absname("mix.exs"), env)
     else
       []
     end
@@ -24,7 +29,7 @@ defmodule Mix.Dep.Umbrella do
   Gets all umbrella dependencies in the loaded format.
   """
   def loaded do
-    deps = unloaded
+    deps = unloaded(Mix.env)
     apps = Enum.map(deps, &(&1.app))
 
     Enum.map(deps, fn umbrella_dep ->
@@ -48,10 +53,10 @@ defmodule Mix.Dep.Umbrella do
     for {app, _} = pair <- pairs, app in apps, do: pair
   end
 
-  defp to_umbrella_dep(paths, build, from) do
+  defp to_umbrella_dep(paths, build, from, env) 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)])]
+              env: env, build: Path.join([build, "lib", Atom.to_string(app)])]
       %Mix.Dep{
         scm: Mix.SCM.Path,
         app: app,

lib/mix/test/mix/dep_test.exs

@@ -85,7 +85,7 @@ defmodule Mix.DepTest do
     Mix.Project.push DepsApp
 
     {_, true, _} =
-      Mix.Dep.Converger.converge(false, [], nil, fn dep, acc, lock ->
+      Mix.Dep.Converger.converge(Mix.Dep.children, false, [], nil, fn dep, acc, lock ->
         assert is_nil(dep.manager)
         {dep, acc or true, lock}
       end)

lib/mix/test/mix/umbrella_test.exs

@@ -34,7 +34,7 @@ defmodule Mix.UmbrellaTest do
     end
   end
 
-  test "compiles umbrella with protocol consolidation (via build embedded)" do
+  test "compiles umbrella with protocol consolidation" do
     in_fixture "umbrella_dep/deps/umbrella", fn ->
       Mix.Project.in_project(:umbrella, ".", [build_embedded: true], fn _ ->
         assert_raise Mix.Error, ~r"Cannot execute task because the project was not yet compiled", fn ->
@@ -98,6 +98,41 @@ defmodule Mix.UmbrellaTest do
     end
   end
 
+  test "loads umbrella child dependencies in all environments" do
+    in_fixture "umbrella_dep/deps/umbrella", fn ->
+      Mix.Project.in_project :umbrella, ".", fn _ ->
+        File.write! "apps/bar/mix.exs", """
+        defmodule Bar.Mix do
+          use Mix.Project
+
+          def project do
+            [app: :bar,
+             version: "0.1.0",
+             deps: [{:git_repo, git: MixTest.Case.fixture_path("git_repo"), only: :other}]]
+          end
+        end
+        """
+
+        # Should work across all environments
+        Mix.Tasks.Deps.Get.run []
+        assert_received {:mix_shell, :info, ["* Getting git_repo" <> _]}
+
+        # Works on the current environment only
+        Mix.Tasks.Deps.run []
+        refute_received {:mix_shell, :info, ["* git_repo " <> _]}
+
+        # Works on the other environment only
+        Mix.env(:other)
+        Mix.Tasks.Deps.run []
+        assert_received {:mix_shell, :info, ["* git_repo " <> _]}
+      end
+    end
+  after
+    Mix.env(:test)
+  end
+
+  ## Umbrellas as a dependency
+
   test "list deps for umbrella as dependency" do
     in_fixture("umbrella_dep", fn ->
       Mix.Project.in_project(:umbrella_dep, ".", fn _ ->