Fetch all refs unless using Git :depth option

Reverts to the behavior from Elixir/Mix 1.16 when the :depth option
didn't exist.

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.

When using :ref with :depth, a full SHA1 is required by Git and surfaced
in our docs. For now, we don't enforce it when validating the options to
avoid unnecessary coupling with current Git behavior -- if a new Git
client version and a server supports resolving short refs, or if the
repo is using a different hash algorithm altogether (e.g. SHA256), `mix`
will not prevent it from being used.

Elixir maintainers may become aware of such changes through a new test
case that verifies the currently observed behavior.

This commit also brings back support for :ref and :depth being used
together (with the caveat above on :ref), along with two tests that
cover that functionality.

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,16 @@ 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.++(depth_switch(opts[:depth], 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
@@ -212,11 +211,11 @@ defmodule Mix.SCM.Git do
   defp tags_switch(nil), do: []
   defp tags_switch(_), do: ["--tags"]
 
-  defp depth_switch(nil), do: []
+  defp depth_switch(nil, _rev), do: []
 
-  defp depth_switch(n) when is_integer(n) and n > 0 do
+  defp depth_switch(n, rev) when is_integer(n) and n > 0 do
     check_depth_support(git_version())
-    ["--depth=#{n}"]
+    ["--depth=#{n}"] ++ if rev, do: ["origin", rev], else: []
   end
 
   ## Helpers
@@ -250,24 +249,18 @@ defmodule Mix.SCM.Git do
   end
 
   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 ->
+      [{: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 +289,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,67 @@ 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 "fails to find short ref" do
+      # Failure to resolve a short ref depends on Git behavior.
+      #
+      # 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. For now,
+      # when fetching a specific refspec, Git requires a fully spelled hex
+      # object name as the <src>.
+      #
+      # 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) =~ "fatal: couldn't find remote ref"
+      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