Move module writing from mix to compiler (#9149)

5 files changed, 89 insertions, 88 deletions

diff --git a/lib/elixir/lib/kernel/parallel_compiler.ex b/lib/elixir/lib/kernel/parallel_compiler.ex
index b6a91ceae..b51c064d0 100644
--- a/lib/elixir/lib/kernel/parallel_compiler.ex
+++ b/lib/elixir/lib/kernel/parallel_compiler.ex
@@ -72,6 +72,8 @@ defmodule Kernel.ParallelCompiler do
       they are loaded into memory. If you want a file to actually be written to
       `dest`, use `compile_to_path/3` instead.
 
+    * `:beam_timestamp` - the modification timestamp to give all BEAM files
+
   """
   @doc since: "1.6.0"
   def compile(files, options \\ []) when is_list(options) do
@@ -139,6 +141,7 @@ defmodule Kernel.ParallelCompiler do
         each_file: Keyword.get(options, :each_file, fn _, _ -> :ok end) |> each_file(),
         each_long_compilation: Keyword.get(options, :each_long_compilation, fn _file -> :ok end),
         each_module: Keyword.get(options, :each_module, fn _file, _module, _binary -> :ok end),
+        beam_timestamp: Keyword.get(options, :beam_timestamp),
         output: output,
         long_compilation_threshold: Keyword.get(options, :long_compilation_threshold, 15),
         schedulers: schedulers
@@ -218,25 +221,9 @@ defmodule Kernel.ParallelCompiler do
 
         try do
           case output do
-            {:compile, path} ->
-              :erlang.process_flag(:error_handler, Kernel.ErrorHandler)
-              :erlang.put(:elixir_compiler_dest, path)
-              :elixir_compiler.file_to_path(file, path, &each_file(&1, &2, parent))
-
-            :compile ->
-              :erlang.process_flag(:error_handler, Kernel.ErrorHandler)
-              :erlang.put(:elixir_compiler_dest, dest)
-              :elixir_compiler.file(file, &each_file(&1, &2, parent))
-
-            :require ->
-              case :elixir_code_server.call({:acquire, file}) do
-                :required ->
-                  send(parent, {:file_cancel, self()})
-
-                :proceed ->
-                  :elixir_compiler.file(file, &each_file(&1, &2, parent))
-                  :elixir_code_server.cast({:required, file})
-              end
+            {:compile, path} -> compile_file(file, path, parent)
+            :compile -> compile_file(file, dest, parent)
+            :require -> require_file(file, parent)
           end
         catch
           kind, reason ->
@@ -253,9 +240,11 @@ defmodule Kernel.ParallelCompiler do
 
   # No more queue, nothing waiting, this cycle is done
   defp spawn_workers([], 0, [], [], result, warnings, state) do
+    %{output: output, beam_timestamp: beam_timestamp} = state
+
     case state.each_cycle.() do
       [] ->
-        modules = for {{:module, mod}, _} <- result, do: mod
+        modules = write_module_binaries(output, beam_timestamp, result)
         warnings = Enum.reverse(warnings)
         {:ok, modules, warnings}
 
@@ -311,6 +300,37 @@ defmodule Kernel.ParallelCompiler do
     wait_for_messages([], spawned, waiting, files, result, warnings, state)
   end
 
+  defp compile_file(file, path, parent) do
+    :erlang.process_flag(:error_handler, Kernel.ErrorHandler)
+    :erlang.put(:elixir_compiler_dest, path)
+    :elixir_compiler.file(file, &each_file(&1, &2, parent))
+  end
+
+  defp require_file(file, parent) do
+    case :elixir_code_server.call({:acquire, file}) do
+      :required ->
+        send(parent, {:file_cancel, self()})
+
+      :proceed ->
+        :elixir_compiler.file(file, &each_file(&1, &2, parent))
+        :elixir_code_server.cast({:required, file})
+    end
+  end
+
+  defp write_module_binaries({:compile, path}, timestamp, result) do
+    for {{:module, module}, binary} <- result do
+      full_path = Path.join(path, Atom.to_string(module) <> ".beam")
+      File.write!(full_path, binary)
+      if timestamp, do: File.touch!(full_path, timestamp)
+
+      module
+    end
+  end
+
+  defp write_module_binaries(_output, _timestamp, result) do
+    for {{:module, module}, _} <- result, do: module
+  end
+
   # The goal of this function is to find leaves in the dependency graph,
   # i.e. to find code that depends on code that we know is not being defined.
   defp without_definition(waiting, files) do
@@ -357,7 +377,7 @@ defmodule Kernel.ParallelCompiler do
               do: {ref, :found}
 
         cancel_waiting_timer(files, child)
-        result = Map.put(result, {:module, module}, true)
+        result = Map.put(result, {:module, module}, binary)
         spawn_workers(available ++ queue, spawned, waiting, files, result, warnings, state)
 
       # If we are simply requiring files, we do not add to waiting.
diff --git a/lib/elixir/test/elixir/kernel/cli_test.exs b/lib/elixir/test/elixir/kernel/cli_test.exs
index f63222f28..72f6479cf 100644
--- a/lib/elixir/test/elixir/kernel/cli_test.exs
+++ b/lib/elixir/test/elixir/kernel/cli_test.exs
@@ -147,7 +147,7 @@ defmodule Kernel.CLI.CompileTest do
       output = elixirc(compilation_args)
 
       expected =
-        "(File.Error) could not write to #{inspect(context[:beam_file_path])}: permission denied"
+        "(File.Error) could not write to file #{inspect(context[:beam_file_path])}: permission denied"
 
       assert String.contains?(output, expected)
     end
diff --git a/lib/mix/lib/mix/compilers/elixir.ex b/lib/mix/lib/mix/compilers/elixir.ex
index f0dc58ac8..abdb71649 100644
--- a/lib/mix/lib/mix/compilers/elixir.ex
+++ b/lib/mix/lib/mix/compilers/elixir.ex
@@ -1,11 +1,11 @@
 defmodule Mix.Compilers.Elixir do
   @moduledoc false
 
-  @manifest_vsn 2
+  @manifest_vsn 3
 
   import Record
 
-  defrecord :module, [:module, :kind, :sources, :beam, :binary, :struct]
+  defrecord :module, [:module, :kind, :sources, :struct]
 
   defrecord :source,
     source: nil,
@@ -52,8 +52,8 @@ defmodule Mix.Compilers.Elixir do
         # A config, path dependency or manifest has changed, let's just compile everything
         all_paths = MapSet.to_list(all_paths)
 
-        for module(module: module, beam: beam) <- all_modules,
-            do: remove_and_purge(beam, module)
+        for module(module: module) <- all_modules,
+            do: remove_and_purge(beam_path(dest, module), module)
 
         sources_stats =
           for path <- all_paths,
@@ -94,7 +94,8 @@ defmodule Mix.Compilers.Elixir do
             all_sources,
             removed ++ changed,
             stale_local_deps,
-            stale_local_deps
+            stale_local_deps,
+            dest
           )
 
         {modules, structs, changed, sources_stats}
@@ -116,7 +117,7 @@ defmodule Mix.Compilers.Elixir do
       # We need to return ok if stale_local_deps changed
       # because we want that to propagate to compile.protocols
       removed != [] or stale_local_deps != %{} ->
-        write_manifest(manifest, modules, sources, dest, timestamp)
+        write_manifest(manifest, modules, sources, timestamp)
         {:ok, warning_diagnostics(sources)}
 
       true ->
@@ -136,8 +137,8 @@ defmodule Mix.Compilers.Elixir do
   Removes compiled files for the given `manifest`.
   """
   def clean(manifest, compile_path) do
-    Enum.each(read_manifest(manifest, compile_path), fn
-      module(beam: beam) -> File.rm(beam)
+    Enum.each(read_manifest(manifest), fn
+      module(module: module) -> File.rm(beam_path(compile_path, module))
       _ -> :ok
     end)
   end
@@ -146,21 +147,21 @@ defmodule Mix.Compilers.Elixir do
   Returns protocols and implementations for the given `manifest`.
   """
   def protocols_and_impls(manifest, compile_path) do
-    for module(beam: beam, module: module, kind: kind) <- read_manifest(manifest, compile_path),
+    for module(module: module, kind: kind) <- read_manifest(manifest),
         match?(:protocol, kind) or match?({:impl, _}, kind),
-        do: {module, kind, beam}
+        do: {module, kind, beam_path(compile_path, module)}
   end
 
   @doc """
   Reads the manifest.
   """
-  def read_manifest(manifest, compile_path) do
+  def read_manifest(manifest) do
     try do
       manifest |> File.read!() |> :erlang.binary_to_term()
     rescue
       _ -> []
     else
-      [@manifest_vsn | data] -> expand_beam_paths(data, compile_path)
+      [@manifest_vsn | data] -> data
       _ -> []
     end
   end
@@ -179,21 +180,21 @@ defmodule Mix.Compilers.Elixir do
     verbose = opts[:verbose] || false
 
     compile_opts = [
-      each_cycle: &each_cycle/0,
+      each_cycle: fn -> each_cycle(dest) end,
       each_file: &each_file(&1, &2, cwd, verbose),
       each_module: &each_module(&1, &2, &3, cwd),
       each_long_compilation: &each_long_compilation(&1, cwd, long_compilation_threshold),
       long_compilation_threshold: long_compilation_threshold,
-      dest: dest
+      beam_timestamp: timestamp
     ]
 
     try do
-      Kernel.ParallelCompiler.compile(stale, compile_opts)
+      Kernel.ParallelCompiler.compile_to_path(stale, dest, compile_opts)
     else
       {:ok, _, warnings} ->
         {modules, _structs, sources, _pending_modules, _pending_structs} = get_compiler_info()
         sources = apply_warnings(sources, warnings)
-        write_manifest(manifest, modules, sources, dest, timestamp)
+        write_manifest(manifest, modules, sources, timestamp)
         {:ok, warning_diagnostics(sources)}
 
       {:error, errors, warnings} ->
@@ -217,11 +218,11 @@ defmodule Mix.Compilers.Elixir do
     |> Code.compiler_options()
   end
 
-  defp each_cycle() do
+  defp each_cycle(compile_path) do
     {modules, _structs, sources, pending_modules, pending_structs} = get_compiler_info()
 
     {pending_modules, structs, changed} =
-      update_stale_entries(pending_modules, sources, [], %{}, pending_structs)
+      update_stale_entries(pending_modules, sources, [], %{}, pending_structs, compile_path)
 
     if changed == [] do
       []
@@ -237,7 +238,7 @@ defmodule Mix.Compilers.Elixir do
     end
   end
 
-  defp each_module(file, module, binary, cwd) do
+  defp each_module(file, module, _binary, cwd) do
     {modules, structs, sources, pending_modules, pending_structs} = get_compiler_info()
     kind = detect_kind(module)
     file = Path.relative_to(file, cwd)
@@ -278,9 +279,7 @@ defmodule Mix.Compilers.Elixir do
         module: module,
         kind: kind,
         sources: module_sources,
-        beam: nil,
-        struct: struct,
-        binary: binary
+        struct: struct
       )
 
     modules = prepend_or_merge(modules, module, module(:module), module, existing_module?)
@@ -387,14 +386,14 @@ 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 update_stale_entries(modules, _sources, [], stale_files, stale_structs)
+  defp update_stale_entries(modules, _sources, [], stale_files, stale_structs, _compile_path)
        when stale_files == %{} and stale_structs == %{} do
     {modules, %{}, []}
   end
 
-  defp update_stale_entries(modules, sources, changed, stale_files, stale_structs) do
+  defp update_stale_entries(modules, sources, changed, stale_files, stale_structs, compile_path) do
     changed = Enum.into(changed, %{}, &{&1, true})
-    reducer = &remove_stale_entry(&1, &2, sources, stale_structs)
+    reducer = &remove_stale_entry(&1, &2, sources, stale_structs, compile_path)
     remove_stale_entries(modules, %{}, changed, stale_files, reducer)
   end
 
@@ -409,8 +408,9 @@ defmodule Mix.Compilers.Elixir do
     end
   end
 
-  defp remove_stale_entry(entry, {rest, structs, changed, stale}, sources, stale_structs) do
-    module(module: module, beam: beam, sources: source_files, struct: struct) = entry
+  defp remove_stale_entry(entry, acc, sources, stale_structs, compile_path) do
+    module(module: module, sources: source_files, struct: struct) = entry
+    {rest, structs, changed, stale} = acc
 
     {compile_references, struct_references, runtime_references} =
       Enum.reduce(source_files, {[], [], []}, fn file, {compile_acc, struct_acc, runtime_acc} ->
@@ -429,7 +429,7 @@ defmodule Mix.Compilers.Elixir do
       # I need to be recompiled.
       has_any_key?(changed, source_files) or has_any_key?(stale, compile_references) or
           has_any_key?(stale_structs, struct_references) ->
-        remove_and_purge(beam, module)
+        remove_and_purge(beam_path(compile_path, module), module)
         changed = Enum.reduce(source_files, changed, &Map.put(&2, &1, true))
         {rest, Map.put(structs, module, struct), changed, Map.put(stale, module, true)}
 
@@ -512,11 +512,11 @@ defmodule Mix.Compilers.Elixir do
         {[], []}
     else
       [@manifest_vsn | data] ->
-        split_manifest(data, compile_path)
+        split_manifest(data)
 
       [v | data] when is_integer(v) ->
         for module <- data, is_record(module, :module) do
-          File.rm(Path.join(compile_path, module(module, :beam)))
+          File.rm(beam_path(compile_path, module(module, :module)))
           :code.purge(module(module, :module))
           :code.delete(module(module, :module))
         end
@@ -528,50 +528,24 @@ defmodule Mix.Compilers.Elixir do
     end
   end
 
-  defp split_manifest(data, compile_path) do
+  defp split_manifest(data) do
     Enum.reduce(data, {[], []}, fn
       module() = module, {modules, sources} ->
-        {[expand_beam_path(module, compile_path) | modules], sources}
+        {[module | modules], sources}
 
       source() = source, {modules, sources} ->
         {modules, [source | sources]}
     end)
   end
 
-  defp expand_beam_path(module(beam: beam) = module, compile_path) do
-    module(module, beam: Path.join(compile_path, beam))
-  end
-
-  defp expand_beam_paths(modules, ""), do: modules
-
-  defp expand_beam_paths(modules, compile_path) do
-    Enum.map(modules, fn
-      module() = module -> expand_beam_path(module, compile_path)
-      other -> other
-    end)
-  end
-
-  defp write_manifest(manifest, [], [], _compile_path, _timestamp) do
+  defp write_manifest(manifest, [], [], _timestamp) do
     File.rm(manifest)
     :ok
   end
 
-  defp write_manifest(manifest, modules, sources, compile_path, timestamp) do
+  defp write_manifest(manifest, modules, sources, timestamp) do
     File.mkdir_p!(Path.dirname(manifest))
 
-    modules =
-      for module(binary: binary, module: module) = entry <- modules do
-        beam = Atom.to_string(module) <> ".beam"
-
-        if binary do
-          beam_path = Path.join(compile_path, beam)
-          File.write!(beam_path, binary)
-          File.touch!(beam_path, timestamp)
-        end
-
-        module(entry, binary: nil, beam: beam)
-      end
-
     manifest_data =
       [@manifest_vsn | modules ++ sources]
       |> :erlang.term_to_binary([:compressed])
@@ -584,4 +558,8 @@ defmodule Mix.Compilers.Elixir do
     # is properly stored.
     Mix.Dep.ElixirSCM.update()
   end
+
+  defp beam_path(compile_path, module) do
+    Path.join(compile_path, Atom.to_string(module) <> ".beam")
+  end
 end
diff --git a/lib/mix/lib/mix/compilers/test.ex b/lib/mix/lib/mix/compilers/test.ex
index 3824823eb..6fb92a13f 100644
--- a/lib/mix/lib/mix/compilers/test.ex
+++ b/lib/mix/lib/mix/compilers/test.ex
@@ -187,12 +187,15 @@ defmodule Mix.Compilers.Test do
     [elixir_manifest] = Mix.Tasks.Compile.Elixir.manifests()
 
     if Mix.Utils.stale?([elixir_manifest], [test_manifest]) do
+      compile_path = Mix.Project.compile_path()
+
       elixir_manifest_entries =
-        CE.read_manifest(elixir_manifest, Mix.Project.compile_path())
+        CE.read_manifest(elixir_manifest)
         |> Enum.group_by(&elem(&1, 0))
 
       stale_modules =
-        for CE.module(module: module, beam: beam) <- elixir_manifest_entries.module,
+        for CE.module(module: module) <- elixir_manifest_entries.module,
+            beam = Path.join(compile_path, Atom.to_string(module) <> ".beam"),
             Mix.Utils.stale?([beam], [test_manifest]),
             do: module,
             into: MapSet.new()
diff --git a/lib/mix/lib/mix/tasks/xref.ex b/lib/mix/lib/mix/tasks/xref.ex
index 6b9e1b657..18f6dcedf 100644
--- a/lib/mix/lib/mix/tasks/xref.ex
+++ b/lib/mix/lib/mix/tasks/xref.ex
@@ -2,7 +2,7 @@ defmodule Mix.Tasks.Xref do
   use Mix.Task
 
   import Mix.Compilers.Elixir,
-    only: [read_manifest: 2, source: 0, source: 1, source: 2, module: 1]
+    only: [read_manifest: 1, source: 0, source: 1, source: 2, module: 1]
 
   @shortdoc "Performs cross reference checks"
   @recursive true
@@ -218,7 +218,7 @@ defmodule Mix.Tasks.Xref do
   def calls(opts \\ []) do
     module_sources =
       for manifest <- manifests(opts),
-          manifest_data = read_manifest(manifest, ""),
+          manifest_data = read_manifest(manifest),
           module(module: module, sources: sources) <- manifest_data,
           source <- sources,
           source = Enum.find(manifest_data, &match?(source(source: ^source), &1)),
@@ -564,7 +564,7 @@ defmodule Mix.Tasks.Xref do
 
     module_sources =
       for manifest_path <- manifests(opts),
-          manifest_data = read_manifest(manifest_path, ""),
+          manifest_data = read_manifest(manifest_path),
           module(module: module, sources: sources) <- manifest_data,
           source <- sources,
           source = Enum.find(manifest_data, &match?(source(source: ^source), &1)),
@@ -743,7 +743,7 @@ defmodule Mix.Tasks.Xref do
 
   defp sources(opts) do
     for manifest <- manifests(opts),
-        source() = source <- read_manifest(manifest, ""),
+        source() = source <- read_manifest(manifest),
         do: source
   end