Integrate non-fetchable (path) dependencies into compilers

This commit changes Elixir compilers so a path dependencies
no longer forces the "parent" project to recompile. This
means each compiler must know how to track dependencies but
it gives faster compilation times as a benefit.

Signed-off-by: José Valim <[email protected]>

Author: José Valim

lib/mix/lib/mix/compilers/elixir.ex

@@ -20,6 +20,8 @@ defmodule Mix.Compilers.Elixir do
     all  = Mix.Utils.extract_files(keep, exts)
     {all_entries, skip_entries, all_sources, skip_sources} = parse_manifest(manifest, keep)
 
+    modified = Mix.Utils.last_modified(manifest)
+
     removed =
       for {source, _files} <- all_sources,
           not(source in all),
@@ -31,25 +33,25 @@ defmodule Mix.Compilers.Elixir do
         # changed, let's just compile everything
         all
       else
-        modified   = Mix.Utils.last_modified(manifest)
-        all_mtimes = mtimes(all_sources)
+        sources_mtimes = mtimes(all_sources)
 
-        # Otherwise let's start with the new ones
-        # plus the ones that have changed
+        # Otherwise let's start with the new sources
         for(source <- all,
             not Map.has_key?(all_sources, source),
             do: source)
           ++
+        # Plus the sources that have changed in disk
         for({source, files} <- all_sources,
-            times = Enum.map([source|files], &Map.fetch!(all_mtimes, &1)),
+            times = Enum.map([source|files], &Map.fetch!(sources_mtimes, &1)),
             Mix.Utils.stale?(times, [modified]),
             do: source)
       end
 
     sources = update_stale_sources(all_sources, removed, changed)
-    {entries, changed} = remove_stale_entries(all_entries, removed ++ changed)
-    stale = changed -- removed
+    {entries, changed} = update_stale_entries(all_entries, removed ++ changed,
+                                              stale_local_deps(modified))
 
+    stale = changed -- removed
     new_entries = entries ++ skip_entries
     new_sources = Map.merge(sources, skip_sources)
 
