Do not fetch references when the need commit is there already

alibuild_helpers/git.py

@@ -76,6 +76,12 @@ def checkoutCmd(self, tag):
   def fetchCmd(self, remote, *refs):
     return ["fetch", "-f", "--prune"] + clone_speedup_options() + [remote, *refs]
 
+  def hasCommitCmd(self, commit):
+    # -e is silent and just sets the exit code. The ^{commit} peel matters: a
+    # bare cat-file -e succeeds for any object, so a hash that happens to name
+    # a blob or a tree would look like a usable commit.
+    return ["cat-file", "-e", commit + "^{commit}"]
+
   def setWriteUrlCmd(self, url):
     return ["remote", "set-url", "--push", "origin", url]
 

alibuild_helpers/scm.py

@@ -19,6 +19,8 @@ def checkoutCmd(self, tag):
     raise NotImplementedError
   def fetchCmd(self, remote, *refs):
     raise NotImplementedError
+  def hasCommitCmd(self, commit):
+    raise NotImplementedError
   def cloneReferenceCmd(self, spec, referenceRepo, usePartialClone):
     raise NotImplementedError
   def cloneSourceCmd(self, source, destination, referenceRepo, usePartialClone):

alibuild_helpers/workarea.py

@@ -2,15 +2,46 @@
 import errno
 import os
 import os.path
+import re
 import shutil
 import tempfile
 from collections import OrderedDict
 
 from alibuild_helpers.log import dieOnError, debug, error
+from alibuild_helpers.scm import SCMError
 from alibuild_helpers.utilities import call_ignoring_oserrors, symlink, short_commit_hash, asList
 
 FETCH_LOG_NAME = "fetch-log.txt"
 
