From 831bb542828f0c7aaf8307416d688628b13fc82b Mon Sep 17 00:00:00 2001 From: Ronnie Dutta <61982285+MetRonnie@users.noreply.github.com> Date: Tue, 22 Oct 2024 12:34:10 +0100 Subject: [PATCH] Fix traceback when git is not installed --- metomi/rose/loc_handlers/git.py | 58 ++++++++++++++++++++------------- metomi/rose/popen.py | 2 +- metomi/rose/tests/test_git.py | 45 +++++++++++++++++++++++++ 3 files changed, 82 insertions(+), 23 deletions(-) create mode 100644 metomi/rose/tests/test_git.py diff --git a/metomi/rose/loc_handlers/git.py b/metomi/rose/loc_handlers/git.py index e6f6b932a3..5cc640100d 100644 --- a/metomi/rose/loc_handlers/git.py +++ b/metomi/rose/loc_handlers/git.py @@ -16,11 +16,20 @@ # ----------------------------------------------------------------------------- """A handler of Git locations.""" +import errno import os import re import tempfile +from typing import TYPE_CHECKING, Optional, Tuple from urllib.parse import urlparse +from metomi.rose.popen import RosePopenError + +if TYPE_CHECKING: + from metomi.rose.config_processors.fileinstall import ( + PullableLocHandlersManager, + ) + REC_COMMIT_HASH = re.compile(r"^[0-9a-f]+$") @@ -33,23 +42,27 @@ class GitLocHandler: WEB_SCHEMES = ["https"] URI_SEPARATOR = "::" - def __init__(self, manager): + def __init__(self, manager: 'PullableLocHandlersManager'): self.manager = manager # Determine (just once) what git version we have, if any. - ret_code, versiontext, stderr = self.manager.popen.run( - "git", "version") - if ret_code: - # Git not installed. - self.git_version = None - else: - # Git is installed, get the version. - version_nums = [] - for num_string in versiontext.split()[-1].split("."): - try: - version_nums.append(int(num_string)) - except ValueError: - break - self.git_version = tuple(version_nums) + try: + _ret_code, versiontext, _stderr = self.manager.popen.run( + self.GIT, "version" + ) + except RosePopenError as exc: + if exc.ret_code == errno.ENOENT: + # Git not installed. + self.git_version: Optional[Tuple[int, ...]] = None + return + raise + # Git is installed, get the version. + version_nums = [] + for num_string in versiontext.split()[-1].split("."): + try: + version_nums.append(int(num_string)) + except ValueError: + break + self.git_version = tuple(version_nums) def can_pull(self, loc): """Determine if this is a suitable handler for loc.""" @@ -65,7 +78,7 @@ def can_pull(self, loc): scheme in self.WEB_SCHEMES and not os.path.exists(loc.name) # same as svn... and not self.manager.popen.run( - "git", "ls-remote", "--exit-code", remote)[0] + self.GIT, "ls-remote", "--exit-code", remote)[0] # https://superuser.com/questions/227509/git-ping-check-if-remote-repository-exists ) @@ -111,27 +124,28 @@ async def pull(self, loc, conf_tree): with tempfile.TemporaryDirectory() as tmpdirname: git_dir_opt = f"--git-dir={tmpdirname}/.git" await self.manager.popen.run_ok_async( - "git", git_dir_opt, "init" + self.GIT, git_dir_opt, "init" ) if self.git_version >= (2, 25, 0) and path != "./": # sparse-checkout available and suitable for this case. await self.manager.popen.run_ok_async( - "git", git_dir_opt, "sparse-checkout", "set", path, + self.GIT, git_dir_opt, "sparse-checkout", "set", path, "--no-cone" ) await self.manager.popen.run_ok_async( - "git", git_dir_opt, "fetch", "--depth=1", + self.GIT, git_dir_opt, "fetch", "--depth=1", "--filter=blob:none", remote, loc.key ) else: # Fallback. await self.manager.popen.run_ok_async( - "git", git_dir_opt, "fetch", "--depth=1", remote, loc.key + self.GIT, git_dir_opt, "fetch", "--depth=1", remote, + loc.key ) # Checkout to temporary location, then extract only 'path' later. await self.manager.popen.run_ok_async( - "git", git_dir_opt, f"--work-tree={tmpdirname}", "checkout", + self.GIT, git_dir_opt, f"--work-tree={tmpdirname}", "checkout", loc.key ) name = tmpdirname + "/" + path @@ -165,7 +179,7 @@ def _get_commithash(self, remote, ref): """ ret_code, info, _ = self.manager.popen.run( - "git", "ls-remote", "--exit-code", remote, ref) + self.GIT, "ls-remote", "--exit-code", remote, ref) if ret_code and ret_code != 2: # repo not found raise ValueError(f"ls-remote: could not locate '{remote}'") diff --git a/metomi/rose/popen.py b/metomi/rose/popen.py index 8993fb937c..30272e285b 100644 --- a/metomi/rose/popen.py +++ b/metomi/rose/popen.py @@ -268,7 +268,7 @@ def run_bg(self, *args, **kwargs): except OSError as exc: if exc.filename is None and args: exc.filename = args[0] - raise RosePopenError(args, 1, "", str(exc)) + raise RosePopenError(args, exc.errno, "", str(exc)) from None return proc def run_nohup_gui(self, cmd): diff --git a/metomi/rose/tests/test_git.py b/metomi/rose/tests/test_git.py new file mode 100644 index 0000000000..10572f6314 --- /dev/null +++ b/metomi/rose/tests/test_git.py @@ -0,0 +1,45 @@ +# Copyright (C) British Crown (Met Office) & Contributors. +# This file is part of Rose, a framework for meteorological suites. +# +# Rose is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# Rose is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with Rose. If not, see . + + +import shutil +import pytest +from secrets import token_hex + +from metomi.rose.config_processors.fileinstall import ( + PullableLocHandlersManager, +) +from metomi.rose.loc_handlers.git import GitLocHandler + + +require_git = pytest.mark.skipif( + shutil.which('git') is None, + reason="git is not installed" +) + + +@require_git +def test_init_ok(): + handler = GitLocHandler(PullableLocHandlersManager()) + assert len(handler.git_version) > 1 + assert all(isinstance(i, int) for i in handler.git_version) + + +def test_init_no_git(monkeypatch: pytest.MonkeyPatch): + """Test the handler doesn't throw a tantrum if git is not installed.""" + monkeypatch.setattr(GitLocHandler, 'GIT', token_hex(8)) + handler = GitLocHandler(PullableLocHandlersManager()) + assert handler.git_version is None