@@ -89,9 +91,9 @@ defmodule Mix.Compilers.Elixir do
   Returns protocols and implementations for the given manifest.
   """
   def protocols_and_impls(manifest) do
-    for {_, module, kind, _, _, _, _} <- read_manifest(manifest),
+    for {beam, module, kind, _, _, _, _} <- read_manifest(manifest),
         match?(:protocol, kind) or match?({:impl, _}, kind),
-        do: {module, kind}
+        do: {module, kind, beam}
   end
 
   defp compile_manifest(manifest, entries, sources, stale, dest, on_start) do
@@ -183,12 +185,13 @@ defmodule Mix.Compilers.Elixir do
   # files that have changed. It then, recursively, figures out
   # all the files that changed (via the module dependencies) and
   # return the non-changed entries and the removed sources.
-  defp remove_stale_entries(all, []) do
+  defp update_stale_entries(all, [], stale) when stale == %{} do
     {all, []}
   end
 
-  defp remove_stale_entries(all, changed) do
-    remove_stale_entries(all, %{}, Enum.into(changed, %{}, &{&1, true}))
+  defp update_stale_entries(all, changed, stale) do
+    removed = Enum.into(changed, %{}, &{&1, true})
+    remove_stale_entries(all, stale, removed)
   end
 
   defp remove_stale_entries(entries, old_stale, old_removed) do
@@ -225,6 +228,16 @@ defmodule Mix.Compilers.Elixir do
     end
   end
 
+  defp stale_local_deps(modified) do
+    for %{scm: scm} = dep <- Mix.Dep.children,
+        not scm.fetchable?,
+        path <- Mix.Dep.load_paths(dep),
+        beam <- Path.wildcard(Path.join(path, "*.beam")),
+        Mix.Utils.last_modified(beam) > modified,
+        do: {beam |> Path.basename |> Path.rootname |> String.to_atom, true},
+        into: %{}
+  end
+
   ## Manifest handling
 
   defp read_manifest(manifest) do

lib/mix/lib/mix/tasks/compile.elixir.ex

@@ -80,11 +80,6 @@ defmodule Mix.Tasks.Compile.Elixir do
     Mix.Compilers.Elixir.clean(manifest())
   end
 
-  @doc false
-  def protocols_and_impls do
-    Mix.Compilers.Elixir.protocols_and_impls(manifest())
-  end
-
   defp set_compiler_opts(project, opts, extra) do
     opts = Keyword.take(opts, Code.available_compiler_options)
     opts = Keyword.merge(project[:elixirc_options] || [], opts)

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

@@ -81,10 +81,6 @@ defmodule Mix.Tasks.Compile do
     Mix.Project.get!
     Mix.Task.run "loadpaths", args
 
-    if local_deps_changed?() do
-      Mix.Dep.Lock.touch_manifest
-    end
-
     res = Mix.Task.run "compile.all", args
     res = if :ok in List.wrap(res), do: :ok, else: :noop
 
@@ -105,17 +101,6 @@ defmodule Mix.Tasks.Compile do
     Mix.Project.config[:consolidate_protocols]
   end
 
-  defp local_deps_changed? do
-    manifest = Path.absname(Mix.Dep.Lock.manifest())
-
-    Enum.any?(Mix.Dep.children(), fn %{scm: scm} = dep ->
-      not scm.fetchable? and Mix.Dep.in_dependency(dep, fn _ ->
-        files = Mix.Project.config_files ++ manifests()
-        Mix.Utils.stale?(files, [manifest])
-      end)
-    end)
-  end
-
   @doc """
   Returns all compilers.
   """

lib/mix/lib/mix/tasks/compile.protocols.ex

@@ -2,7 +2,7 @@ defmodule Mix.Tasks.Compile.Protocols do
   use Mix.Task
 
   @manifest ".compile.protocols"
-  @manifest_vsn :v1
+  @manifest_vsn :v2
 
   @moduledoc ~S"""
   Consolidates all protocols in all paths.
@@ -41,11 +41,12 @@ defmodule Mix.Tasks.Compile.Protocols do
     Mix.Task.run "compile", args
     {opts, _, _} = OptionParser.parse(args, switches: [force: :boolean])
 
-    output = default_path()
+    output   = default_path(config)
     manifest = Path.join(output, @manifest)
+
     protocols_and_impls =
       unless Mix.Project.umbrella?(config) do
-        Mix.Tasks.Compile.Elixir.protocols_and_impls
+        protocols_and_impls(config)
       end
 
     cond do
@@ -74,7 +75,24 @@ defmodule Mix.Tasks.Compile.Protocols do
   end
 
   @doc false
-  def default_path, do: Path.join(Mix.Project.build_path, "consolidated")
+  def default_path(config \\ Mix.Project.config) do
+    Path.join(Mix.Project.build_path(config), "consolidated")
+  end
+
+  defp protocols_and_impls(config) do
+    deps = for(%{scm: scm, app: app} <- Mix.Dep.children,
+               not scm.fetchable?,
+               do: app)
+
+    build_path = Path.join(Mix.Project.build_path(config), "lib")
+
+    protocols_and_impls =
+      for app <- [config[:app] | deps],
+          elixir = Path.join([build_path, Atom.to_string(app), ".compile.elixir"]),
+          do: Mix.Compilers.Elixir.protocols_and_impls(elixir)
+
+    Enum.concat(protocols_and_impls)
+  end
 
   defp consolidation_paths do
     filter_otp(:code.get_path, :code.lib_dir)
@@ -134,23 +152,34 @@ defmodule Mix.Tasks.Compile.Protocols do
   end
 
   defp diff_manifest(manifest, new_metadata, output) do
+    modified = Mix.Utils.last_modified(manifest)
     old_metadata = read_manifest(manifest)
 
