Rely on modification time and hash to determine modified sources (#11080)

Author: marcandre

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

@@ -1,7 +1,7 @@
 defmodule Mix.Compilers.Elixir do
   @moduledoc false
 
-  @manifest_vsn 8
+  @manifest_vsn 9
 
   import Record
 
@@ -10,6 +10,7 @@ defmodule Mix.Compilers.Elixir do
   defrecord :source,
     source: nil,
     size: 0,
+    digest: nil,
     compile_references: [],
     export_references: [],
     runtime_references: [],
@@ -206,12 +207,13 @@ defmodule Mix.Compilers.Elixir do
     # Sources that have changed on disk or
     # any modules associated with them need to be recompiled
     changed =
-      for source(source: source, external: external, size: size, modules: modules) <-
+      for source(source: source, external: external, size: size, digest: digest, modules: modules) <-
             all_sources,
           {last_mtime, last_size} = Map.fetch!(sources_stats, source),
           times = Enum.map(external, &(sources_stats |> Map.fetch!(&1) |> elem(0))),
-          size != last_size or Mix.Utils.stale?([last_mtime | times], [modified]) or
-            Enum.any?(modules, &Map.has_key?(modules_to_recompile, &1)),
+          Enum.any?(modules, &Map.has_key?(modules_to_recompile, &1)) or
+            Mix.Utils.stale?(times, [modified]) or
+            (size != last_size or (last_mtime > modified and digest != digest(source))),
           do: source
 
     changed = new_paths ++ changed
@@ -246,6 +248,12 @@ defmodule Mix.Compilers.Elixir do
     end)
   end
 
+  defp digest(file) do
+    file
+    |> File.read!()
+    |> :erlang.md5()
+  end
+
   defp compile_path(stale, dest, timestamp, opts) do
     cwd = File.cwd!()
     long_compilation_threshold = opts[:long_compilation_threshold] || 10
@@ -460,6 +468,8 @@ defmodule Mix.Compilers.Elixir do
     source =
       source(
         source,
+        # We preserve the digest if the file is recompiled but not changed
+        digest: source(source, :digest) || digest(file),
         compile_references: compile_references,
         export_references: export_references,
         runtime_references: runtime_references,
@@ -509,11 +519,11 @@ defmodule Mix.Compilers.Elixir do
   # to be recompiled (but were not changed on disk)
   defp update_stale_sources(sources, changed) do
     Enum.reduce(changed, {sources, %{}}, fn file, {acc_sources, acc_modules} ->
-      {source(size: size, modules: modules), acc_sources} =
+      {source(size: size, digest: digest, modules: modules), acc_sources} =
         List.keytake(acc_sources, file, source(:source))
 
       acc_modules = Enum.reduce(modules, acc_modules, &Map.put(&2, &1, true))
-      {[source(source: file, size: size) | acc_sources], acc_modules}
+      {[source(source: file, size: size, digest: digest) | acc_sources], acc_modules}
     end)
   end
 

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

@@ -195,7 +195,7 @@ defmodule Mix.Tasks.Compile.ElixirTest do
     end)
   end
 
-  test "compiles mtime changed files" do
+  test "compiles mtime changed files if content changed but not length" do
     in_fixture("no_mixfile", fn ->
       assert Mix.Tasks.Compile.Elixir.run(["--verbose"]) == {:ok, []}
       assert_received {:mix_shell, :info, ["Compiled lib/a.ex"]}
@@ -204,6 +204,8 @@ defmodule Mix.Tasks.Compile.ElixirTest do
       Mix.shell().flush
       purge([A, B])
 
+      same_length_content = "lib/a.ex" |> File.read!() |> String.replace("A", "Z")
+      File.write!("lib/a.ex", same_length_content)
       future = {{2038, 1, 1}, {0, 0, 0}}
       File.touch!("lib/a.ex", future)
       Mix.Tasks.Compile.Elixir.run(["--verbose"])
@@ -225,6 +227,36 @@ defmodule Mix.Tasks.Compile.ElixirTest do
     end)
   end
 
