Bring back support for Git :ref and :depth used together (#13713)

An optimization to fetch only the about-to-be-used refs introduced when
the :depth option was added broke support for specifying a SHA1 prefix
as :ref. Such possibility was neither documented nor tested, but
certainly used in the wild. This use case is now explicitly supported
with a new test case.

We maintain this fetch optimization whenever :branch, :tag or :depth is
specified. We fetch all refs when :ref is used without :depth to be able
to resolve short SHA1 refs, and require full commit hashes to use :ref
and :depth together.

When using :ref with :depth, a full SHA1 is required by Git and surfaced
in our docs. We enforce it when validating the options, leaving room for
longer hashes.

This commit brings back two tests to cover the :ref and :depth
functionality, and adds a new one to cover the validation mentioned
above.

Author: rhcarvalho

lib/mix/lib/mix/scm/git.ex

@@ -9,7 +9,7 @@ defmodule Mix.SCM.Git do
 
   @impl true
   def format(opts) do
-    if rev = opts[:ref] || opts[:branch] || opts[:tag] do
+    if rev = get_opts_rev(opts) do
       "#{redact_uri(opts[:git])} - #{rev}"
     else
       redact_uri(opts[:git])
@@ -126,17 +126,17 @@ defmodule Mix.SCM.Git do
     update_origin(opts[:git])
 
     # Fetch external data
-    branch_or_tag = opts[:branch] || opts[:tag]
+    rev = get_lock_rev(opts[:lock], opts) || get_opts_rev(opts)
 
     ["--git-dir=.git", "fetch", "--force", "--quiet"]
     |> Kernel.++(progress_switch(git_version()))
     |> Kernel.++(tags_switch(opts[:tag]))
     |> Kernel.++(depth_switch(opts[:depth]))
-    |> Kernel.++(if branch_or_tag, do: ["origin", branch_or_tag], else: [])
+    |> Kernel.++(refspec_switch(opts, rev))
     |> git!()
 
     # Migrate the Git repo
-    rev = get_lock_rev(opts[:lock], opts) || opts[:ref] || branch_or_tag || default_branch()
+    rev = rev || default_branch()
     git!(["--git-dir=.git", "checkout", "--quiet", rev])
 
     if opts[:submodules] do
@@ -219,6 +219,15 @@ defmodule Mix.SCM.Git do
     ["--depth=#{n}"]
   end
 
+  defp refspec_switch(_opts, nil), do: []
+
+  defp refspec_switch(opts, rev) do
+    case Keyword.take(opts, [:depth, :branch, :tag]) do
+      [_ | _] -> ["origin", rev]
+      _ -> []
+    end
+  end
+
   ## Helpers
 
   defp validate_git_options(opts) do
@@ -249,25 +258,30 @@ defmodule Mix.SCM.Git do
     end
   end
 
+  @sha1_size 40
+
   defp validate_depth(opts) do
-    case Keyword.take(opts, [:depth, :ref]) do
-      [_, _] ->
-        Mix.raise(
-          "Cannot specify :depth and :ref at the same time. " <>
-            "Error on Git dependency: #{redact_uri(opts[:git])}"
-        )
+    case Keyword.take(opts, [:depth]) do
+      [] ->
+        opts
+
+      [{:depth, depth}] when is_integer(depth) and depth > 0 ->
+        ref = opts[:ref]
+
+        if ref && byte_size(ref) < @sha1_size do
+          Mix.raise(
+            "When :depth is used with :ref, a full commit hash is required. " <>
+              "Error on Git dependency: #{redact_uri(opts[:git])}"
+          )
+        end
 
-      [depth: depth] when is_integer(depth) and depth > 0 ->
         opts
 
-      [depth: invalid_depth] ->
+      invalid_depth ->
         Mix.raise(
           "The depth must be a positive integer, and be specified only once, got: #{inspect(invalid_depth)}. " <>
             "Error on Git dependency: #{redact_uri(opts[:git])}"
         )
-
-      _ ->
-        opts
     end
   end
 
@@ -296,6 +310,10 @@ defmodule Mix.SCM.Git do
     end
   end
 
+  defp get_opts_rev(opts) do
+    opts[:branch] || opts[:ref] || opts[:tag]
+  end
+
   defp redact_uri(git) do
     case URI.parse(git) do
       %{userinfo: nil} -> git

lib/mix/lib/mix/tasks/deps.ex

@@ -125,8 +125,8 @@ defmodule Mix.Tasks.Deps do
     * `:depth` *(since v1.17.0)* - creates a shallow clone of the Git repository,
       limiting the history to the specified number of commits. This can significantly
       improve clone speed for large repositories when full history is not needed.
-      The value must be a positive integer, typically `1`. Cannot be used with the
-      `:ref` option.
+      The value must be a positive integer, typically `1`. When using `:depth` with
+      `:ref`, a fully spelled hex object name (a 40-character SHA-1 hash) is required.
 
   If your Git repository requires authentication, such as basic username:password
   HTTP authentication via URLs, it can be achieved via Git configuration, keeping

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

@@ -478,6 +478,22 @@ defmodule Mix.Tasks.DepsGitTest do
     purge([GitRepo, GitRepo.MixProject])
   end
 
+  test "fetches with short ref" do
+    [<<short_sha1::binary-size(8), _::binary>>, _ | _] = get_git_repo_revs("git_repo")
+
+    Process.put(:git_repo_opts, ref: short_sha1)
+
+    in_fixture("no_mixfile", fn ->
+      Mix.Project.push(GitApp)
+
+      Mix.Tasks.Deps.Get.run([])
+      message = "* Getting git_repo (#{fixture_path("git_repo")} - #{short_sha1})"
+      assert_received {:mix_shell, :info, [^message]}
+
+      refute_received {:mix_shell, :error, _}
+    end)
+  end
+
   describe "Git depth option" do
     @describetag :git_depth
 
@@ -530,6 +546,66 @@ defmodule Mix.Tasks.DepsGitTest do
       end)
     end
 
