Ensure umbrella is recompiled when a child dependency changes

Author: José Valim

lib/mix/lib/mix/dep.ex

@@ -31,10 +31,11 @@ defmodule Mix.Dep do
   * `:app` - The application name
   * `:dest` - The destination path for the dependency
   * `:lock` - The lock information retrieved from mix.lock
+  * `:build` - The build path for the dependency
 
   """
-  defstruct scm: nil, app: nil, requirement: nil, status: nil, opts: nil,
-            deps: [], top_level: false, extra: nil, manager: nil, from: nil
+  defstruct scm: nil, app: nil, requirement: nil, status: nil, opts: [],
+            deps: [], top_level: false, extra: [], manager: nil, from: nil
 
   @doc """
   Returns all children dependencies for the current project,

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

@@ -91,8 +91,9 @@ defmodule Mix.Dep.Fetcher do
 
     # 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.
+    # dependency is missing, it could directly affect one of the
+    # dependencies we are trying to compile, causing the whole thing
+    # to 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
@@ -107,15 +108,17 @@ defmodule Mix.Dep.Fetcher do
     lock = Dict.merge(old_lock, new_lock)
     Mix.Dep.Lock.write(lock)
 
-    require_compilation(deps)
+    mark_as_fetched(deps)
     {apps, all_deps}
   end
 
-  defp require_compilation(deps) do
-    envs = Path.wildcard("_build/*/lib")
-
-    for %Mix.Dep{app: app} <- deps, env <- envs do
-      File.touch Path.join [env, Atom.to_string(app), ".compile"]
+  defp mark_as_fetched(deps) do
+    # If the dependency is fetchable, we are going to write a .fetch
+    # file to it. Each build, regardless of the environment and location,
+    # will compared against this .fetch file to know if the depednency
+    # needs recompiling.
+    for %Mix.Dep{scm: scm, opts: opts} <- deps, scm.fetchable? do
+      File.touch Path.join opts[:dest], ".fetch"
     end
   end
 

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

@@ -245,22 +245,27 @@ defmodule Mix.Dep.Loader do
 
   defp validate_app(%Mix.Dep{opts: opts, requirement: req, app: app, status: status} = dep) do
     opts_app = opts[:app]
-    build    = opts[:build]
 
     cond do
       not ok?(status) ->
         dep
-      File.exists?(Path.join(opts[:build], ".compile")) ->
+      recently_fetched?(dep) ->
         %{dep | status: :compile}
       opts_app == false ->
         dep
       true ->
-        path  = if is_binary(opts_app), do: opts_app, else: "ebin/#{app}.app"
-        path  = Path.expand(path, build)
+        path = if is_binary(opts_app), do: opts_app, else: "ebin/#{app}.app"
+        path = Path.expand(path, opts[:build])
         %{dep | status: app_status(path, app, req)}
     end
   end
 
+  defp recently_fetched?(%Mix.Dep{opts: opts, scm: scm}) do
+    scm.fetchable? &&
+      Mix.Utils.stale?([Path.join(opts[:dest], ".fetch")],
+                       [Path.join(opts[:build], ".compile.lock")])
+  end
+
   defp app_status(app_path, app, req) do
     case :file.consult(app_path) do
       {:ok, [{:application, ^app, config}]} ->

lib/mix/lib/mix/dep/lock.ex

@@ -56,8 +56,7 @@ defmodule Mix.Dep.Lock do
     case File.read(lockfile) do
       {:ok, info} ->
         {value, _binding} = Code.eval_string(info)
-        # TODO: Remove Enum.into() once apps migrate to new lock
-        Enum.into(value || [], %{})
+        value || %{}
       {:error, _} ->
         %{}
     end

lib/mix/lib/mix/project_stack.ex

@@ -73,7 +73,7 @@ defmodule Mix.ProjectStack do
         [h|t] ->
           h = %{h | io_done: true}
           t = Enum.map(t, &%{&1 | io_done: false})
-          {true, %{state | stack: [h|t]}}
+          {has_app?(h), %{state | stack: [h|t]}}
       end
     end
   end
@@ -151,6 +151,10 @@ defmodule Mix.ProjectStack do
     {name, config, file}
   end
 
+  defp has_app?(%{config: config}) do
+    config[:app]
+  end
+
   defp get_and_update(fun) do
     Agent.get_and_update __MODULE__, fun, @timeout
   end

lib/mix/lib/mix/scm.ex

@@ -12,6 +12,9 @@ defmodule Mix.SCM do
   Returns a boolean if the dependency can be fetched
   or it is meant to be previously available in the
   filesystem.
+
+  Local dependencies (i.e. non fetchable ones) are automatically
+  recompiled every time the parent project is compiled.
   """
   defcallback fetchable? :: boolean
 

lib/mix/lib/mix/tasks/deps.compile.ex

@@ -24,7 +24,7 @@ defmodule Mix.Tasks.Deps.Compile do
   """
 
   import Mix.Dep, only: [loaded: 1, available?: 1, loaded_by_name: 2,
-                          format_dep: 1, make?: 1, mix?: 1, rebar?: 1]
+                         format_dep: 1, make?: 1, mix?: 1, rebar?: 1]
 
   def run(args) do
     Mix.Project.get! # Require the project to be available