-    additions =
-      Enum.flat_map(new_metadata -- old_metadata, fn
-        {_, {:impl, protocol}} -> [protocol]
-        {protocol, :protocol} -> [protocol]
+    protocols =
+      for {protocol, :protocol, beam} <- new_metadata,
+          Mix.Utils.last_modified(beam) > modified,
+          remove_consolidated(protocol, output),
+          do: {protocol, true},
+          into: %{}
+
+    protocols =
+      Enum.reduce(new_metadata -- old_metadata, protocols, fn
+        {_, {:impl, protocol}, _beam}, protocols ->
+          Map.put(protocols, protocol, true)
+        {protocol, :protocol, _beam}, protocols ->
+          Map.put(protocols, protocol, true)
       end)
 
-    removals =
-      Enum.flat_map(old_metadata -- new_metadata, fn
-        {_, {:impl, protocol}} -> [protocol]
-        {protocol, :protocol} ->
+    protocols =
+      Enum.reduce(old_metadata -- new_metadata, protocols, fn
+        {_, {:impl, protocol}, _beam}, protocols ->
+          Map.put(protocols, protocol, true)
+        {protocol, :protocol, _beam}, protocols ->
           remove_consolidated(protocol, output)
-          []
+          protocols
       end)
 
-    additions ++ removals
+    Map.keys(protocols)
   end
 
   defp remove_consolidated(protocol, output) do

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

@@ -65,7 +65,7 @@ defmodule Mix.Tasks.Deps.Compile do
       Enum.map(deps, fn %Mix.Dep{app: app, status: status, opts: opts, scm: scm} = dep ->
         check_unavailable!(app, status)
 
-        compiled = cond do
+        compiled? = cond do
           not is_nil(opts[:compile]) ->
             do_compile dep, config
           mix?(dep) ->
@@ -79,20 +79,27 @@ defmodule Mix.Tasks.Deps.Compile do
           true ->
             shell.error "Could not compile #{inspect app}, no \"mix.exs\", \"rebar.config\" or \"Makefile\" " <>
               "(pass :compile as an option to customize compilation, set it to \"false\" to do nothing)"
+            false
         end
 
         unless mix?(dep), do: build_structure(dep, config)
-        touch_fetchable(scm, opts[:build])
-        compiled
+        # We should touch fetchable dependencies even if they
+        # did not compile otherwise they will always be marked
+        # as stale, even when there is nothing to do.
+        fetchable? = touch_fetchable(scm, opts[:build])
+        compiled? and fetchable?
       end)
 
-    if Enum.any?(compiled), do: Mix.Dep.Lock.touch_manifest, else: :ok
+    if true in compiled, do: Mix.Dep.Lock.touch_manifest, else: :ok
   end
 
   defp touch_fetchable(scm, path) do
     if scm.fetchable? do
       File.mkdir_p!(path)
       File.touch!(Path.join(path, ".compile.fetch"))
+      true
+    else
+      false
     end
   end
 

lib/mix/test/mix/umbrella_test.exs

@@ -261,34 +261,72 @@ defmodule Mix.UmbrellaTest do
     in_fixture("umbrella_dep/deps/umbrella/apps", fn ->
       Mix.Project.in_project(:bar, "bar", fn _ ->
         Mix.Task.run "compile"
-        assert Mix.Tasks.Compile.Elixir.run([]) == :noop
         assert_receive {:mix_shell, :info, ["Compiled lib/foo.ex"]}
         assert_receive {:mix_shell, :info, ["Compiled lib/bar.ex"]}
-        assert File.regular?("_build/dev/consolidated/Elixir.Enumerable.beam")
 
-        # Ensure we can measure a timestamp difference
-        ensure_touched("_build/dev/lib/foo/.compile.elixir",
-                       File.stat!("_build/dev/lib/bar/.compile.lock").mtime)
-        ensure_touched("../foo/mix.exs",
-                       File.stat!("_build/dev/lib/foo/.compile.elixir").mtime)
+        # Noop by default
+        assert Mix.Tasks.Compile.Elixir.run([]) == :noop
 
-        # Mark locks and protocols as outdated
-        File.touch!("_build/dev/lib/foo/.compile.elixir",
-                    {{2010, 1, 1}, {0, 0, 0}})
-        File.touch!("_build/dev/consolidated/Elixir.Enumerable.beam",
-                    {{2010, 1, 1}, {0, 0, 0}})
+        # Noop when there is no runtime dependency
+        ensure_touched("_build/dev/lib/foo/ebin/Elixir.Foo.beam",
+                       File.stat!("_build/dev/lib/bar/.compile.elixir").mtime)
+        assert Mix.Tasks.Compile.Elixir.run([]) == :noop
 
-        purge [Foo, Bar]
-        Mix.Task.clear
-        Mix.shell.flush
+        # Add runtime dependency
+        File.write!("lib/bar.ex", """
+        defmodule Bar do
+          def bar, do: Foo.foo
+        end
+        """)
+        assert Mix.Tasks.Compile.Elixir.run([]) == :ok
+        assert_receive {:mix_shell, :info, ["Compiled lib/bar.ex"]}
 
-        assert Mix.Task.run("compile") == :ok
+        # Noop for runtime dependencies
+        ensure_touched("_build/dev/lib/foo/ebin/Elixir.Foo.beam",
+                       File.stat!("_build/dev/lib/bar/.compile.elixir").mtime)
         assert Mix.Tasks.Compile.Elixir.run([]) == :noop
-        assert_receive {:mix_shell, :info, ["Compiled lib/foo.ex"]}
+
+        # Add compile time dependency
+        File.write!("lib/bar.ex", "defmodule Bar, do: Foo.foo")
+        assert Mix.Tasks.Compile.Elixir.run([]) == :ok
+        assert_receive {:mix_shell, :info, ["Compiled lib/bar.ex"]}
+
+        # Recompiles for compile time dependencies
+        ensure_touched("_build/dev/lib/foo/ebin/Elixir.Foo.beam",
+                       File.stat!("_build/dev/lib/bar/.compile.elixir").mtime)
+        assert Mix.Tasks.Compile.Elixir.run([]) == :ok
         assert_receive {:mix_shell, :info, ["Compiled lib/bar.ex"]}
-        assert File.stat!("_build/dev/consolidated/Elixir.Enumerable.beam").mtime >
-               {{2010, 1, 1}, {0, 0, 0}}
-        purge [Foo, Bar]
+      end)
+    end)
+  end
+
+  test "reconsolidates after path dependency changes" do
+    in_fixture("umbrella_dep/deps/umbrella/apps", fn ->
+      Mix.Project.in_project(:bar, "bar", fn _ ->
+        # Add a protocol dependency
+        File.write!("../foo/lib/foo.ex", """
+        defprotocol Foo do
+          def foo(arg)
+        end
+        defimpl Foo, for: List do
+          def foo(list), do: list
+        end
+        """)
+        Mix.Task.run("compile")
+        assert File.regular?("_build/dev/consolidated/Elixir.Foo.beam")
+        assert Mix.Tasks.Compile.Protocols.run([]) == :noop
+
+        # Mark protocol as outdated
+        File.touch!("_build/dev/consolidated/Elixir.Foo.beam",
+                    {{2010, 1, 1}, {0, 0, 0}})
+
+        ensure_touched("_build/dev/lib/foo/ebin/Elixir.Foo.beam",
+                       File.stat!("_build/dev/consolidated/.compile.protocols").mtime)
+        assert Mix.Tasks.Compile.Protocols.run([]) == :ok
+
+        # Check new timestamp
+        assert File.stat!("_build/dev/consolidated/Elixir.Foo.beam").mtime >
+                         {{2010, 1, 1}, {0, 0, 0}}
       end)
     end)
   end