Notify modules diff from compile.erlang (#13904)

Author: jonatanklosko

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

@@ -207,19 +207,17 @@ defmodule Mix.Compilers.Elixir do
             modules_diff(modules, removed_modules, all_modules, timestamp)
           end
 
-          unless_previous_warnings_as_errors(
-            previous_warnings,
-            opts,
-            {:ok, all_warnings, lazy_modules_diff}
-          )
+          Mix.Task.Compiler.notify_modules_compiled(lazy_modules_diff)
+
+          unless_previous_warnings_as_errors(previous_warnings, opts, {:ok, all_warnings})
 
         {:error, errors, %{runtime_warnings: r_warnings, compile_warnings: c_warnings}, state} ->
           # In case of errors, we show all previous warnings and all new ones.
           {_, _, sources, _, _, _} = state
           errors = Enum.map(errors, &diagnostic/1)
           warnings = Enum.map(r_warnings ++ c_warnings, &diagnostic/1)
           all_warnings = Keyword.get(opts, :all_warnings, errors == [])
-          {:error, previous_warnings(sources, all_warnings) ++ warnings ++ errors, nil}
+          {:error, previous_warnings(sources, all_warnings) ++ warnings ++ errors}
       after
         Code.compiler_options(previous_opts)
       end
@@ -257,11 +255,7 @@ defmodule Mix.Compilers.Elixir do
       all_warnings = Keyword.get(opts, :all_warnings, true)
       previous_warnings = previous_warnings(sources, all_warnings)
 
-      unless_previous_warnings_as_errors(
-        previous_warnings,
-        opts,
-        {status, previous_warnings, nil}
-      )
+      unless_previous_warnings_as_errors(previous_warnings, opts, {status, previous_warnings})
     end
   end
 
@@ -1003,17 +997,13 @@ defmodule Mix.Compilers.Elixir do
     File.rm(manifest <> ".checkpoint")
   end
 
-  defp unless_previous_warnings_as_errors(
-         previous_warnings,
-         opts,
-         {status, all_warnings, modules_diff}
-       ) do
+  defp unless_previous_warnings_as_errors(previous_warnings, opts, {status, all_warnings}) do
     if previous_warnings != [] and opts[:warnings_as_errors] do
       message = "Compilation failed due to warnings while using the --warnings-as-errors option"
       IO.puts(:stderr, message)
-      {:error, all_warnings, modules_diff}
+      {:error, all_warnings}
     else
-      {status, all_warnings, modules_diff}
+      {status, all_warnings}
     end
   end
 

lib/mix/lib/mix/compilers/erlang.ex

@@ -54,7 +54,8 @@ defmodule Mix.Compilers.Erlang do
   `{:error, errors, warnings}` in case of error. This function returns
   `{status, diagnostics}` as specified in `Mix.Task.Compiler`.
   """
-  def compile(manifest, mappings, src_ext, dest_ext, opts, callback) when is_list(opts) do
+  def compile(manifest, mappings, src_ext, dest_ext, opts, callback)
+      when is_atom(src_ext) and is_atom(dest_ext) and is_list(opts) do
     force = opts[:force]
 
     entries =
@@ -66,40 +67,26 @@ defmodule Mix.Compilers.Erlang do
       preload.()
     end
 
-    compile(manifest, entries, src_ext, opts, callback)
+    compile_entries(manifest, entries, src_ext, dest_ext, opts, callback)
   end
 
-  @doc """
-  Compiles the given `entries`.
-
-  `entries` are a list of `{:ok | :stale, src, dest}` tuples.
-
-  A `manifest` file and a `callback` to be invoked for each stale
-  src/dest pair must also be given.
-
-  ## Options
-
-    * `:force` - forces compilation regardless of modification times
-
-    * `:parallel` - a mapset of files to compile in parallel
-
-  """
+  # TODO: remove me on v1.22
+  @deprecated "Use compile/6 or open an up an issue"
   def compile(manifest, entries, opts \\ [], callback) do
-    compile(manifest, entries, :erl, opts, callback)
+    compile_entries(manifest, entries, :erl, :beam, opts, callback)
   end
 
-  defp compile(manifest, mappings, ext, opts, callback) do
+  @doc false
+  def compile_entries(manifest, mappings, src_ext, dest_ext, opts, callback) do
     stale = for {:stale, src, dest} <- mappings, do: {src, dest}
 
     # Get the previous entries from the manifest
     timestamp = System.os_time(:second)
-    entries = read_manifest(manifest)
+    old_entries = entries = read_manifest(manifest)
 
     # Files to remove are the ones in the manifest but they no longer have a source
     removed =
-      entries
-      |> Enum.filter(fn {dest, _} -> not List.keymember?(mappings, dest, 2) end)
-      |> Enum.map(&elem(&1, 0))
+      for {dest, _} <- entries, not List.keymember?(mappings, dest, 2), do: dest
 
     # Remove manifest entries with no source
     Enum.each(removed, &File.rm/1)
@@ -116,7 +103,7 @@ defmodule Mix.Compilers.Erlang do
     if stale == [] and removed == [] do
       {:noop, manifest_warnings(entries)}
     else
-      Mix.Utils.compiling_n(length(stale), ext)
+      Mix.Utils.compiling_n(length(stale), src_ext)
       Mix.Project.ensure_structure()
 
       # Let's prepend the newly created path so compiled files
@@ -151,6 +138,19 @@ defmodule Mix.Compilers.Erlang do
       # Return status and diagnostics
       warnings = manifest_warnings(entries) ++ to_diagnostics(warnings, :warning)
 
+      if dest_ext == :beam do
+        lazy_modules_diff = fn ->
+          {changed, added} =
+            stale
+            |> Enum.map(&elem(&1, 1))
+            |> Enum.split_with(fn dest -> List.keymember?(old_entries, dest, 0) end)
+
+          modules_diff(added, changed, removed, timestamp)
+        end
+
+        Mix.Task.Compiler.notify_modules_compiled(lazy_modules_diff)
+      end
+
       case status do
         :ok ->
           {:ok, warnings}
@@ -320,4 +320,21 @@ defmodule Mix.Compilers.Erlang do
   defp location_to_string({line, column}), do: "#{line}:#{column}:"
   defp location_to_string(0), do: ""
   defp location_to_string(line), do: "#{line}:"
+
+  defp modules_diff(added, changed, removed, timestamp) do
+    %{
+      added: modules_from_paths(added),
+      changed: modules_from_paths(changed),
+      removed: modules_from_paths(removed),
+      timestamp: timestamp
+    }
+  end
+
+  defp modules_from_paths(paths) do
+    Enum.map(paths, &module_from_path/1)
+  end
+
+  defp module_from_path(path) do
+    path |> Path.basename() |> Path.rootname() |> String.to_atom()
+  end
 end

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

@@ -127,22 +127,15 @@ defmodule Mix.Tasks.Compile.Elixir do
 
     with_logger_app(project, fn ->
       Mix.Project.with_build_lock(project, fn ->
-        {status, warnings, lazy_modules_diff} =
-          Mix.Compilers.Elixir.compile(
-            manifest,
-            srcs,
-            dest,
-            cache_key,
-            Mix.Tasks.Compile.Erlang.manifests(),
-            Mix.Tasks.Compile.Erlang.modules(),
-            opts
-          )
-
-        if lazy_modules_diff do
-          Mix.Task.Compiler.notify_modules_compiled(lazy_modules_diff)
-        end
-
-        {status, warnings}
+        Mix.Compilers.Elixir.compile(
+          manifest,
+          srcs,
+          dest,
+          cache_key,
+          Mix.Tasks.Compile.Erlang.manifests(),
+          Mix.Tasks.Compile.Erlang.modules(),
+          opts
+        )
       end)
     end)
   end

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

@@ -84,7 +84,7 @@ defmodule Mix.Tasks.Compile.Erlang do
 
     opts = [parallel: MapSet.new(find_parallel(erls))] ++ opts
 
-    Erlang.compile(manifest(), tuples, opts, fn input, _output ->
+    Erlang.compile_entries(manifest(), tuples, :erl, :beam, opts, fn input, _output ->
       # We're purging the module because a previous compiler (for example, Phoenix)
       # might have already loaded the previous version of it.
       module = input |> Path.basename(".erl") |> String.to_atom()

lib/mix/test/mix/sync/pubsub_test.exs

@@ -75,16 +75,16 @@ defmodule Mix.Sync.PubSubTest do
       %{event: "event1"}
     end
 
-    PubSub.broadcast(@pubsub_key, lazy_message)
+    PubSub.broadcast([@pubsub_key, "lazy"], lazy_message)
 
-    PubSub.subscribe(@pubsub_key)
+    PubSub.subscribe([@pubsub_key, "lazy"])
 
     lazy_message = fn ->
       send(self(), :lazy2)
       %{event: "event2"}
     end
 
-    PubSub.broadcast(@pubsub_key, lazy_message)
+    PubSub.broadcast([@pubsub_key, "lazy"], lazy_message)
 
     refute_received :lazy1
     assert_received :lazy2

lib/mix/test/mix/tasks/compile_test.exs

@@ -415,6 +415,12 @@ defmodule Mix.Tasks.CompileTest do
       File.write!("lib/b.ex", "defmodule B do end")
       File.write!("lib/c.ex", "defmodule C do end")
 
+      File.mkdir_p!("src")
+
+      File.write!("src/a.erl", "-module(a).")
+      File.write!("src/b.erl", "-module(b).")
+      File.write!("src/c.erl", "-module(c).")
+
       mix(["deps.compile"])
 
       parent = self()
@@ -433,8 +439,9 @@ defmodule Mix.Tasks.CompileTest do
         assert_receive {^port, {:data, "ok\n"}}, timeout
         send(parent, :mix_started)
 
-        assert_receive {^port, {:data, output}}, timeout
-        send(parent, {:output, output})
+        assert_receive {^port, {:data, output_erl}}, timeout
+        assert_receive {^port, {:data, output_ex}}, timeout
+        send(parent, {:output, output_erl <> output_ex})
       end)
 
       assert_receive :mix_started, timeout
@@ -445,6 +452,11 @@ defmodule Mix.Tasks.CompileTest do
       assert_receive {:output, output}, timeout
 
       assert output == """
+             Received :modules_compiled with
+               added: [:a, :b, :c], changed: [], removed: []
+               app: :with_reloader
+               build_scm: Mix.SCM.Path
+               os_pid: "#{os_pid}"
              Received :modules_compiled with
                added: [A, B, C], changed: [], removed: []
                app: :with_reloader
@@ -454,10 +466,14 @@ defmodule Mix.Tasks.CompileTest do
 
       # Changed
       File.write!("lib/a.ex", "defmodule A do @moduledoc false end")
+      File.write!("src/a.erl", "-module(a). -export([a/0]). a() -> ok.")
+      File.touch!("src/a.erl", System.os_time(:second) + 10)
       # Removed
       File.rm!("lib/b.ex")
+      File.rm!("src/b.erl")
       # New
       File.write!("lib/d.ex", "defmodule D do end")
+      File.write!("src/d.erl", "-module(d).")
 
       spawn_link(fn ->
         port =
@@ -473,8 +489,9 @@ defmodule Mix.Tasks.CompileTest do
         assert_receive {^port, {:data, "ok\n"}}, timeout
         send(parent, :mix_started)
 
-        assert_receive {^port, {:data, output}}, timeout
-        send(parent, {:output, output})
+        assert_receive {^port, {:data, output_erl}}, timeout
+        assert_receive {^port, {:data, output_ex}}, timeout
+        send(parent, {:output, output_erl <> output_ex})
       end)
 
       assert_receive :mix_started, timeout
@@ -485,6 +502,11 @@ defmodule Mix.Tasks.CompileTest do
       assert_receive {:output, output}, timeout
 
       assert output == """
+             Received :modules_compiled with
+               added: [:d], changed: [:a], removed: [:b]
+               app: :with_reloader
+               build_scm: Mix.SCM.Path
+               os_pid: "#{os_pid}"
              Received :modules_compiled with
                added: [D], changed: [A], removed: [B]
                app: :with_reloader