From 5934d3541efd7497452c9025f329f15d27dd8f4f Mon Sep 17 00:00:00 2001 From: Sofia Leon Date: Thu, 6 Aug 2026 22:10:01 +0000 Subject: [PATCH 1/6] Apply concurrency patch to prevent git index.lock collisions --- synthtool/sources/git.py | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/synthtool/sources/git.py b/synthtool/sources/git.py index 55c3e2b61..9820dde9a 100644 --- a/synthtool/sources/git.py +++ b/synthtool/sources/git.py @@ -109,13 +109,20 @@ def clone( shutil.rmtree(dest) default_branch = None - if not dest.exists(): - cmd = ["git", "clone", "--recurse-submodules", "--single-branch", url, dest] - shell.run(cmd, check=True) - else: - default_branch = _local_default_branch(dest) - shell.run(["git", "checkout", default_branch], cwd=str(dest), check=True) - shell.run(["git", "pull"], cwd=str(dest), check=True) + import fcntl + lock_file = dest.parent / (dest.name + ".lock") + with open(lock_file, "w") as lock_f: + fcntl.flock(lock_f, fcntl.LOCK_EX) + try: + if not dest.exists(): + cmd = ["git", "clone", "--recurse-submodules", "--single-branch", url, dest] + shell.run(cmd, check=True) + else: + default_branch = _local_default_branch(dest) + shell.run(["git", "checkout", default_branch], cwd=str(dest), check=True) + shell.run(["git", "pull"], cwd=str(dest), check=True) + finally: + fcntl.flock(lock_f, fcntl.LOCK_UN) committish = committish or default_branch if committish: From 464d75e4535526c679d3f62791df05ebb732c2f1 Mon Sep 17 00:00:00 2001 From: Sofia Leon Date: Thu, 6 Aug 2026 22:14:29 +0000 Subject: [PATCH 2/6] chore: add tests --- tests/test_git.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/tests/test_git.py b/tests/test_git.py index 0a1082ace..5e416c1c0 100644 --- a/tests/test_git.py +++ b/tests/test_git.py @@ -87,6 +87,16 @@ def tearDown(self): os.environ = self.env return super().tearDown() + @mock.patch("fcntl.flock") + def testCloneConcurrencyPatch(self, mock_flock): + import fcntl + metadata.reset() + local_directory = git.clone("https://github.com/googleapis/nodejs-vision.git") + self.assertTrue(mock_flock.called) + # Should be called with LOCK_EX then LOCK_UN + mock_flock.assert_any_call(mock.ANY, fcntl.LOCK_EX) + mock_flock.assert_any_call(mock.ANY, fcntl.LOCK_UN) + def testClone(self): # clear out metadata before creating new metadata and asserting on it metadata.reset() From 4361c4b778058b9da271dc87b239cd34ead208a5 Mon Sep 17 00:00:00 2001 From: Sofia Leon Date: Thu, 6 Aug 2026 22:18:50 +0000 Subject: [PATCH 3/6] fix(git): expand lock to encompass git reset --- synthtool/sources/git.py | 29 +++++++++++++++-------------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/synthtool/sources/git.py b/synthtool/sources/git.py index 9820dde9a..0f94b4583 100644 --- a/synthtool/sources/git.py +++ b/synthtool/sources/git.py @@ -105,15 +105,16 @@ def clone( dest = dest / pathlib.Path(url).stem - if force and dest.exists(): - shutil.rmtree(dest) - - default_branch = None - import fcntl - lock_file = dest.parent / (dest.name + ".lock") - with open(lock_file, "w") as lock_f: - fcntl.flock(lock_f, fcntl.LOCK_EX) - try: + import fcntl + lock_file = dest.parent / (dest.name + ".lock") + with open(lock_file, "w") as lock_f: + fcntl.flock(lock_f, fcntl.LOCK_EX) + try: + if not preclone: + if force and dest.exists(): + shutil.rmtree(dest) + + default_branch = None if not dest.exists(): cmd = ["git", "clone", "--recurse-submodules", "--single-branch", url, dest] shell.run(cmd, check=True) @@ -121,12 +122,12 @@ def clone( default_branch = _local_default_branch(dest) shell.run(["git", "checkout", default_branch], cwd=str(dest), check=True) shell.run(["git", "pull"], cwd=str(dest), check=True) - finally: - fcntl.flock(lock_f, fcntl.LOCK_UN) - committish = committish or default_branch + committish = committish or default_branch - if committish: - shell.run(["git", "reset", "--hard", committish], cwd=str(dest)) + if committish: + shell.run(["git", "reset", "--hard", committish], cwd=str(dest)) + finally: + fcntl.flock(lock_f, fcntl.LOCK_UN) # track all git repositories _tracked_paths.add(dest) From 8057ef2c2d6b6fc5369934287d1bd6144cc81090 Mon Sep 17 00:00:00 2001 From: Sofia Leon Date: Thu, 6 Aug 2026 22:30:25 +0000 Subject: [PATCH 4/6] chore: fix black and flake8 formatting issues --- synthtool/sources/git.py | 14 ++++++++++++-- tests/test_git.py | 2 ++ 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/synthtool/sources/git.py b/synthtool/sources/git.py index 0f94b4583..ff6012077 100644 --- a/synthtool/sources/git.py +++ b/synthtool/sources/git.py @@ -106,6 +106,7 @@ def clone( dest = dest / pathlib.Path(url).stem import fcntl + lock_file = dest.parent / (dest.name + ".lock") with open(lock_file, "w") as lock_f: fcntl.flock(lock_f, fcntl.LOCK_EX) @@ -116,11 +117,20 @@ def clone( default_branch = None if not dest.exists(): - cmd = ["git", "clone", "--recurse-submodules", "--single-branch", url, dest] + cmd = [ + "git", + "clone", + "--recurse-submodules", + "--single-branch", + url, + dest, + ] shell.run(cmd, check=True) else: default_branch = _local_default_branch(dest) - shell.run(["git", "checkout", default_branch], cwd=str(dest), check=True) + shell.run( + ["git", "checkout", default_branch], cwd=str(dest), check=True + ) shell.run(["git", "pull"], cwd=str(dest), check=True) committish = committish or default_branch diff --git a/tests/test_git.py b/tests/test_git.py index 5e416c1c0..524edc408 100644 --- a/tests/test_git.py +++ b/tests/test_git.py @@ -90,8 +90,10 @@ def tearDown(self): @mock.patch("fcntl.flock") def testCloneConcurrencyPatch(self, mock_flock): import fcntl + metadata.reset() local_directory = git.clone("https://github.com/googleapis/nodejs-vision.git") + self.assertEqual("nodejs-vision", local_directory.name) self.assertTrue(mock_flock.called) # Should be called with LOCK_EX then LOCK_UN mock_flock.assert_any_call(mock.ANY, fcntl.LOCK_EX) From 2cdd89931b8249d5381a0bec2597cb9e5fd4b45e Mon Sep 17 00:00:00 2001 From: Sofia Leon Date: Fri, 7 Aug 2026 00:08:34 +0000 Subject: [PATCH 5/6] chore: move import fcntl to top of files --- synthtool/sources/git.py | 3 +-- tests/test_git.py | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/synthtool/sources/git.py b/synthtool/sources/git.py index ff6012077..b3f316f67 100644 --- a/synthtool/sources/git.py +++ b/synthtool/sources/git.py @@ -12,6 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. +import fcntl import os import pathlib import re @@ -105,8 +106,6 @@ def clone( dest = dest / pathlib.Path(url).stem - import fcntl - lock_file = dest.parent / (dest.name + ".lock") with open(lock_file, "w") as lock_f: fcntl.flock(lock_f, fcntl.LOCK_EX) diff --git a/tests/test_git.py b/tests/test_git.py index 524edc408..bafc23984 100644 --- a/tests/test_git.py +++ b/tests/test_git.py @@ -14,6 +14,7 @@ import copy import importlib +import fcntl import os import unittest from unittest import mock @@ -89,8 +90,6 @@ def tearDown(self): @mock.patch("fcntl.flock") def testCloneConcurrencyPatch(self, mock_flock): - import fcntl - metadata.reset() local_directory = git.clone("https://github.com/googleapis/nodejs-vision.git") self.assertEqual("nodejs-vision", local_directory.name) From 295bec6f53249819b1db76516058116c4a807c9a Mon Sep 17 00:00:00 2001 From: Sofia Leon Date: Fri, 7 Aug 2026 00:10:19 +0000 Subject: [PATCH 6/6] refactor: extract file lock into context manager --- synthtool/sources/git.py | 67 ++++++++++++++++++++++------------------ 1 file changed, 37 insertions(+), 30 deletions(-) diff --git a/synthtool/sources/git.py b/synthtool/sources/git.py index b3f316f67..d79165d2e 100644 --- a/synthtool/sources/git.py +++ b/synthtool/sources/git.py @@ -12,6 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. +import contextlib import fcntl import os import pathlib @@ -45,6 +46,16 @@ def make_repo_clone_url(repo: str) -> str: return f"https://github.com/{repo}.git" +@contextlib.contextmanager +def file_lock(lock_path: pathlib.Path): + with open(lock_path, "w") as lock_f: + fcntl.flock(lock_f, fcntl.LOCK_EX) + try: + yield lock_f + finally: + fcntl.flock(lock_f, fcntl.LOCK_UN) + + def _local_default_branch(path: pathlib.Path) -> Union[str, None]: """Helper method to infer the default branch. @@ -107,36 +118,32 @@ def clone( dest = dest / pathlib.Path(url).stem lock_file = dest.parent / (dest.name + ".lock") - with open(lock_file, "w") as lock_f: - fcntl.flock(lock_f, fcntl.LOCK_EX) - try: - if not preclone: - if force and dest.exists(): - shutil.rmtree(dest) - - default_branch = None - if not dest.exists(): - cmd = [ - "git", - "clone", - "--recurse-submodules", - "--single-branch", - url, - dest, - ] - shell.run(cmd, check=True) - else: - default_branch = _local_default_branch(dest) - shell.run( - ["git", "checkout", default_branch], cwd=str(dest), check=True - ) - shell.run(["git", "pull"], cwd=str(dest), check=True) - committish = committish or default_branch - - if committish: - shell.run(["git", "reset", "--hard", committish], cwd=str(dest)) - finally: - fcntl.flock(lock_f, fcntl.LOCK_UN) + with file_lock(lock_file): + if not preclone: + if force and dest.exists(): + shutil.rmtree(dest) + + default_branch = None + if not dest.exists(): + cmd = [ + "git", + "clone", + "--recurse-submodules", + "--single-branch", + url, + dest, + ] + shell.run(cmd, check=True) + else: + default_branch = _local_default_branch(dest) + shell.run( + ["git", "checkout", default_branch], cwd=str(dest), check=True + ) + shell.run(["git", "pull"], cwd=str(dest), check=True) + committish = committish or default_branch + + if committish: + shell.run(["git", "reset", "--hard", committish], cwd=str(dest)) # track all git repositories _tracked_paths.add(dest)