diff --git a/alibuild_helpers/git.py b/alibuild_helpers/git.py index 9596a7ea..dbd4f7f2 100644 --- a/alibuild_helpers/git.py +++ b/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] diff --git a/alibuild_helpers/scm.py b/alibuild_helpers/scm.py index 0ada8502..71559b45 100644 --- a/alibuild_helpers/scm.py +++ b/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): diff --git a/alibuild_helpers/workarea.py b/alibuild_helpers/workarea.py index 31475cea..d727d4fb 100644 --- a/alibuild_helpers/workarea.py +++ b/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) diff --git a/tests/test_workarea.py b/tests/test_workarea.py index f6d38272..bd077537 100644 --- a/tests/test_workarea.py +++ b/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()