+  test "does not recompile mtime changed but identical files" do
+    in_fixture("no_mixfile", fn ->
+      assert Mix.Tasks.Compile.Elixir.run(["--verbose"]) == {:ok, []}
+      assert_received {:mix_shell, :info, ["Compiled lib/a.ex"]}
+      assert_received {:mix_shell, :info, ["Compiled lib/b.ex"]}
+
+      Mix.shell().flush
+      purge([A, B])
+
+      future = {{2038, 1, 1}, {0, 0, 0}}
+      File.touch!("lib/a.ex", future)
+      Mix.Tasks.Compile.Elixir.run(["--verbose"])
+
+      message =
+        "warning: mtime (modified time) for \"lib/a.ex\" was set to the future, resetting to now"
+
+      assert_received {:mix_shell, :error, [^message]}
+
+      message =
+        "warning: mtime (modified time) for \"lib/b.ex\" was set to the future, resetting to now"
+
+      refute_received {:mix_shell, :error, [^message]}
+      refute_received {:mix_shell, :info, ["Compiled lib/a.ex"]}
+      refute_received {:mix_shell, :info, ["Compiled lib/b.ex"]}
+
+      File.touch!("_build/dev/lib/sample/.mix/compile.elixir", future)
+      assert Mix.Tasks.Compile.Elixir.run([]) == {:noop, []}
+    end)
+  end
+
   test "compiles size changed files" do
     in_fixture("no_mixfile", fn ->
       past = {{2010, 1, 1}, {0, 0, 0}}
@@ -257,8 +289,7 @@ defmodule Mix.Tasks.Compile.ElixirTest do
       Mix.shell().flush
       purge([A, B])
 
-      future = {{2038, 1, 1}, {0, 0, 0}}
-      File.touch!("lib/b.ex", future)
+      force_recompilation("lib/b.ex")
       Mix.Tasks.Compile.Elixir.run(["--verbose"])
 
       assert_received {:mix_shell, :info, ["Compiled lib/a.ex"]}
@@ -283,7 +314,7 @@ defmodule Mix.Tasks.Compile.ElixirTest do
 
       Code.put_compiler_option(:ignore_module_conflict, true)
       Code.compile_file("lib/b.ex")
-      File.touch!("lib/a.ex", {{2038, 1, 1}, {0, 0, 0}})
+      force_recompilation("lib/a.ex")
 
       Mix.Tasks.Compile.Elixir.run(["--verbose"])
       assert_received {:mix_shell, :info, ["Compiled lib/a.ex"]}
@@ -472,8 +503,7 @@ defmodule Mix.Tasks.Compile.ElixirTest do
       assert_received {:mix_shell, :info, ["Compiled lib/b.ex"]}
       purge([A, B])
 
-      future = {{2038, 1, 1}, {0, 0, 0}}
-      File.touch!("lib/a.ex", future)
+      force_recompilation("lib/a.ex")
 
       assert Mix.Tasks.Compile.Elixir.run(["--verbose"]) == {:ok, []}
       assert_received {:mix_shell, :info, ["Compiled lib/a.ex"]}
@@ -615,8 +645,7 @@ defmodule Mix.Tasks.Compile.ElixirTest do
       Mix.shell().flush
       purge([A, B])
 
-      future = {{2038, 1, 1}, {0, 0, 0}}
-      File.touch!("lib/a.ex", future)
+      force_recompilation("lib/a.ex")
       Mix.Tasks.Compile.Elixir.run(["--verbose"])
 
       assert_received {:mix_shell, :info, ["Compiled lib/a.ex"]}

lib/mix/test/mix/tasks/test_test.exs

@@ -68,12 +68,12 @@ defmodule Mix.Tasks.TestTest do
         assert_stale_run_output("2 tests, 0 failures")
 
         set_all_mtimes()
-        File.touch!("lib/b.ex")
+        force_recompilation("lib/b.ex")
 
         assert_stale_run_output("1 test, 0 failures")
 
         set_all_mtimes()
-        File.touch!("lib/a.ex")
+        force_recompilation("lib/a.ex")
 
         assert_stale_run_output("2 tests, 0 failures")
       end)

lib/mix/test/test_helper.exs

@@ -179,6 +179,10 @@ defmodule MixTest.Case do
     ])
   end
 
+  def force_recompilation(file) do
+    File.write!(file, File.read!(file) <> "\n")
+  end
+
   defp mix_executable do
     Path.expand("../../../bin/mix", __DIR__)
   end