Mark stale fetchable deps for compile before convergence

A fetched git dependency could report {:ok, vsn} from its
stale .app before Mix checked the stored SCM manifest lock.
If the same dependency then appeared again through a stricter
transitive requirement, the converger would mark it as
:divergedreq instead of :compile.

Validate the SCM manifest while loading the dependency so
fetchable deps with a stale stored lock are marked for
compilation before convergence reuses their status.
Add regression tests for the transitive git case and for
the direct converger case.

Closes #15406.

Author: josevalim

lib/mix/lib/mix/dep.ex

@@ -438,7 +438,10 @@ defmodule Mix.Dep do
   Checks the lock for the given dependency and update its status accordingly.
   """
   def check_lock(%Mix.Dep{scm: scm, opts: opts} = dep) do
-    if available?(dep) do
+    # We only update the lock if the dependency is compilable or available.
+    # That's because a dependency may need compilation but if it has the wrong version,
+    # then the version reporting gets higher priority.
+    if available?(dep) or compilable?(dep) do
       case scm.lock_status(opts) do
         :mismatch ->
           status = if rev = opts[:lock], do: {:lockmismatch, rev}, else: :nolock
@@ -449,44 +452,13 @@ defmodule Mix.Dep do
           %{dep | status: :lockoutdated}
 
         :ok ->
-          check_manifest(dep)
+          dep
       end
     else
       dep
     end
   end
 
-  defp check_manifest(%{scm: scm, opts: opts} = dep) do
-    vsn = {System.version(), :erlang.system_info(:otp_release)}
-    lock = opts[:lock]
-
-    case Mix.Dep.ElixirSCM.read(Path.join(opts[:build], ".mix")) do
-      {:ok, old_vsn, _, _} when old_vsn != vsn ->
-        %{dep | status: {:vsnlock, old_vsn}}
-
-      {:ok, _, old_scm, _} when old_scm != scm ->
-        %{dep | status: {:scmlock, old_scm}}
-
-      {:ok, _, _, old_lock} when old_lock != lock ->
-        if scm.fetchable?() do
-          %{dep | status: :compile}
-        else
-          dep
-        end
-
-      :error ->
-        if scm.fetchable?() do
-          %{dep | status: :compile}
-        else
-          dep
-        end
-
-      # If the file is missing, it is handled in the loader
-      _ ->
-        dep
-    end
-  end
-
   @doc """
   Returns `true` if the dependency is ok.
   """

lib/mix/lib/mix/dep/loader.ex

@@ -110,7 +110,9 @@ defmodule Mix.Dep.Loader do
           {dep, []}
       end
 
-    validate_app(%{dep | deps: attach_only_and_targets(children, opts)})
+    %{dep | deps: attach_only_and_targets(children, opts)}
+    |> validate_manifest()
+    |> validate_app()
   end
 
   @doc """
@@ -410,6 +412,33 @@ defmodule Mix.Dep.Loader do
     end
   end
 
+  defp validate_manifest(dep) do
+    if ok?(dep) do
+      %{scm: scm, opts: opts} = dep
+      vsn = {System.version(), :erlang.system_info(:otp_release)}
+      lock = opts[:lock]
+
+      case Mix.Dep.ElixirSCM.read(Path.join(opts[:build], ".mix")) do
+        {:ok, old_vsn, _, _} when old_vsn != vsn ->
+          %{dep | status: {:vsnlock, old_vsn}}
+
+        {:ok, _, old_scm, _} when old_scm != scm ->
+          %{dep | status: {:scmlock, old_scm}}
+
+        {:ok, _, _, old_lock} when old_lock != lock ->
+          if scm.fetchable?(), do: %{dep | status: :compile}, else: dep
+
+        :error ->
+          if scm.fetchable?(), do: %{dep | status: :compile}, else: dep
+
+        _ ->
+          dep
+      end
+    else
+      dep
+    end
+  end
+
   defp app_status(app_path, app, req) do
     case Mix.AppLoader.read_app(app, app_path) do
       {:ok, properties} ->

lib/mix/test/mix/dep_test.exs

@@ -408,6 +408,36 @@ defmodule Mix.DepTest do
     end)
   end
 
+  test "marks fetchable dep for compile when stored lock is stale" do
+    deps = [{:git_repo, "0.1.0", git: MixTest.Case.fixture_path("git_repo")}]
+
+    with_deps(deps, fn ->
+      in_fixture("no_mixfile", fn ->
+        File.mkdir_p!("deps")
+        File.cp_r!(fixture_path("git_repo"), "deps/git_repo")
+
+        File.mkdir_p!("_build/dev/lib/git_repo/ebin")
+        File.mkdir_p!("_build/dev/lib/git_repo/.mix")
+
+        File.write!("_build/dev/lib/git_repo/ebin/git_repo.app", """
+        {application, git_repo, [
+          {vsn,"0.1.0"}
+        ]}.
+        """)
+
+        manifest_data =
+          :erlang.term_to_binary(
+            {2, {System.version(), :erlang.system_info(:otp_release)}, Mix.SCM.Git, :stale_lock}
+          )
+
+        File.write!("_build/dev/lib/git_repo/.mix/compile.elixir_scm", manifest_data)
+
+        [git_repo] = Mix.Dep.Converger.converge([])
+        assert git_repo.status == :compile
+      end)
+    end)
+  end
+
   ## Remote converger
 
   defmodule IdentityRemoteConverger do

