Topsort Erlang modules before compilation, closes #15100

Author: josevalim

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

@@ -34,36 +34,17 @@ defmodule Mix.Dep.Converger do
           Enum.find(deps, fn %Mix.Dep{app: other_app} -> app == other_app end)
         end)
       else
-        find_cycle(graph)
+        Mix.raise(
+          "Could not sort dependencies. " <>
+            "The following dependencies form a cycle: " <>
+            Enum.join(Mix.Utils.find_cycle!(graph), ", ")
+        )
       end
     after
       :digraph.delete(graph)
     end
   end
 
-  @doc """
-  A Depth-First Search to find where is the dependency graph cycle
-  and then display the cyclic dependencies back to the developer.
-  """
-  def find_cycle(graph), do: find_cycle(graph, :digraph.vertices(graph), MapSet.new())
-
-  defp find_cycle(_, [], _visited), do: nil
-
-  defp find_cycle(graph, [v | vs], visited) do
-    if v in visited, do: cycle_found(visited)
-
-    find_cycle(graph, :digraph.out_neighbours(graph, v), MapSet.put(visited, v))
-    find_cycle(graph, vs, visited)
-  end
-
-  defp cycle_found(visited) do
-    Mix.raise(
-      "Could not sort dependencies. " <>
-        "The following dependencies form a cycle: " <>
-        Enum.join(visited, ", ")
-    )
-  end
-
   @doc """
   Converge without lock and accumulator updates.
 

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

@@ -83,13 +83,12 @@ defmodule Mix.Tasks.Compile.Erlang do
       end)
 
     compile_path = Path.relative_to(compile_path, File.cwd!())
+    erls = scan_sources(files, include_path, source_paths, compile_path, opts)
 
-    {erls, tuples} =
-      Enum.unzip(scan_sources(files, include_path, source_paths, compile_path, opts))
+    {sorted, parallel} = topsort_and_parallelize(erls, compile_path)
+    opts = [parallel: MapSet.new(parallel)] ++ opts
 
-    opts = [parallel: MapSet.new(find_parallel(erls))] ++ opts
-
-    Erlang.compile_entries(manifest(), tuples, :erl, :beam, opts, fn input, _output ->
+    Erlang.compile_entries(manifest(), sorted, :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()
@@ -147,7 +146,7 @@ defmodule Mix.Tasks.Compile.Erlang do
       ordered: false
     )
     |> Enum.flat_map(fn
-      {:ok, {:ok, erl_file, target_tuple}} -> [{erl_file, target_tuple}]
+      {:ok, {:ok, erl_file}} -> [erl_file]
       {:ok, :error} -> []
     end)
   end
@@ -156,17 +155,15 @@ defmodule Mix.Tasks.Compile.Erlang do
     erl_file = %{
       file: file,
       module: module_from_artifact(file),
-      behaviours: [],
-      compile: [],
+      deps: [],
       includes: [],
-      invalid: false
+      status: :ok
     }
 
     case :epp.parse_file(Erlang.to_erl_file(file), include_paths, []) do
       {:ok, forms} ->
         erl_file = List.foldl(tl(forms), erl_file, &do_form(file, &1, &2))
-        target_tuple = annotate_target(erl_file, compile_path, opts[:force])
-        {:ok, erl_file, target_tuple}
+        {:ok, maybe_stale(erl_file, compile_path, opts[:force])}
 
       {:error, _error} ->
         :error
@@ -183,49 +180,71 @@ defmodule Mix.Tasks.Compile.Erlang do
         end
 
       {:attribute, _, :behaviour, behaviour} ->
-        %{erl | behaviours: [behaviour | erl.behaviours]}
+        %{erl | deps: [behaviour | erl.deps]}
 
       {:attribute, _, :behavior, behaviour} ->
-        %{erl | behaviours: [behaviour | erl.behaviours]}
+        %{erl | deps: [behaviour | erl.deps]}
 
       {:attribute, _, :compile, value} when is_list(value) ->
-        %{erl | compile: value ++ erl.compile}
+        %{erl | deps: Enum.reduce(value, erl.deps, &add_parse_transforms/2)}
 
       {:attribute, _, :compile, value} ->
-        %{erl | compile: [value | erl.compile]}
+        %{erl | deps: add_parse_transforms(value, erl.deps)}
 
       _ ->
         erl
     end
   end
 
-  defp find_parallel(erls) do
-    serial = MapSet.new(find_dependencies(erls))
+  defp topsort_and_parallelize(erls, compile_path) do
+    graph = :digraph.new()
 
-    erls
-    |> Enum.reject(&(&1.module in serial))
-    |> Enum.map(& &1.file)
-  end
+    for %{module: module, status: status, file: file} <- erls do
+      :digraph.add_vertex(graph, module, {file, status})
+    end
+
+    for %{module: module, deps: deps} <- erls, dep <- deps do
+      # It may error if the behaviour/parse transform is not in the project,
+      # which is expected
+      :digraph.add_edge(graph, module, dep)
+    end
 
-  defp find_dependencies(erls) do
-    Enum.flat_map(erls, fn erl ->
-      transforms =
-        Enum.flat_map(erl.compile, fn
-          {:parse_transform, transform} -> [transform]
-          _ -> []
+    if vertices = :digraph_utils.topsort(graph) do
+      sorted =
+        Enum.reduce(vertices, [], fn module, acc ->
+          {_, {file, status}} = :digraph.vertex(graph, module)
+          [{status, file, Path.join(compile_path, "#{module}.beam")} | acc]
         end)
 
-      transforms ++ erl.behaviours
+      parallel =
+        for %{file: file, module: module} <- erls,
+            :digraph.in_neighbours(graph, module) == [],
+            do: file
+
+      {sorted, parallel}
+    else
+      Mix.raise(
+        "Could not compile Erlang. " <>
+          "The following modules form a cycle: " <>
+          Enum.join(Mix.Utils.find_cycle!(graph), ", ")
+      )
+    end
+  end
+
+  defp add_parse_transforms(compile, deps) do
+    Enum.reduce(compile, deps, fn
+      {:parse_transform, transform}, deps -> [transform | deps]
+      _, deps -> deps
     end)
   end
 
-  defp annotate_target(erl, compile_path, force) do
+  defp maybe_stale(erl, compile_path, force) do
     beam = Path.join(compile_path, "#{erl.module}.beam")
 
     if force || Mix.Utils.stale?([erl.file | erl.includes], [beam]) do
-      {:stale, erl.file, beam}
+      %{erl | status: :stale}
     else
-      {:ok, erl.file, beam}
+      erl
     end
   end
 

lib/mix/lib/mix/utils.ex

@@ -1049,4 +1049,22 @@ defmodule Mix.Utils do
   defp to_erl_tail([h | t]), do: [?,, to_erl_term(h) | to_erl_tail(t)]
   defp to_erl_tail([]), do: []
   defp to_erl_tail(other), do: [?|, to_erl_term(other)]
+
+  @doc """
+  A Depth-First Search to find where is the dependency graph cycle
+  and then display the cyclic dependencies back to the developer.
+  """
+  def find_cycle!(graph),
+    do: find_cycle(graph, :digraph.vertices(graph), MapSet.new()) || raise("no cycle found")
+
+  defp find_cycle(_, [], _visited), do: nil
+
+  defp find_cycle(graph, [v | vs], visited) do
+    if v in visited do
+      visited
+    else
+      find_cycle(graph, :digraph.out_neighbours(graph, v), MapSet.put(visited, v)) ||
+        find_cycle(graph, vs, visited)
+    end
+  end
 end

lib/mix/test/fixtures/compile_erlang/src/b.erl

@@ -1,7 +1,7 @@
 -module(b).
--export([b/0]).
+-export([z/0]).
 
--callback c() -> term().
+-callback b() -> term().
 -record(br, {cell=undefined}).
 
-b() -> #br{cell=specified}.
+z() -> #br{cell=specified}.

lib/mix/test/fixtures/compile_erlang/src/c.erl

@@ -1,7 +1,7 @@
 -module(c).
--export([c/0]).
+-export([b/0]).
 
 -include("r.hrl").
 -behaviour(b).
 
-c() -> #r{cell=specified}.
+b() -> #r{cell=specified}.

lib/mix/test/fixtures/compile_erlang/src/z.erl

@@ -0,0 +1,3 @@
+%% Have modules flowing in both directions to check for topsort
+-module(z).
+-callback z() -> term().

lib/mix/test/mix/tasks/compile.erlang_test.exs

@@ -28,25 +28,29 @@ defmodule Mix.Tasks.Compile.ErlangTest do
     end)
   end
 
-  test "compiles and cleans src/b.erl and src/c.erl" do
+  test "compiles and cleans src/b.erl, src/c.erl, and src/z.erl" do
     in_fixture("compile_erlang", fn ->
       assert Mix.Tasks.Compile.Erlang.run(["--verbose"]) == {:ok, []}
       assert_received {:mix_shell, :info, ["Compiled src/b.erl"]}
       assert_received {:mix_shell, :info, ["Compiled src/c.erl"]}
+      assert_received {:mix_shell, :info, ["Compiled src/z.erl"]}
 
       assert File.regular?("_build/dev/lib/sample/ebin/b.beam")
       assert File.regular?("_build/dev/lib/sample/ebin/c.beam")
+      assert File.regular?("_build/dev/lib/sample/ebin/z.beam")
 
       assert Mix.Tasks.Compile.Erlang.run(["--verbose"]) == {:noop, []}
       refute_received {:mix_shell, :info, ["Compiled src/b.erl"]}
 
       assert Mix.Tasks.Compile.Erlang.run(["--force", "--verbose"]) == {:ok, []}
       assert_received {:mix_shell, :info, ["Compiled src/b.erl"]}
       assert_received {:mix_shell, :info, ["Compiled src/c.erl"]}
+      assert_received {:mix_shell, :info, ["Compiled src/z.erl"]}
 
       assert Mix.Tasks.Compile.Erlang.clean()
       refute File.regular?("_build/dev/lib/sample/ebin/b.beam")
       refute File.regular?("_build/dev/lib/sample/ebin/c.beam")
+      refute File.regular?("_build/dev/lib/sample/ebin/z.beam")
     end)
   end
 
@@ -182,53 +186,16 @@ defmodule Mix.Tasks.Compile.ErlangTest do
 
       ensure_touched(file)
 
-      output =
-        capture_io(fn ->
-          Mix.Tasks.Compile.Erlang.run(["--all-warnings"])
-        end)
-
-      assert output == ""
-    end)
-  end
-
-  test "returns syntax error from an Erlang file when --return-errors is set" do
-    in_fixture("no_mixfile", fn ->
-      import ExUnit.CaptureIO
-
-      file = Path.absname("src/a.erl")
-      source = deterministic_source(file)
-
-      File.mkdir!("src")
-
-      File.write!(file, """
-      -module(b).
-      def b(), do: b
-      """)
-
-      capture_io(fn ->
-        assert {:error, [diagnostic]} =
-                 Mix.Tasks.Compile.Erlang.run(["--force", "--return-errors"])
-
-        assert %Mix.Task.Compiler.Diagnostic{
-                 compiler_name: "erl_parse",
-                 file: ^source,
-                 source: ^source,
-                 message: "syntax error before: b",
-                 position: position(2, 5),
-                 severity: :error
-               } = diagnostic
-      end)
-
-      refute File.regular?("ebin/Elixir.A.beam")
-      refute File.regular?("ebin/Elixir.B.beam")
+      assert capture_io(fn ->
+               Mix.Tasks.Compile.Erlang.run(["--all-warnings"])
+             end) == ""
     end)
   end
 
   @tag erlc_options: [{:warnings_as_errors, true}]
   test "adds :debug_info to erlc_options by default" do
     in_fixture("compile_erlang", fn ->
       Mix.Tasks.Compile.Erlang.run([])
-
       binary = File.read!("_build/dev/lib/sample/ebin/b.beam")
 
       {:ok, {_, [debug_info: {:debug_info_v1, _, {debug_info, _}}]}} =