From 75e0bcad9252d584b4f470e30532fff0d62a1ca9 Mon Sep 17 00:00:00 2001 From: James Frost Date: Fri, 4 Sep 2026 13:00:16 +0100 Subject: [PATCH 1/5] Make check_existing function return whether a path exists This avoids the need to separately check this in other code. --- github_scripts/get_git_sources.py | 11 ++++++----- github_scripts/tests/test_get_git_sources.py | 3 ++- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/github_scripts/get_git_sources.py b/github_scripts/get_git_sources.py index 313313ac..fa4c378b 100644 --- a/github_scripts/get_git_sources.py +++ b/github_scripts/get_git_sources.py @@ -280,10 +280,10 @@ def get_unmerged(loc: Path) -> list[str]: return files.stdout.split() -def check_existing(loc: Path) -> None: +def check_existing(loc: Path) -> bool: """ - If the repository exists and isn't a git repo, exit now as we don't want to - overwrite it + Return whether the repository already exists. If it does but isn't a git + repo, exit now as we don't want to overwrite it. """ if loc.exists(): @@ -292,6 +292,8 @@ def check_existing(loc: Path) -> None: f"The destination, '{loc}', already exists but isn't a git directory. " "Exiting so as to not overwrite it." ) + return True + return False def clone_repo_mirror( @@ -361,7 +363,7 @@ def clone_repo(repo_source: str, repo_ref: str, loc: Path) -> None: - loc: path to clone the repository to """ - if not loc.exists(): + if not check_existing(loc): # Create a clean clone location loc.mkdir(parents=True) @@ -377,7 +379,6 @@ def clone_repo(repo_source: str, repo_ref: str, loc: Path) -> None: for command in commands: run_command(command) else: - check_existing(loc) commands = ( f"git -C {loc} fetch origin {repo_ref}", f"git -C {loc} checkout FETCH_HEAD", diff --git a/github_scripts/tests/test_get_git_sources.py b/github_scripts/tests/test_get_git_sources.py index 081281fd..66112be3 100644 --- a/github_scripts/tests/test_get_git_sources.py +++ b/github_scripts/tests/test_get_git_sources.py @@ -147,7 +147,8 @@ def test_check_exists(setup_sources): Test check_existing """ - assert check_existing(setup_sources / "SimSys_Scripts") is None + assert check_existing(setup_sources / "SimSys_Scripts") is True + assert check_existing(setup_sources / "does_not_exist") is False with pytest.raises(FileExistsError): check_existing(setup_sources / "empty_dir") From be8785f3c79af827e9de7e4286838d45b9535191 Mon Sep 17 00:00:00 2001 From: James Frost Date: Fri, 4 Sep 2026 13:02:36 +0100 Subject: [PATCH 2/5] Combine checkout into clone and share objects where possible MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This directly clones the refs that are desired. Where the localmirrors exist on the same system as the new clone the objects are merely referenced instead of being copied, making it much faster, especially on high latency filesystems. A small test on my VDI gives a 3x speedup: | Command | Mean [s] |Min [s]|Max [s]| Relative | |:-------------------------------------------------------------------|--------------:|------:|------:|------------:| | `git clone --branch vn14.2 localmirrors:MetOffice/um.git` | 2.524 ± 0.802 | 1.988 | 4.641 | 3.16 ± 1.00 | | `git clone --shared --branch vn14.2 localmirrors:MetOffice/um.git` | 0.799 ± 0.014 | 0.770 | 0.821 | 1.00 | The `--shared` flag won't do anything for remote repositories that can't directly share objects and will be silently ignored. --- github_scripts/get_git_sources.py | 30 ++++++++++++------------------ 1 file changed, 12 insertions(+), 18 deletions(-) diff --git a/github_scripts/get_git_sources.py b/github_scripts/get_git_sources.py index fa4c378b..490fb384 100644 --- a/github_scripts/get_git_sources.py +++ b/github_scripts/get_git_sources.py @@ -311,25 +311,19 @@ def clone_repo_mirror( - loc: path to clone the repository to """ - if loc.exists(): - check_existing(loc) - # Clone if the repo doesn't exist + fetch = determine_mirror_fetch(repo_source, repo_ref) if repo_ref else "HEAD" + if check_existing(loc): + # If not provided a ref, pull the latest version of the current branch. + if not repo_ref: + run_command(f"git -C {loc} pull") + return + # Update existing repository. + run_command(f"git -C {loc} fetch origin {fetch}") + run_command(f"git -C {loc} checkout FETCH_HEAD") else: - command = f"git clone {mirror_loc} {loc}" - run_command(command) - - # If not provided a ref, pull the latest repository and return - if not repo_ref: - run_command(f"git -C {loc} pull") - return - - fetch = determine_mirror_fetch(repo_source, repo_ref) - commands = ( - f"git -C {loc} fetch origin {fetch}", - f"git -C {loc} checkout FETCH_HEAD", - ) - for command in commands: - run_command(command) + # Clone if the repo doesn't exist. If the mirror is local we don't copy + # the objects to make it much faster. + run_command(f"git clone --shared --branch {fetch} {mirror_loc} {loc}") def determine_mirror_fetch(repo_source: str, repo_ref: str) -> str: From 8e5e0341fb3bc858d9934ec192954c4325ea1d0e Mon Sep 17 00:00:00 2001 From: James Frost Date: Fri, 4 Sep 2026 13:11:53 +0100 Subject: [PATCH 3/5] Add help when cloning from a local mirror fails People not having their mirrors setup is a common support question; hopefully this will encourage people to actively seek out our written guidance on this. I have however left it fairly generic as the process will be different at different sites. --- github_scripts/get_git_sources.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/github_scripts/get_git_sources.py b/github_scripts/get_git_sources.py index 490fb384..2ec5865c 100644 --- a/github_scripts/get_git_sources.py +++ b/github_scripts/get_git_sources.py @@ -323,7 +323,14 @@ def clone_repo_mirror( else: # Clone if the repo doesn't exist. If the mirror is local we don't copy # the objects to make it much faster. - run_command(f"git clone --shared --branch {fetch} {mirror_loc} {loc}") + try: + run_command(f"git clone --shared --branch {fetch} {mirror_loc} {loc}") + except SubprocessRunError: + logger.error( + "Cloning from local mirror failed. " + "Check your local guidance on how to set up mirror access." + ) + raise def determine_mirror_fetch(repo_source: str, repo_ref: str) -> str: From 8b2133a548b5076546129b1e81156cec1054d761 Mon Sep 17 00:00:00 2001 From: James Frost Date: Fri, 4 Sep 2026 14:10:32 +0100 Subject: [PATCH 4/5] Sign CLA by adding self to CONTRIBUTORS.md --- CONTRIBUTORS.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CONTRIBUTORS.md b/CONTRIBUTORS.md index 8913a005..8da40770 100644 --- a/CONTRIBUTORS.md +++ b/CONTRIBUTORS.md @@ -3,3 +3,4 @@ | GitHub user | Real Name | Affiliation | Date | | ----------- | --------------- | ----------- | ---------- | | yaswant | Yaswant Pradhan | Met Office | 2026-07-17 | +| jfrost-mo | James Frost | Met Office | 2026-09-04 | From d4ca08f850dd245fc38e048ed07239c1aa2cd9e3 Mon Sep 17 00:00:00 2001 From: James Frost Date: Fri, 4 Sep 2026 14:48:57 +0100 Subject: [PATCH 5/5] Checkout after clone to support specific hashes The `--branch` option to git clone only takes branchs or tags. --- github_scripts/get_git_sources.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/github_scripts/get_git_sources.py b/github_scripts/get_git_sources.py index 2ec5865c..883fd635 100644 --- a/github_scripts/get_git_sources.py +++ b/github_scripts/get_git_sources.py @@ -324,7 +324,11 @@ def clone_repo_mirror( # Clone if the repo doesn't exist. If the mirror is local we don't copy # the objects to make it much faster. try: - run_command(f"git clone --shared --branch {fetch} {mirror_loc} {loc}") + # Adding `--revision {fetch}` to the clone would be more efficient + # due to avoiding an unnecessary checkout, however we need to + # support versions of git older than v2.49. + run_command(f"git clone --shared {mirror_loc} {loc}") + run_command(f"git -C {loc} checkout {fetch}") except SubprocessRunError: logger.error( "Cloning from local mirror failed. "