diff --git a/synthtool/sources/git.py b/synthtool/sources/git.py index 55c3e2b61..d79165d2e 100644 --- a/synthtool/sources/git.py +++ b/synthtool/sources/git.py @@ -12,6 +12,8 @@ # See the License for the specific language governing permissions and # limitations under the License. +import contextlib +import fcntl import os import pathlib import re @@ -44,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. @@ -105,21 +117,33 @@ def clone( dest = dest / pathlib.Path(url).stem - 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)) + lock_file = dest.parent / (dest.name + ".lock") + 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) diff --git a/tests/test_git.py b/tests/test_git.py index 0a1082ace..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 @@ -87,6 +88,16 @@ def tearDown(self): os.environ = self.env return super().tearDown() + @mock.patch("fcntl.flock") + def testCloneConcurrencyPatch(self, mock_flock): + 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) + 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()