diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f614bcce..8cda3bf7 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -19,7 +19,7 @@ jobs: fail-fast: false matrix: python-version: ["3.10", "3.11", "3.12", "3.13", "3.14"] - os: [ubuntu-latest] + os: [ubuntu-24.04] runs-on: ${{ matrix.os }} steps: - uses: actions/checkout@v7 @@ -40,7 +40,7 @@ jobs: fail-fast: false matrix: python-version: ["3.14"] - os: [ubuntu-latest] + os: [ubuntu-24.04] runs-on: ${{ matrix.os }} steps: - uses: actions/checkout@v7 @@ -60,7 +60,7 @@ jobs: fail-fast: false matrix: python-version: ["3.10", "3.11", "3.12", "3.13", "3.14"] - os: [ubuntu-latest] + os: [ubuntu-24.04] runs-on: ${{ matrix.os }} steps: - uses: actions/checkout@v7 @@ -80,7 +80,7 @@ jobs: - name: Start MySQL service run: sudo systemctl start mysql.service - name: Install Mercurial - run: sudo apt-get install -y mercurial + run: sudo apt-get update && sudo apt-get install -y mercurial - name: Test updatebot run: poetry run coverage run test.py - name: Generate coverage metrics diff --git a/components/scmprovider.py b/components/scmprovider.py index f6ab5372..59639c18 100644 --- a/components/scmprovider.py +++ b/components/scmprovider.py @@ -4,6 +4,7 @@ import os import copy +import json import collections import shutil import tempfile @@ -124,9 +125,55 @@ def _contains_commit(list_of_commits, revision): return False +class RelativePreExistingRepoPath(Exception): + pass + + class SCMProvider(BaseProvider, INeedsCommandProvider, INeedsLoggingProvider): def __init__(self, config): - pass + # A JSON file mapping library names to local clones of their upstream + # repositories (e.g. ones baked into the Docker image), which clones can + # borrow objects from instead of downloading them. + self.pre_existing_repos_file = config.get('pre-existing-repos') + self._pre_existing_repos = None + + # We chdir into the clone directory before reading the file or using a path + # from it, so relative paths would resolve against the wrong directory. + def _report_relative_path(self, message): + self.logger.log(message, level=LogLevel.Error) + try: + raise RelativePreExistingRepoPath(message) + except RelativePreExistingRepoPath as e: + self.logger.log_exception(e) + + def _load_pre_existing_repos(self): + if not self.pre_existing_repos_file: + return {} + if not os.path.isabs(self.pre_existing_repos_file): + self._report_relative_path("The pre-existing repositories file %s is not an absolute path, so we're ignoring it." % self.pre_existing_repos_file) + return {} + try: + with open(self.pre_existing_repos_file) as f: + mapping = json.load(f) + if not isinstance(mapping, dict): + raise ValueError("it isn't a JSON object") + except Exception as e: + self.logger.log("Could not read the pre-existing repositories from %s, so cloning without them: %s" % (self.pre_existing_repos_file, e), level=LogLevel.Warning) + return {} + repos = {} + for name, path in mapping.items(): + if not isinstance(path, str): + self.logger.log("The pre-existing repository for %s in %s is %r, not a path, so we're ignoring it." % (name, self.pre_existing_repos_file, path), level=LogLevel.Warning) + elif os.path.isabs(path): + repos[name] = path + else: + self._report_relative_path("The pre-existing repository %s for %s in %s is not an absolute path, so we're ignoring it." % (path, name, self.pre_existing_repos_file)) + return repos + + def _pre_existing_repo(self, library_name): + if self._pre_existing_repos is None: + self._pre_existing_repos = self._load_pre_existing_repos() + return self._pre_existing_repos.get(library_name) def _initialize(self): self.tmpdirname = tempfile.mkdtemp() @@ -139,15 +186,21 @@ def _reset(self): # Remote servers occasionally fail a clone (e.g. 'error: RPC failed; HTTP 502' then # 'fatal: the remote end hung up unexpectedly'), so retry it for a few minutes. @retry(times=4, sleep_s=30) - def _clone(self, repo_url): + def _clone(self, repo_url, pre_existing_repo=None): # A failed clone can leave a partial checkout behind, which would make the next # attempt fail because the directory isn't empty. for name in os.listdir(self.tmpdirname): path = os.path.join(self.tmpdirname, name) shutil.rmtree(path) if os.path.isdir(path) and not os.path.islink(path) else os.remove(path) - self.run(["git", "clone", repo_url, "."]) - - def _ensure_checkout(self, repo_url, specific_revision=None): + # We only read commits and trees, so don't download file contents or check + # anything out (for V8, 366 MB instead of 1.7 GB). A host that doesn't support + # partial clones ignores the filter and sends everything. + # --reference-if-able takes whatever objects it can from the local copy, and + # falls back to a normal clone if the copy isn't there. + reference = ["--reference-if-able", pre_existing_repo] if pre_existing_repo else [] + self.run(["git", "clone"] + reference + ["--filter=blob:none", "--no-checkout", repo_url, "."]) + + def _ensure_checkout(self, repo_url, specific_revision=None, library_name=None): self.previous_dir_stack.append(os.getcwd()) os.chdir(self.tmpdirname) @@ -156,7 +209,7 @@ def _ensure_checkout(self, repo_url, specific_revision=None): if not repo_url: raise Exception("Wound up in _ensure_checkout but missing a repo_url") - self._clone(repo_url) + self._clone(repo_url, self._pre_existing_repo(library_name)) if specific_revision: self.run(["git", "checkout", specific_revision]) @@ -185,7 +238,7 @@ def check_for_update(self, library, task, new_version, most_recent_job_version): try: # Step 0: Get the repo and update to the correct branch. # If no branch is specified, the default branch we clone is assumed to be correct - self._ensure_checkout(library.repo_url, task.branch) + self._ensure_checkout(library.repo_url, task.branch, library.name) # Step 1: Find the common ancestor between the commit we are and new_version common_ancestor = self.run(["git", "merge-base", library.revision, new_version]).stdout.decode().strip() diff --git a/tests/functionality_utilities.py b/tests/functionality_utilities.py index 5dffa470..6939f3d6 100644 --- a/tests/functionality_utilities.py +++ b/tests/functionality_utilities.py @@ -129,7 +129,7 @@ def patch(cmd): (echo_str("echo {\"transactions\": [{\"type\": \"bugzilla.bug-id\""), lambda: CONDUIT_EDIT_OUTPUT), (echo_str("echo {\"transactions\": [{\"type\": \"parents.add\""), lambda: CONDUIT_EDIT_OUTPUT), ("git log -1 --oneline", lambda: "0481f1c (HEAD -> issue-115-add-revision-to-log, origin/issue-115-add-revision-to-log) Issue #115 - Add revision of updatebot to log output"), - ("git clone https://example.invalid .", command_callbacks.get('upstream_clone', lambda: "")), + ("git clone --filter=blob:none --no-checkout https://example.invalid .", command_callbacks.get('upstream_clone', lambda: "")), ("git merge-base --is-ancestor", lambda cmd: FAKE_HISTORY_IS_ANCESTOR(expected_values, cmd)), ("git merge-base", lambda: "_current"), ("git rev-parse", lambda: "abcdef1234567890abcdef1234567890abcdef12"), diff --git a/tests/scm.py b/tests/scm.py index cd0e8cf7..85420c27 100755 --- a/tests/scm.py +++ b/tests/scm.py @@ -6,6 +6,7 @@ import os import sys +import json import copy import shutil import tempfile @@ -15,7 +16,7 @@ sys.path.append(".") sys.path.append("..") from components.scmprovider import SCMProvider, Commit -from components.logging import SimpleLogger, log +from components.logging import SimpleLogger, LogLevel, log from components.commandprovider import CommandProvider from components.utilities import Struct import components.utilities @@ -319,6 +320,128 @@ def run(args, **kwargs): self.assertEqual(attempts, [[], []], "The retry should start from an empty directory") +class TestPartialClone(unittest.TestCase): + def testWorksWithoutFileContents(self): + # Local clones ignore --filter, so serve the test repo over file:// with filtering on + runner = CommandProvider({}) + runner.update_config({'LoggingProvider': SimpleLogger({'local': True})}) + scm = SCMProvider({}) + scm.update_config({'CommandProvider': runner, 'LoggingProvider': SimpleLogger({'local': True})}) + with tempfile.TemporaryDirectory() as d: + upstream = os.path.join(d, "upstream") + subprocess.run(["git", "clone", "-q", "--bare", test_repo_path_wrapper(default_test_repo()), upstream], check=True) + subprocess.run(["git", "-C", upstream, "config", "uploadpack.allowFilter", "true"], check=True) + subprocess.run(["git", "-C", upstream, "config", "uploadpack.allowAnySHA1InWant", "true"], check=True) + + scm.initialize() + try: + library = Struct(repo_url="file://" + upstream, name='Partial-Library', revision=COMMITS_MAIN[2], should_show_commit_details=True) + all_commits, unseen_commits = scm.check_for_update(library, Struct(branch=None), COMMITS_MAIN[0], None) + + filter_config = subprocess.run(["git", "-C", scm.tmpdirname, "config", "remote.origin.partialclonefilter"], + capture_output=True).stdout.decode().strip() + self.assertEqual(filter_config, "blob:none", "The clone should have been partial") + self.assertEqual([c.revision for c in all_commits], [COMMITS_MAIN[1], COMMITS_MAIN[0]]) + for c in all_commits: + self.assertTrue(c.author.strip()) + self.assertTrue(c.files_modified + c.files_added + c.files_deleted + c.files_other) + self.assertFalse(c.signed) + finally: + scm.reset() + + +class TestPreExistingRepos(unittest.TestCase): + def setUp(self): + self.dir = tempfile.TemporaryDirectory() + self.upstream = os.path.join(self.dir.name, "upstream") + subprocess.run(["git", "clone", "-q", "--bare", test_repo_path_wrapper(default_test_repo()), self.upstream], check=True) + subprocess.run(["git", "-C", self.upstream, "config", "uploadpack.allowFilter", "true"], check=True) + subprocess.run(["git", "-C", self.upstream, "config", "uploadpack.allowAnySHA1InWant", "true"], check=True) + self.seed = os.path.join(self.dir.name, "seed") + subprocess.run(["git", "clone", "-q", self.upstream, self.seed], check=True) + self.mapping = os.path.join(self.dir.name, "pre-existing-repos.json") + with open(self.mapping, "w") as f: + json.dump({"Seeded-Library": self.seed}, f) + self.warnings = [] + self.errors = [] + self.exceptions = [] + + def tearDown(self): + self.dir.cleanup() + + def _check_for_update(self, library_name, pre_existing_repos): + runner = CommandProvider({}) + runner.update_config({'LoggingProvider': SimpleLogger({'local': True})}) + scm = SCMProvider({'pre-existing-repos': pre_existing_repos}) + logger = SimpleLogger({'local': True}) + logger.log = lambda msg, level=None, **kwargs: self.warnings.append(msg) if level == LogLevel.Warning else self.errors.append(msg) if level == LogLevel.Error else None + logger.log_exception = self.exceptions.append + scm.update_config({'CommandProvider': runner, 'LoggingProvider': logger}) + scm.initialize() + try: + library = Struct(repo_url="file://" + self.upstream, name=library_name, revision=COMMITS_MAIN[2], should_show_commit_details=True) + all_commits, _ = scm.check_for_update(library, Struct(branch=None), COMMITS_MAIN[0], None) + self.assertEqual([c.revision for c in all_commits], [COMMITS_MAIN[1], COMMITS_MAIN[0]]) + for c in all_commits: + self.assertTrue(c.author.strip()) + self.assertTrue(c.files_modified + c.files_added + c.files_deleted + c.files_other) + alternates = os.path.join(scm.tmpdirname, ".git", "objects", "info", "alternates") + return open(alternates).read().strip() if os.path.exists(alternates) else None + finally: + scm.reset() + + def testBorrowsFromTheListedCopy(self): + self.assertEqual(self._check_for_update("Seeded-Library", self.mapping), os.path.join(self.seed, ".git", "objects")) + self.assertEqual(self.warnings, []) + + def testUnlistedLibraryClonesNormally(self): + self.assertIsNone(self._check_for_update("Other-Library", self.mapping)) + self.assertEqual(self.warnings, []) + + def testMissingCopyClonesNormally(self): + shutil.rmtree(self.seed) + self.assertIsNone(self._check_for_update("Seeded-Library", self.mapping)) + + def testRelativeFileIsIgnored(self): + previous_dir = os.getcwd() + os.chdir(self.dir.name) + try: + self.assertIsNone(self._check_for_update("Seeded-Library", "pre-existing-repos.json")) + finally: + os.chdir(previous_dir) + self.assertEqual(len(self.errors), 1) + self.assertIn("is not an absolute path", self.errors[0]) + self.assertEqual([type(e).__name__ for e in self.exceptions], ["RelativePreExistingRepoPath"]) + + def testRelativeRepoIsIgnored(self): + with open(self.mapping, "w") as f: + json.dump({"Seeded-Library": "seed", "Other-Library": self.seed}, f) + self.assertIsNone(self._check_for_update("Seeded-Library", self.mapping)) + self.assertEqual(len(self.errors), 1) + self.assertIn("seed for Seeded-Library", self.errors[0]) + self.assertEqual([type(e).__name__ for e in self.exceptions], ["RelativePreExistingRepoPath"]) + # The absolute entry is still used + self.assertEqual(self._check_for_update("Other-Library", self.mapping), os.path.join(self.seed, ".git", "objects")) + + def testMalformedMappingClonesNormally(self): + # A different library each time, because check_for_update is memoized + for i, contents in enumerate(([], {"Malformed-1": None}, {"Malformed-2": ["/a"]})): + self.warnings = [] + with open(self.mapping, "w") as f: + json.dump(contents, f) + self.assertIsNone(self._check_for_update("Malformed-%d" % i, self.mapping)) + self.assertEqual(len(self.warnings), 1, contents) + + def testWithoutTheSetting(self): + self.assertIsNone(self._check_for_update("Seeded-Library", None)) + self.assertEqual(self.warnings, []) + + def testUnreadableMappingClonesNormally(self): + self.assertIsNone(self._check_for_update("Seeded-Library", os.path.join(self.dir.name, "missing.json"))) + self.assertEqual(len(self.warnings), 1) + self.assertIn("Could not read the pre-existing repositories", self.warnings[0]) + + class TestNonUTF8Commit(unittest.TestCase): def testISO88591Commit(self): # git cat-file prints a commit as stored, so one in another encoding isn't UTF-8