From 19d0bece214c34d6ab5ae55cdad81747dff682d7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Filip=20V=C3=A1gner?= Date: Wed, 13 May 2026 15:21:43 +0200 Subject: [PATCH 1/4] Add filelock for git operations --- pyproject.toml | 1 + src/tmt_web/utils/git_handler.py | 67 ++++++++++++++++++++++---------- 2 files changed, 47 insertions(+), 21 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 6484a11..d13a176 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -28,6 +28,7 @@ dependencies = [ "httpx~=0.27", "uvicorn~=0.30", "valkey", + "filelock", ] [project.urls] diff --git a/src/tmt_web/utils/git_handler.py b/src/tmt_web/utils/git_handler.py index cc2689b..1d8cd6d 100644 --- a/src/tmt_web/utils/git_handler.py +++ b/src/tmt_web/utils/git_handler.py @@ -10,6 +10,7 @@ import re from shutil import rmtree +from filelock import FileLock from tmt import Logger from tmt._compat.pathlib import Path from tmt.utils import Command, Common, GeneralError, RunError @@ -21,13 +22,31 @@ ROOT_DIR = Path(__file__).resolve().parents[2] -def create_hash(text: str): +def _create_hash(text: str): """Create hash of the given text that is consistent across runs.""" hashed_text = hashlib.new("sha1", usedforsecurity=False) hashed_text.update(text.encode()) return hashed_text.hexdigest() +def get_repo_hash(url: str) -> str: + url = url.rstrip("/").removesuffix(".git") + return _create_hash(url) + + +def get_repo_lock_path(url: str) -> Path: + """ + Get the path for a repository lock file. + + :param url: Repository URL + :return: Path to the lock file for this repository + """ + lock_path = ROOT_DIR / settings.CLONE_DIR_PATH / f"{get_repo_hash(url)}.lock" + if not lock_path.parent.exists(): + lock_path.parent.mkdir(parents=True, exist_ok=True) + return lock_path + + def get_unique_clone_path(url: str) -> Path: """ Generate a unique path for cloning a repository. @@ -35,9 +54,7 @@ def get_unique_clone_path(url: str) -> Path: :param url: Repository URL :return: Unique path for cloning """ - url = url.rstrip("/").removesuffix(".git") - clone_dir_name = create_hash(url) - return ROOT_DIR / settings.CLONE_DIR_PATH / clone_dir_name + return ROOT_DIR / settings.CLONE_DIR_PATH / get_repo_hash(url) def clear_tmp_dir(logger: Logger) -> None: @@ -77,6 +94,8 @@ def clone_repository(url: str, logger: Logger) -> Path: # Get unique path destination = get_unique_clone_path(url) + rmtree(destination, ignore_errors=True) + # Clone with retry logic git_clone(url=url, destination=destination, logger=logger) @@ -95,29 +114,35 @@ def get_git_repository(url: str, logger: Logger, ref: str | None = None) -> Path :raises: GeneralError if cloning, fetching, or updating a branch fails :raises: AttributeError if ref doesn't exist """ - destination = get_unique_clone_path(url) - if not destination.exists(): - clone_repository(url, logger) + lock_path = get_repo_lock_path(url) + with FileLock(lock_path): + destination = get_unique_clone_path(url) + if not destination.exists(): + clone_repository(url, logger) - common = Common(logger=logger) + common = Common(logger=logger) - # Fetch remote refs - _fetch_remote(common, destination, logger) + try: + # Fetch remote refs + _fetch_remote(common, destination, logger) + except GeneralError: + logger.warning("Unable to fetch remote repository. Trying to clone again.") + clone_repository(url, logger) - # If no ref is specified, the default branch is used - ref = ref or _get_default_branch(common, destination, logger) + # If no ref is specified, the default branch is used + ref = ref or _get_default_branch(common, destination, logger) - try: - common.run(Command("git", "checkout", ref), cwd=destination) - except RunError as err: - logger.fail(f"Failed to checkout ref '{ref}': {err.stderr}") - raise AttributeError(f"Failed to checkout ref '{ref}'") from err + try: + common.run(Command("git", "checkout", ref), cwd=destination) + except RunError as err: + logger.fail(f"Failed to checkout ref '{ref}': {err.stderr}") + raise AttributeError(f"Failed to checkout ref '{ref}'") from err - _ensure_no_changes(common, destination, logger) + _ensure_no_changes(common, destination, logger) - # If the ref is a branch, ensure it's up to date - if _is_branch(common, destination, ref): - _update_branch(common, destination, ref, logger) + # If the ref is a branch, ensure it's up to date + if _is_branch(common, destination, ref): + _update_branch(common, destination, ref, logger) return destination From b5cbeb14810228cdbb5a7f2caebcd43d83de12bf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Filip=20V=C3=A1gner?= Date: Tue, 26 May 2026 16:26:41 +0200 Subject: [PATCH 2/4] squash: refactor --- src/tmt_web/utils/git_handler.py | 84 ++++++-------------------------- 1 file changed, 15 insertions(+), 69 deletions(-) diff --git a/src/tmt_web/utils/git_handler.py b/src/tmt_web/utils/git_handler.py index 1d8cd6d..20d22b2 100644 --- a/src/tmt_web/utils/git_handler.py +++ b/src/tmt_web/utils/git_handler.py @@ -34,19 +34,6 @@ def get_repo_hash(url: str) -> str: return _create_hash(url) -def get_repo_lock_path(url: str) -> Path: - """ - Get the path for a repository lock file. - - :param url: Repository URL - :return: Path to the lock file for this repository - """ - lock_path = ROOT_DIR / settings.CLONE_DIR_PATH / f"{get_repo_hash(url)}.lock" - if not lock_path.parent.exists(): - lock_path.parent.mkdir(parents=True, exist_ok=True) - return lock_path - - def get_unique_clone_path(url: str) -> Path: """ Generate a unique path for cloning a repository. @@ -76,13 +63,13 @@ def clear_tmp_dir(logger: Logger) -> None: raise GeneralError(f"Failed to clear repository clone directory '{path}'") from err -def clone_repository(url: str, logger: Logger) -> Path: +def clone_repository(url: str, destination: Path, logger: Logger) -> Path: """ Clone a Git repository to a unique path. :param url: Repository URL + :param destination: Path to where repository should be cloned :param logger: Logger instance - :param ref: Optional ref to checkout :return: Path to the cloned repository :raises: GitUrlError if URL is invalid :raises: GeneralError if clone fails @@ -91,11 +78,6 @@ def clone_repository(url: str, logger: Logger) -> Path: # Validate URL url = check_git_url(url, logger) - # Get unique path - destination = get_unique_clone_path(url) - - rmtree(destination, ignore_errors=True) - # Clone with retry logic git_clone(url=url, destination=destination, logger=logger) @@ -114,11 +96,10 @@ def get_git_repository(url: str, logger: Logger, ref: str | None = None) -> Path :raises: GeneralError if cloning, fetching, or updating a branch fails :raises: AttributeError if ref doesn't exist """ - lock_path = get_repo_lock_path(url) - with FileLock(lock_path): - destination = get_unique_clone_path(url) + destination = get_unique_clone_path(url) + with FileLock(destination.with_name(f"{destination.name}.lock")): if not destination.exists(): - clone_repository(url, logger) + clone_repository(url, destination, logger) common = Common(logger=logger) @@ -127,23 +108,22 @@ def get_git_repository(url: str, logger: Logger, ref: str | None = None) -> Path _fetch_remote(common, destination, logger) except GeneralError: logger.warning("Unable to fetch remote repository. Trying to clone again.") - clone_repository(url, logger) + rmtree(destination, ignore_errors=True) + clone_repository(url, destination, logger) # If no ref is specified, the default branch is used ref = ref or _get_default_branch(common, destination, logger) + # If the ref is a branch, ensure it's up to date + if _is_branch(common, destination, ref): + _reset_branch(common, destination, ref, logger) + try: common.run(Command("git", "checkout", ref), cwd=destination) except RunError as err: logger.fail(f"Failed to checkout ref '{ref}': {err.stderr}") raise AttributeError(f"Failed to checkout ref '{ref}'") from err - _ensure_no_changes(common, destination, logger) - - # If the ref is a branch, ensure it's up to date - if _is_branch(common, destination, ref): - _update_branch(common, destination, ref, logger) - return destination @@ -191,50 +171,16 @@ def _fetch_remote(common: Common, repo_path: Path, logger: Logger) -> None: raise GeneralError(f"Failed to fetch remote for repository '{repo_path}'") from err -def _update_branch(common: Common, repo_path: Path, branch: str, logger: Logger) -> None: +def _reset_branch(common: Common, repo_path: Path, branch: str, logger: Logger) -> None: """Ensure the specified branch is up to date with its remote counterpart.""" try: - common.run(Command("git", "show-branch", f"origin/{branch}"), cwd=repo_path) - except RunError as err: - logger.fail(f"Branch '{branch}' does not exist in repository '{repo_path}': {err.stderr}") - raise GeneralError(f"Branch {branch}' does not exist in repository '{repo_path}'") from err - try: - # Check if the branch is already up to date - common.run(Command("git", "diff", "--quiet", branch, f"origin/{branch}"), cwd=repo_path) - return - except RunError: - # Branch is not up to date, proceed with update - try: - common.run(Command("git", "reset", "--hard", f"origin/{branch}"), cwd=repo_path) - except RunError as err: - logger.fail( - f"Failed to update branch '{branch}' for repository '{repo_path}': {err.stderr}" - ) - raise GeneralError( - f"Failed to update branch '{branch}' for repository '{repo_path}'" - ) from err - - -def _ensure_no_changes(common: Common, repo_path: Path, logger: Logger) -> None: - """Ensure there are no changes in the repository.""" - try: - output = common.run(Command("git", "status", "--porcelain"), cwd=repo_path) - if not output.stdout or not output.stdout.strip(): - return - logger.warning(f"Repository '{repo_path}' has changes:\n{output.stdout.strip()}") - except RunError as err: - logger.fail(f"Failed to check repository status for '{repo_path}': {err.stderr}") - raise GeneralError(f"Failed to check repository status for '{repo_path}'") from err - - try: - common.run(Command("git", "restore", "."), cwd=repo_path) - common.run(Command("git", "clean", "-fdx"), cwd=repo_path) + common.run(Command("git", "reset", "--hard", f"origin/{branch}"), cwd=repo_path) except RunError as err: logger.fail( - f"Repository '{repo_path}' has changes that could not be reverted: {err.stderr}" + f"Failed to update branch '{branch}' for repository '{repo_path}': {err.stderr}" ) raise GeneralError( - f"Repository '{repo_path}' has changes that could not be reverted" + f"Failed to update branch '{branch}' for repository '{repo_path}'" ) from err From 5aaaa613f692b009e42c823b336351bc8fe6c477 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Filip=20V=C3=A1gner?= Date: Wed, 27 May 2026 10:58:52 +0200 Subject: [PATCH 3/4] squash: show repository status on fail --- src/tmt_web/utils/git_handler.py | 45 ++++++++++++++++++++++++-------- 1 file changed, 34 insertions(+), 11 deletions(-) diff --git a/src/tmt_web/utils/git_handler.py b/src/tmt_web/utils/git_handler.py index 20d22b2..c1bd8bb 100644 --- a/src/tmt_web/utils/git_handler.py +++ b/src/tmt_web/utils/git_handler.py @@ -29,11 +29,6 @@ def _create_hash(text: str): return hashed_text.hexdigest() -def get_repo_hash(url: str) -> str: - url = url.rstrip("/").removesuffix(".git") - return _create_hash(url) - - def get_unique_clone_path(url: str) -> Path: """ Generate a unique path for cloning a repository. @@ -41,7 +36,7 @@ def get_unique_clone_path(url: str) -> Path: :param url: Repository URL :return: Unique path for cloning """ - return ROOT_DIR / settings.CLONE_DIR_PATH / get_repo_hash(url) + return ROOT_DIR / settings.CLONE_DIR_PATH / _create_hash(url.rstrip("/").removesuffix(".git")) def clear_tmp_dir(logger: Logger) -> None: @@ -118,15 +113,22 @@ def get_git_repository(url: str, logger: Logger, ref: str | None = None) -> Path if _is_branch(common, destination, ref): _reset_branch(common, destination, ref, logger) - try: - common.run(Command("git", "checkout", ref), cwd=destination) - except RunError as err: - logger.fail(f"Failed to checkout ref '{ref}': {err.stderr}") - raise AttributeError(f"Failed to checkout ref '{ref}'") from err + _checkout(common, destination, ref, logger) return destination +def _checkout(common: Common, repo_path: Path, ref: str, logger: Logger): + repo_status = _get_repository_status(common, repo_path) + try: + common.run(Command("git", "checkout", ref), cwd=repo_path) + except RunError as err: + logger.fail(f"Failed to checkout ref '{ref}': {err.stderr}") + if repo_status: + logger.fail(f"Previous repository status:\n{repo_status}") + raise AttributeError(f"Failed to checkout ref '{ref}'") from err + + def _get_default_branch(common: Common, repo_path: Path, logger: Logger) -> str: """Determine the default branch of a Git repository using a remote HEAD.""" try: @@ -152,6 +154,7 @@ def _get_default_branch(common: Common, repo_path: Path, logger: Logger) -> str: def _fetch_remote(common: Common, repo_path: Path, logger: Logger) -> None: """Fetch updates from the remote repository.""" + repo_status = _get_repository_status(common, repo_path) try: # Fetch all branches and tags, prune deleted ones common.run( @@ -168,17 +171,22 @@ def _fetch_remote(common: Common, repo_path: Path, logger: Logger) -> None: ) except RunError as err: logger.fail(f"Failed to fetch remote for repository '{repo_path}': {err.stderr}") + if repo_status: + logger.fail(f"Previous repository status:\n{repo_status}") raise GeneralError(f"Failed to fetch remote for repository '{repo_path}'") from err def _reset_branch(common: Common, repo_path: Path, branch: str, logger: Logger) -> None: """Ensure the specified branch is up to date with its remote counterpart.""" + repo_status = _get_repository_status(common, repo_path) try: common.run(Command("git", "reset", "--hard", f"origin/{branch}"), cwd=repo_path) except RunError as err: logger.fail( f"Failed to update branch '{branch}' for repository '{repo_path}': {err.stderr}" ) + if repo_status: + logger.fail(f"Previous repository status:\n{repo_status}") raise GeneralError( f"Failed to update branch '{branch}' for repository '{repo_path}'" ) from err @@ -195,3 +203,18 @@ def _is_branch(common: Common, repo_path: Path, ref: str) -> bool: return True except RunError: return False + + +def _get_repository_status(common: Common, repo_path: Path) -> str | None: + """Get the current status of a Git repository.""" + try: + log_result = common.run(Command("git", "log", "-1"), cwd=repo_path) + status_result = common.run(Command("git", "status"), cwd=repo_path) + + outputs = [output.strip() for output in (log_result.stdout, status_result.stdout) if output] + if outputs: + return "\n".join(outputs) + + return None + except RunError: + return None From 23f1720827ae28719ada14f37c09ecb69d57a8b6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Filip=20V=C3=A1gner?= Date: Wed, 27 May 2026 17:32:22 +0200 Subject: [PATCH 4/4] squash: fix tests --- tests/unit/test_git_handler.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/tests/unit/test_git_handler.py b/tests/unit/test_git_handler.py index 99ecebc..10e8536 100644 --- a/tests/unit/test_git_handler.py +++ b/tests/unit/test_git_handler.py @@ -74,7 +74,9 @@ def test_get_unique_clone_path_valid_url(self): def test_clone_repository_invalid_url(self, logger): """Test cloning with invalid repository URL.""" with pytest.raises(GitUrlError): - git_handler.clone_repository(self.INVALID_REPO, logger) + git_handler.clone_repository( + self.INVALID_REPO, git_handler.get_unique_clone_path(self.INVALID_REPO), logger + ) @pytest.mark.usefixtures("_clean_repo_dir") def test_get_git_repository_new(self, logger): @@ -124,7 +126,10 @@ def test_get_git_repository_existing_checkout_error(self, mocker, logger): def side_effect(cmd, *args, **kwargs): if cmd._command == ["git", "checkout", "invalid-branch"]: raise RunError("Command failed", cmd, 1) - return mocker.DEFAULT + result = mocker.MagicMock() + result.stdout = None + result.stderr = None + return result mocker.patch("tmt.utils.Command.run", side_effect=side_effect, autospec=True)