@@ -43,7 +43,7 @@ defmodule Mix.Tasks.Deps.Compile do
     config = Mix.Project.deps_config
 
     compiled =
-      Enum.map(deps, fn %Mix.Dep{app: app, status: status, opts: opts} = dep ->
+      Enum.map(deps, fn %Mix.Dep{app: app, status: status, opts: opts, scm: scm} = dep ->
         check_unavailable!(app, status)
 
         compiled = cond do
@@ -61,7 +61,7 @@ defmodule Mix.Tasks.Deps.Compile do
         end
 
         unless mix?(dep), do: build_structure(dep, config)
-        File.rm(Path.join(opts[:build], ".compile"))
+        if scm.fetchable?, do: Mix.Dep.Lock.touch(opts[:build])
         compiled
       end)
 
@@ -70,8 +70,8 @@ defmodule Mix.Tasks.Deps.Compile do
 
   # All available dependencies can be compiled
   # except for umbrella applications.
-  defp compilable?(%Mix.Dep{manager: manager, extra: extra} = dep) do
-    available?(dep) and (manager != :mix or !extra[:umbrella?])
+  defp compilable?(%Mix.Dep{extra: extra} = dep) do
+    available?(dep) and !extra[:umbrella?]
   end
 
   defp check_unavailable!(app, {:unavailable, _}) do

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

@@ -32,11 +32,16 @@ defmodule Mix.Tasks.Deps do
   Mix also supports git and path dependencies:
 
       {:foobar, git: "https://github.com/elixir-lang/foobar.git", tag: "0.1"}
+      {:foobar, path: "path/to/foobar"}
 
-  And also umbrella applications:
+  And also in umbrella dependencies:
 
       {:myapp, in_umbrella: true}
 
+  Path and in umbrella dependencies are automatically recompiled by
+  the parent project whenever they change. While fetchable dependencies
+  like git are recompiled only when fetched/updated.
+
   The dependencies versions are expected to follow Semantic Versioning
   and the requirements must be specified as defined in the `Version`
   module.
@@ -74,7 +79,7 @@ defmodule Mix.Tasks.Deps do
 
   This task lists all dependencies in the following format:
 
-    * APP VERSION (SCM)
+      APP VERSION (SCM)
       [locked at REF]
       STATUS
 

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

@@ -41,14 +41,15 @@ defmodule Mix.Tasks.DepsGitTest do
     end
   end
 
-  test "get, update and clean git repos with compilation" do
+  test "gets and updates git repos with compilation" do
     Mix.Project.push GitApp
 
     in_fixture "no_mixfile", fn ->
       Mix.Tasks.Deps.Get.run []
       message = "* Getting git_repo (#{fixture_path("git_repo")})"
       assert_received {:mix_shell, :info, [^message]}
-      assert File.read!("mix.lock") =~ ~r/"git_repo": {:git, #{inspect fixture_path("git_repo")}, "[a-f0-9]+", \[\]}/
+      assert File.read!("mix.lock") =~
+             ~r/"git_repo": {:git, #{inspect fixture_path("git_repo")}, "[a-f0-9]+", \[\]}/
 
       Mix.Tasks.Deps.Update.run ["--all"]
       message = "* Updating git_repo (#{fixture_path("git_repo")})"
@@ -67,7 +68,7 @@ defmodule Mix.Tasks.DepsGitTest do
     end
   end
 
-  test "gets many levels deep dependencies" do
+  test "gets and updates many levels deep dependencies" do
     Mix.Project.push DepsOnGitApp
 
     in_fixture "no_mixfile", fn ->
@@ -80,64 +81,56 @@ defmodule Mix.Tasks.DepsGitTest do
       assert_received {:mix_shell, :info, [^message]}
 
       assert File.exists?("deps/deps_on_git_repo/mix.exs")
+      assert File.rm("deps/deps_on_git_repo/.fetch") == :ok
       assert File.exists?("deps/git_repo/mix.exs")
-    end
-  after
-    purge [GitRepo, GitRepo.Mix]
-  end
-
-  test "checks if repo information changes" do
-    Mix.Project.push GitApp
-
-    in_fixture "no_mixfile", fn ->
-      Mix.Tasks.Deps.Get.run []
-      message = "* Getting git_repo (#{fixture_path("git_repo")})"
-      assert_received {:mix_shell, :info, [^message]}
-
-      # We can compile just fine
-      Mix.Task.clear
-      Mix.Tasks.Run.run ["-e", "1+2"]
-      assert_received {:mix_shell, :info, ["==> git_repo"]}
-
-      # Now let's add a submodules option
-      Mix.Project.pop
-      Mix.Project.push GitSubmodulesApp
+      assert File.rm("deps/git_repo/.fetch") == :ok
 
-      # We should fail because the lock diverged
-      Mix.Task.clear
-      assert_raise Mix.Error, fn ->
-        Mix.Tasks.Run.run ["1+2"]
-      end
+      Mix.Tasks.Deps.Update.run ["deps_on_git_repo"]
+      assert File.exists?("deps/deps_on_git_repo/.fetch")
+      assert File.exists?("deps/git_repo/.fetch")
     end
   after
     purge [GitRepo, GitRepo.Mix]
   end
 
-  test "recompiles the project when a deps change" do
+  test "recompiles the project when a dep is fetched" do
     Mix.Project.push GitApp
 
     in_fixture "no_mixfile", fn ->
       Mix.Tasks.Deps.Get.run []
-      message = "* Getting git_repo (#{fixture_path("git_repo")})"
-      assert File.exists?("_build/dev/lib/git_app/.compile.lock")
-      assert_received {:mix_shell, :info, [^message]}
+      assert File.exists?("deps/git_repo/.fetch")
 
       # We can compile just fine
-      Mix.Task.clear
       Mix.Tasks.Compile.run []
+      assert_received {:mix_shell, :info, ["Compiled lib/git_repo.ex"]}
+      assert_received {:mix_shell, :info, ["Compiled lib/a.ex"]}
 
-      # Notify a deps changed
+      # Clear up to prepare for the update
+      File.rm("_build/dev/lib/git_repo/ebin/Elixir.GitRepo.beam")
+      File.rm("_build/dev/lib/git_repo/.compile.elixir")
+      File.rm("deps/git_repo/.fetch")
+      Mix.Task.clear
       Mix.shell.flush
-      File.touch!("_build/dev/lib/git_app/.compile.lock", {{2020, 4, 17}, {14, 0, 0}})
+      purge [A, B, C, GitRepo]
 
-      # We are forced to recompile
-      purge [A, B, C]
-      Mix.Task.clear
+      # Update will mark the update required
+      Mix.Tasks.Deps.Update.run ["git_repo"]
+      assert File.exists?("deps/git_repo/.fetch")
+      ensure_touched("deps/git_repo/.fetch") # Ensure timestamp differs
+
+      # mix deps.compile is required...
+      Mix.Tasks.Deps.run []
+      msg = "  the dependency build is outdated, please run `mix deps.compile`"
+      assert_received {:mix_shell, :info, [^msg]}
+
+      # But also ran automatically
       Mix.Tasks.Compile.run []
+      assert_received {:mix_shell, :info, ["Compiled lib/git_repo.ex"]}
       assert_received {:mix_shell, :info, ["Compiled lib/a.ex"]}
+      :ok
     end
   after
-    purge [GitRepo, GitRepo.Mix]
+    purge [A, B, C, GitRepo, GitRepo.Mix]
   end
 
   test "all up to date dependencies" do
@@ -233,6 +226,10 @@ defmodule Mix.Tasks.DepsGitTest do
       # Update the project configuration. It should force an update.
       refresh deps: [{:git_repo, "0.1.0", git: fixture_path("git_repo"), ref: last}]
 
+      Mix.Tasks.Deps.run []
+      msg = "  lock outdated: the lock is outdated compared to the options in your mixfile"
+      assert_received {:mix_shell, :info, [^msg]}
+
       # Check an update was triggered
       Mix.Tasks.Deps.Get.run []
       assert File.read!("mix.lock") =~ last
@@ -258,7 +255,7 @@ defmodule Mix.Tasks.DepsGitTest do
     end
   end
 
-  test "does not load bad mix files on get" do
+  test "does not load bad mixfiles on get" do
     Mix.Project.push GitApp
     [last, _, bad|_] = get_git_repo_revs
 
@@ -274,7 +271,7 @@ defmodule Mix.Tasks.DepsGitTest do
     purge [GitRepo, GitRepo.Mix]
   end
 
-  test "does not load bad mix files on update" do
+  test "does not load bad mixfiles on update" do
     Mix.Project.push GitApp
     [last, _, bad|_] = get_git_repo_revs
 

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

@@ -15,20 +15,12 @@ defmodule Mix.Tasks.DepsPathTest do
     end
   end
 
-  test "marks for compilation across environments on get/update" do
+  test "does not mark for compilation on get/update" do
     Mix.Project.push DepsApp
 
     in_fixture "deps_status", fn ->
-      File.mkdir_p!("_build/dev/lib/raw_repo")
-      File.mkdir_p!("_build/test/lib/raw_repo")
-
       Mix.Tasks.Deps.Get.run ["--all"]
-      assert File.exists?("_build/dev/lib/raw_repo/.compile")
-      assert File.exists?("_build/test/lib/raw_repo/.compile")
-
-      Mix.Tasks.Run.run ["-e", "Mix.shell.info RawRepo.hello"]
-      assert_received {:mix_shell, :info, ["==> raw_repo"]}
-      assert_received {:mix_shell, :info, ["world"]}
+      refute File.exists?("custom/raw_repo/.fetch")
     end
   end