+#: A tag that is a full commit id cannot move: it names its own content. 40 hex
+#: digits in a SHA-1 repository, 64 in a SHA-256 one.
+IMMUTABLE_TAG_RE = re.compile(r"[0-9a-fA-F]{40}(?:[0-9a-fA-F]{24})?\Z")
+
+
+def has_pinned_commit(scm, spec, referenceRepo):
+  """True if the spec pins a commit the reference repo already has.
+
+  Fetching then cannot discover anything. The reason to refresh a reference
+  repo at all is that a tag may have been moved, and a commit id cannot be
+  moved -- so if the object is already here, the fetch has nothing to find.
+
+  Worth skipping rather than merely tolerating, because that fetch is not free:
+  it asks the upstream for every ref in the repository, on every round, on
+  every builder. For Eigen3 -- pinned to a commit, and mirrored from gitlab.com
+  rather than from one of the alisw GitHub mirrors -- that is the request most
+  likely to be throttled, and a 403 there is fatal to the whole build.
+  """
+  tag = str(spec.get("tag", ""))
+  if not IMMUTABLE_TAG_RE.match(tag):
+    return False
+  try:
+    err, _ = scm.exec(scm.hasCommitCmd(tag), directory=referenceRepo,
+                      check=False, prompt=False)
+  except (NotImplementedError, SCMError, OSError):
+    # An SCM that cannot answer the question gets the old behaviour.
+    return False
+  return err == 0
+
 
 def cleanup_git_log(referenceSources):
   """Remove a stale fetch-log.txt.
@@ -112,6 +143,9 @@ def updateReferenceRepo(referenceSources, p, spec,
   if not os.path.exists(referenceRepo):
     cmd = scm.cloneReferenceCmd(spec["source"], referenceRepo, usePartialClone)
     logged_scm(scm, p, referenceSources, cmd, ".", allowGitPrompt)
+  elif fetch and has_pinned_commit(scm, spec, referenceRepo):
+    debug("%s is pinned to commit %s, which %s already has: not fetching",
+          p, spec["tag"], referenceRepo)
   elif fetch:
     ref_match_rule = asList(spec.get("ref_match_rule", ["+refs/tags/*:refs/tags/*", "+refs/heads/*:refs/heads/*"]))
     cmd = scm.fetchCmd(spec["source"], *ref_match_rule)

tests/test_workarea.py

@@ -119,6 +119,68 @@ def test_reference_sources_created(self, mock_git, mock_makedirs, mock_exists):
         ], directory=".", check=False, prompt=True)
         self.assertEqual(spec.get("reference"), "%s/sw/MIRROR/aliroot" % getcwd())
 
+    @patch("os.path.exists")
+    @patch("os.makedirs")
+    @patch("codecs.open")
+    @patch("alibuild_helpers.git.git")
+    @patch("alibuild_helpers.workarea.is_writeable", new=MagicMock(return_value=True))
+    def test_pinned_commit_present_is_not_fetched(self, mock_git, mock_open,
+                                                  mock_makedirs, mock_exists):
+        """A tag that is a commit id cannot move, so a present object needs no fetch.
+
+        The point of refreshing a mirror is to notice a moved tag. A commit id
+        names its own content, so there is nothing to notice, and the fetch is
+        pure cost -- it asks upstream for every ref, on every round, on every
+        builder, and upstreams throttle that.
+        """
+        mock_exists.return_value = True
+        mock_git.return_value = 0, ""    # cat-file -e succeeds: the commit is here
+        spec = MOCK_SPEC.copy()
+        spec["tag"] = "e7248b26a1ed53fa030c5c459f7ea095dfd276ac"
+        updateReferenceRepoSpec(referenceSources="sw/MIRROR", p="AliRoot",
+                                spec=spec, fetch=True)
+        # called ONCE: the existence check, and no fetch after it
+        mock_git.assert_called_once_with(
+            ["cat-file", "-e", spec["tag"] + "^{commit}"],
+            directory="%s/sw/MIRROR/aliroot" % getcwd(), check=False, prompt=False)
+        self.assertEqual(spec.get("reference"), "%s/sw/MIRROR/aliroot" % getcwd())
+
+    @patch("os.path.exists")
+    @patch("os.makedirs")
+    @patch("codecs.open")
+    @patch("alibuild_helpers.git.git")
+    @patch("alibuild_helpers.workarea.is_writeable", new=MagicMock(return_value=True))
+    def test_pinned_commit_missing_is_fetched(self, mock_git, mock_open,
+                                              mock_makedirs, mock_exists):
+        """A pinned commit we do NOT have still has to be fetched."""
+        mock_exists.return_value = True
+        mock_git.side_effect = [(1, "not found"), (0, "sentinel output")]
+        spec = MOCK_SPEC.copy()
+        spec["tag"] = "0" * 40
+        updateReferenceRepoSpec(referenceSources="sw/MIRROR", p="AliRoot",
+                                spec=spec, fetch=True)
+        self.assertEqual(mock_git.call_count, 2)
+        self.assertEqual(mock_git.call_args_list[-1][0][0][0], "fetch")
+
+    @patch("os.path.exists")
+    @patch("os.makedirs")
+    @patch("codecs.open")
+    @patch("alibuild_helpers.git.git")
+    @patch("alibuild_helpers.workarea.is_writeable", new=MagicMock(return_value=True))
+    def test_movable_tag_is_always_fetched(self, mock_git, mock_open,
+                                           mock_makedirs, mock_exists):
+        """A named tag can be moved, so it is fetched without even asking."""
+        mock_exists.return_value = True
+        mock_git.return_value = 0, "sentinel output"
+        spec = MOCK_SPEC.copy()
+        spec["tag"] = "v3.4.0"
+        updateReferenceRepoSpec(referenceSources="sw/MIRROR", p="AliRoot",
+                                spec=spec, fetch=True)
+        mock_git.assert_called_once_with([
+            "fetch", "-f", "--prune", "--filter=blob:none", spec["source"],
+            "+refs/tags/*:refs/tags/*", "+refs/heads/*:refs/heads/*",
+        ], directory="%s/sw/MIRROR/aliroot" % getcwd(), check=False, prompt=True)
+
 
 if __name__ == '__main__':
     unittest.main()