From b7e77b43e8878fd9e3feae5c7717d32627c09968 Mon Sep 17 00:00:00 2001 From: Tom Ritter Date: Tue, 10 Nov 2026 17:24:37 +0000 Subject: [PATCH 1/3] Clone upstream repositories without file contents. Fixes #385 Updatebot only reads commits and trees from an upstream clone, never file contents, so make it a blobless clone with no checkout. For irregexp's V8 that's 366 MB instead of 1.7 GB, and a 104 second clone in production should take a few seconds. A host that doesn't support partial clones ignores the filter. --- components/scmprovider.py | 5 ++++- tests/functionality_utilities.py | 2 +- tests/scm.py | 30 ++++++++++++++++++++++++++++++ 3 files changed, 35 insertions(+), 2 deletions(-) diff --git a/components/scmprovider.py b/components/scmprovider.py index f6ab537..1874f59 100644 --- a/components/scmprovider.py +++ b/components/scmprovider.py @@ -145,7 +145,10 @@ def _clone(self, repo_url): 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, "."]) + # 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. + self.run(["git", "clone", "--filter=blob:none", "--no-checkout", repo_url, "."]) def _ensure_checkout(self, repo_url, specific_revision=None): self.previous_dir_stack.append(os.getcwd()) diff --git a/tests/functionality_utilities.py b/tests/functionality_utilities.py index 5dffa47..6939f3d 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 cd0e8cf..e7d4c04 100755 --- a/tests/scm.py +++ b/tests/scm.py @@ -319,6 +319,36 @@ 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 TestNonUTF8Commit(unittest.TestCase): def testISO88591Commit(self): # git cat-file prints a commit as stored, so one in another encoding isn't UTF-8 From d848be122023adb72cc938ac54e6d5bc2ecf5509 Mon Sep 17 00:00:00 2001 From: Tom Ritter Date: Wed, 11 Nov 2026 05:21:12 +0000 Subject: [PATCH 2/3] Borrow objects from a local copy of an upstream repository when there is one. The Docker image already has clones of V8 and PDF.js, which mach vendor updates to the revision being vendored before we clone the same repository again to list its commits. The new 'pre-existing-repos' setting under 'SCM' names a JSON file mapping library names to such copies, and we clone with --reference-if-able so that git downloads only what the copy lacks. A missing copy falls back to a normal clone, and an unreadable or malformed mapping file is a warning. Paths must be absolute, because we read the file and use its paths from inside the clone directory; a relative one is ignored and reported to Sentry. (Bug 2091623, #385) --- components/scmprovider.py | 62 ++++++++++++++++++++++--- tests/scm.py | 95 ++++++++++++++++++++++++++++++++++++++- 2 files changed, 150 insertions(+), 7 deletions(-) diff --git a/components/scmprovider.py b/components/scmprovider.py index 1874f59..59639c1 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,7 +186,7 @@ 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): @@ -148,9 +195,12 @@ def _clone(self, repo_url): # 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. - self.run(["git", "clone", "--filter=blob:none", "--no-checkout", repo_url, "."]) + # --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): + def _ensure_checkout(self, repo_url, specific_revision=None, library_name=None): self.previous_dir_stack.append(os.getcwd()) os.chdir(self.tmpdirname) @@ -159,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]) @@ -188,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/scm.py b/tests/scm.py index e7d4c04..85420c2 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 @@ -349,6 +350,98 @@ def testWorksWithoutFileContents(self): 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 From ba1b2606802bb8b942ca9942061616e27c974b6c Mon Sep 17 00:00:00 2001 From: Tom Ritter Date: Wed, 11 Nov 2026 14:55:42 +0000 Subject: [PATCH 3/3] Run CI on Ubuntu 24.04, and update the package lists before installing Mercurial. GitHub is moving ubuntu-latest to Ubuntu 26.04 (actions/runner-images#14748), and since then installing Mercurial fails. Pin the version so that the OS CI runs on only changes when we choose. apt-get update also keeps the install from depending on the package lists the runner image was built with. --- .github/workflows/ci.yml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f614bcc..8cda3bf 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