lib/mix/test/mix/tasks/deps.git_test.exs

@@ -711,14 +711,81 @@ defmodule Mix.Tasks.DepsGitTest do
         message = "* Getting git_repo (#{fixture_path("git_repo")})"
         assert_received {:mix_shell, :info, [^message]}
         assert_shallow("deps/git_repo", 1)
-
         assert File.read!("mix.lock") =~ "submodules: true"
-        # TODO: assert submodule is not shallow. This would likely require
-        # changes to the fixtures. Apparently, not even the submodules-specific
-        # tests check that the cloned repo contains submodules as expected.
       end)
     end
 
+    test "stale .app for a fetchable dep does not surface as :divergedreq via the converger" do
+      # Reproduces the convergence chain seen in hexpm/hex#1166:
+      #
+      #   1. validate_app reads `_build/.../git_repo.app` (stale vsn 0.1.0)
+      #      against the top-level requirement "0.1.0" and returns
+      #      {:ok, "0.1.0"}.
+      #   2. The converger then encounters git_repo a second time, this
+      #      time as a child of `deps_on_git_repo`, whose requirement
+      #      "~> 0.2.0" does not match the cached {:ok, "0.1.0"}.
+      #      `req_mismatch` fires and the dep is marked :divergedreq.
+      #   3. `show_diverged!` raises before the lock-in-manifest check
+      #      in `check_manifest` gets a chance to flag the build as stale.
+      #
+      # With the loader bypass, step 1 returns :compile instead of
+      # {:ok, _}, `req_mismatch` returns nil, and the dep ends up flagged
+      # for recompile rather than as a spurious requirement conflict.
+      in_fixture("no_mixfile", fn ->
+        File.cp_r!(fixture_path("deps_on_git_repo"), "deps_on_git_repo")
+
+        update_deps_on_git_repo = fn req ->
+          File.write!(
+            "deps_on_git_repo/mix.exs",
+            File.read!("deps_on_git_repo/mix.exs")
+            |> String.replace(
+              ~r/\{:git_repo, (?:".*?", )?git: MixTest\.Case\.fixture_path\("git_repo"\)\}/,
+              "{:git_repo, #{inspect(req)}, git: MixTest.Case.fixture_path(\"git_repo\")}"
+            )
+          )
+        end
+
+        # Bootstrap with a matching requirement so deps.get + compile succeed
+        # and the SCM manifest records the current lock alongside vsn 0.1.0.
+        update_deps_on_git_repo.("0.1.0")
+
+        Mix.ProjectStack.post_config(
+          deps: [
+            {:git_repo, "0.1.0", git: fixture_path("git_repo")},
+            {:deps_on_git_repo, path: "deps_on_git_repo"}
+          ]
+        )
+
+        Mix.Project.push(MixTest.Case.Sample)
+
+        Mix.Tasks.Deps.Get.run([])
+        Mix.Tasks.Deps.Compile.run([])
+
+        # Now simulate the post-fetch state:
+        #   - The transitive parent has moved on to a stricter requirement
+        #     that the build's stale .app vsn ("0.1.0") no longer satisfies.
+        #   - The SCM manifest's stored lock no longer matches opts[:lock],
+        #     signalling that _build is behind the fetched source.
+        update_deps_on_git_repo.("~> 0.2.0")
+
+        manifest = "_build/dev/lib/git_repo/.mix/compile.elixir_scm"
+        {2, vsn, scm, _lock} = manifest |> File.read!() |> :erlang.binary_to_term()
+        File.write!(manifest, :erlang.term_to_binary({2, vsn, scm, :stale_lock}))
+
+        Mix.Task.clear()
+        Mix.State.clear_cache()
+        purge([GitRepo, GitRepo.MixProject, DepsOnGitRepo.MixProject])
+
+        [git_repo_dep] =
+          Mix.Dep.load_and_cache() |> Enum.filter(&(&1.app == :git_repo))
+
+        assert git_repo_dep.status == :compile
+        refute Mix.Dep.diverged?(git_repo_dep)
+      end)
+    after
+      purge([GitRepo, GitRepo.MixProject, DepsOnGitRepo.MixProject])
+    end
+
     defp update_dep(git_repo_opts) do
       # Flush the errors we got, move to a clean slate
       Mix.shell().flush()