+    test "with ref" do
+      [sha1, _ | _] = get_git_repo_revs("git_repo")
+
+      Process.put(:git_repo_opts, depth: 1, ref: sha1)
+
+      in_fixture("no_mixfile", fn ->
+        Mix.Project.push(GitApp)
+
+        Mix.Tasks.Deps.Get.run([])
+        message = "* Getting git_repo (#{fixture_path("git_repo")} - #{sha1})"
+        assert_received {:mix_shell, :info, [^message]}
+        assert_shallow("deps/git_repo", 1)
+      end)
+    end
+
+    test "raises with short ref" do
+      # When fetching, Git requires a fully spelled hex object name. We prevent
+      # this failure mode by validating the ref length.
+      #
+      # If Git ever changes such that it can resolve a short ref in a shallow
+      # fetch, we can update our docs and this test to reflect that.
+      #
+      # https://git-scm.com/docs/git-fetch#Documentation/git-fetch.txt-ltrefspecgt
+      # https://stackoverflow.com/a/43136160
+
+      [<<short_sha1::binary-size(8), _::binary>>, _ | _] = get_git_repo_revs("git_repo")
+
+      Process.put(:git_repo_opts, depth: 1, ref: short_sha1)
+
+      in_fixture("no_mixfile", fn ->
+        Mix.Project.push(GitApp)
+
+        exception = assert_raise Mix.Error, fn -> Mix.Tasks.Deps.Get.run([]) end
+
+        assert Exception.message(exception) =~ "a full commit hash is required"
+      end)
+    end
+
+    test "changing refspec updates retaining depth" do
+      [last, first | _] = get_git_repo_revs("git_repo")
+
+      Process.put(:git_repo_opts, ref: first, depth: 1)
+
+      in_fixture("no_mixfile", fn ->
+        Mix.Project.push(GitApp)
+
+        Mix.Tasks.Deps.Get.run([])
+        message = "* Getting git_repo (#{fixture_path("git_repo")} - #{first})"
+        assert_received {:mix_shell, :info, [^message]}
+        assert_shallow("deps/git_repo", 1)
+        assert File.read!("mix.lock") =~ first
+
+        # Change refspec
+        update_dep(ref: last, depth: 1)
+        Mix.Tasks.Deps.Get.run([])
+        assert_shallow("deps/git_repo", 1)
+        assert File.read!("mix.lock") =~ last
+      end)
+    end
+
     test "removing depth retains shallow repository" do
       # For compatibility and simplicity, we follow Git's behavior and do not
       # attempt to unshallow an existing repository. This should not be a