Fix git repository handling

[therazix]

This PR fixes how tmt web handles git repositories. A cloned local repository is now properly updated when changes occur in the remote repository. Also, when no ref is specified, the default branch will now be used instead of reusing the ref from the previous checkout.

Fixes #21
Related: TFT-3576

[codiumai-pr-agent-free]

User description

This PR fixes how tmt web handles git repositories. A cloned local repository is now properly updated when changes occur in the remote repository. Also, when no ref is specified, the default branch will now be used instead of reusing the ref from the previous checkout.

Fixes #21
Related: TFT-3576

---

PR Type

Bug fix, Enhancement

---

Description

• Improved git repository handling with proper remote updates

• Fixed default branch detection when no ref specified

• Added branch synchronization with remote counterparts

• Enhanced test mocking for checkout error scenarios

---

Diagram Walkthrough

flowchart LR
  A["Clone Repository"] --> B["Fetch Remote Updates"]
  B --> C["Determine Default Branch"]
  C --> D["Checkout Ref/Branch"]
  D --> E["Update Branch if Needed"]

Loading

File Walkthrough

	Relevant files
Bug fix	git_handler.py Enhanced git repository handling with remote sync                src/tmt_web/utils/git_handler.py Removed ref parameter from clone_repository function Enhanced get_git_repository with remote fetching and branch updating Added helper functions for default branch detection and branch management Improved error handling for git operations +90/-19
Tests	test_git_handler.py Fixed test mocking for git checkout errors                              tests/unit/test_git_handler.py Fixed test mocking to properly handle selective command failures Updated mock side_effect to use conditional logic for git checkout +6/-2

---

[codiumai-pr-agent-free]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶 21 - Partially compliant Compliant requirements: Properly handle git repositories to fix the root cause of the issue Non-compliant requirements: Provide more detailed error messages when a plan is not found Requires further human verification: Fix the issue where plan details fail to show for a specific URL - need to verify if the git repository handling changes actually fix the specific URL mentioned in the ticket

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Error Handling The error messages in _get_default_branch function are duplicated. The first failure message is logged and then immediately raised as an exception, which might lead to redundant error reporting. logger.fail(f"Failed to determine default branch for repository '{repo_path}'") raise GeneralError(f"Failed to determine default branch for repository '{repo_path}'") Missing Documentation Several new helper functions (_fetch_remote, _update_branch, _is_branch_up_to_date, _is_branch) lack proper docstring parameters and return value documentation. def _fetch_remote(common: Common, repo_path: Path, logger: Logger) -> None: """ Fetch updates from the remote repository. """ try: common.run(Command("git", "fetch"), cwd=repo_path) except RunError as err: logger.fail(f"Failed to fetch remote for repository '{repo_path}'") 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: """ Update the specified branch of a Git repository to match the remote counterpart. """ 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}'") raise GeneralError(f"Failed to update branch '{branch}' for repository '{repo_path}'") from err def _is_branch_up_to_date(common: Common, repo_path: Path, branch: str) -> bool: """ Compare the specified branch of a Git repository with its remote counterpart. :return: True if the branch is up to date with the remote, False otherwise. """ try: common.run(Command("git", "diff", "--quiet", branch, f"origin/{branch}"), cwd=repo_path) return True except RunError: return False def _is_branch(common: Common, repo_path: Path, ref: str) -> bool: """ Check if the given ref is a branch in the Git repository. :return: True if the ref is a branch, False otherwise. """ try: common.run(Command("git", "show-ref", "-q", "--verify", f"refs/heads/{ref}"), cwd=repo_path) return True except RunError: return False

[codiumai-pr-agent-free]

CI Feedback 🧐

A test triggered by this PR failed. Here is an AI-generated analysis of the failure:

Action: pre-commit

Failed stage: Run pre-commit/action@v3.0.1 [❌]

Failure summary: The action failed due to two pre-commit hook failures: 1. end-of-file-fixer hook failed (line 266) - The hook modified the file src/tmt_web/utils/git_handler.py to fix end-of-file issues. 2. ruff and ruff-format hooks failed (lines 275, 342) with several code style violations: - Line length violations (E501) in src/tmt_web/utils/git_handler.py on lines 120, 129, and 151 - Docstring formatting issue (D200) in src/tmt_web/utils/git_handler.py on line 144 - Blank line after function issue (D202) - Unused import (F401) in tests/unit/test_git_handler.py

Relevant error logs: 1: ##[group]Runner Image Provisioner 2: Hosted Compute Agent ... 251: [INFO]�[m Initializing environment for https://github.com/charliermarsh/ruff-pre-commit. 252: [INFO]�[m Installing environment for https://github.com/pre-commit/pre-commit-hooks. 253: [INFO]�[m Once installed this environment will be reused. 254: [INFO]�[m This may take a few minutes... 255: [INFO]�[m Installing environment for https://github.com/pre-commit/mirrors-mypy. 256: [INFO]�[m Once installed this environment will be reused. 257: [INFO]�[m This may take a few minutes... 258: [INFO]�[m Installing environment for https://github.com/charliermarsh/ruff-pre-commit. 259: [INFO]�[m Once installed this environment will be reused. 260: [INFO]�[m This may take a few minutes... 261: check for merge conflicts................................................�[42mPassed�[m 262: check for broken symlinks............................(no files to check)�[46;30mSkipped�[m 263: check toml...............................................................�[42mPassed�[m 264: detect destroyed symlinks................................................�[42mPassed�[m 265: detect private key.......................................................�[42mPassed�[m 266: fix end of files.........................................................�[41mFailed�[m 267: �[2m- hook id: end-of-file-fixer�[m 268: �[2m- exit code: 1�[m 269: �[2m- files were modified by this hook�[m 270: Fixing src/tmt_web/utils/git_handler.py 271: mixed line ending........................................................�[42mPassed�[m 272: don't commit to branch..................................................�[43;30mSkipped�[m 273: trim trailing whitespace.................................................�[42mPassed�[m 274: mypy.....................................................................�[42mPassed�[m 275: ruff.....................................................................�[41mFailed�[m 276: �[2m- hook id: ruff�[m ... 286: �[1m�[94m119 |�[0m try: 287: �[1m�[94m120 |�[0m output = common.run(Command("git", "symbolic-ref", "refs/remotes/origin/HEAD"), cwd=repo_path) 288: �[1m�[94m|�[0m 289: �[1m�[94m= �[0m�[1m�[96mhelp�[0m: Reformat to one line 290: �[1msrc/tmt_web/utils/git_handler.py�[0m�[36m:�[0m120�[36m:�[0m101�[36m:�[0m �[1;31mE501�[0m Line too long (102 > 100) 291: �[1m�[94m|�[0m 292: �[1m�[94m118 |�[0m """ 293: �[1m�[94m119 |�[0m try: 294: �[1m�[94m120 |�[0m output = common.run(Command("git", "symbolic-ref", "refs/remotes/origin/HEAD"), cwd=repo_path) 295: �[1m�[94m|�[0m �[1m�[91m^^�[0m �[1m�[91mE501�[0m 296: �[1m�[94m121 |�[0m if output.stdout: 297: �[1m�[94m122 |�[0m return output.stdout.strip().removeprefix('refs/remotes/origin/') 298: �[1m�[94m|�[0m 299: �[1msrc/tmt_web/utils/git_handler.py�[0m�[36m:�[0m129�[36m:�[0m101�[36m:�[0m �[1;31mE501�[0m Line too long (103 > 100) 300: �[1m�[94m|�[0m 301: �[1m�[94m127 |�[0m except RunError as err: 302: �[1m�[94m128 |�[0m logger.fail(f"Failed to determine default branch for repository '{repo_path}'") 303: �[1m�[94m129 |�[0m raise GeneralError(f"Failed to determine default branch for repository '{repo_path}'") from err 304: �[1m�[94m|�[0m �[1m�[91m^^^�[0m �[1m�[91mE501�[0m ... 315: �[1m�[94m|�[0m 316: �[1m�[94m= �[0m�[1m�[96mhelp�[0m: Reformat to one line 317: �[1msrc/tmt_web/utils/git_handler.py�[0m�[36m:�[0m144�[36m:�[0m5�[36m:�[0m �[1;31mD200�[0m One-line docstring should fit on one line 318: �[1m�[94m|�[0m 319: �[1m�[94m143 |�[0m def _update_branch(common: Common, repo_path: Path, branch: str, logger: Logger) -> None: 320: �[1m�[94m144 |�[0m �[1m�[91m/�[0m """ 321: �[1m�[94m145 |�[0m �[1m�[91m|�[0m Update the specified branch of a Git repository to match the remote counterpart. 322: �[1m�[94m146 |�[0m �[1m�[91m|�[0m """ 323: �[1m�[94m|�[0m �[1m�[91m|_______^�[0m �[1m�[91mD200�[0m 324: �[1m�[94m147 |�[0m try: 325: �[1m�[94m148 |�[0m common.run(Command("git", "reset", "--hard", f"origin/{branch}"), cwd=repo_path) 326: �[1m�[94m|�[0m 327: �[1m�[94m= �[0m�[1m�[96mhelp�[0m: Reformat to one line 328: �[1msrc/tmt_web/utils/git_handler.py�[0m�[36m:�[0m151�[36m:�[0m101�[36m:�[0m �[1;31mE501�[0m Line too long (103 > 100) 329: �[1m�[94m|�[0m 330: �[1m�[94m149 |�[0m except RunError as err: 331: �[1m�[94m150 |�[0m logger.fail(f"Failed to update branch '{branch}' for repository '{repo_path}'") 332: �[1m�[94m151 |�[0m raise GeneralError(f"Failed to update branch '{branch}' for repository '{repo_path}'") from err 333: �[1m�[94m|�[0m �[1m�[91m^^^�[0m �[1m�[91mE501�[0m 334: �[1m�[94m|�[0m 335: �[1;32mFixed 2 errors:�[0m 336: �[36m-�[0m �[1msrc/tmt_web/utils/git_handler.py�[0m�[36m:�[0m 337: 1 × �[1;31mD202�[0m (blank-line-after-function) 338: �[36m-�[0m �[1mtests/unit/test_git_handler.py�[0m�[36m:�[0m 339: 1 × �[1;31mF401�[0m (unused-import) 340: Found 8 errors (2 fixed, 6 remaining). 341: No fixes available (3 hidden fixes can be enabled with the `--unsafe-fixes` option). 342: ruff-format..............................................................�[41mFailed�[m 343: �[2m- hook id: ruff-format�[m ... 353: �[1m+++ b/src/tmt_web/utils/git_handler.py�[m 354: �[36m@@ -116,18 +116,21 @@�[m �[mdef _get_default_branch(common: Common, repo_path: Path, logger: Logger) -> str:�[m 355: """�[m 356: Determine the default branch of a Git repository using a remote HEAD.�[m 357: """�[m 358: �[31m-�[m 359: try:�[m 360: �[31m- output = common.run(Command("git", "symbolic-ref", "refs/remotes/origin/HEAD"), cwd=repo_path)�[m 361: �[32m+�[m�[32m output = common.run(�[m 362: �[32m+�[m�[32m Command("git", "symbolic-ref", "refs/remotes/origin/HEAD"), cwd=repo_path�[m 363: �[32m+�[m�[32m )�[m 364: if output.stdout:�[m 365: �[31m- return output.stdout.strip().removeprefix('refs/remotes/origin/')�[m 366: �[32m+�[m�[32m return output.stdout.strip().removeprefix("refs/remotes/origin/")�[m 367: �[m 368: logger.fail(f"Failed to determine default branch for repository '{repo_path}'")�[m 369: raise GeneralError(f"Failed to determine default branch for repository '{repo_path}'")�[m 370: �[m 371: except RunError as err:�[m 372: logger.fail(f"Failed to determine default branch for repository '{repo_path}'")�[m 373: �[31m- raise GeneralError(f"Failed to determine default branch for repository '{repo_path}'") from err�[m 374: �[32m+�[m�[32m raise GeneralError(�[m 375: �[32m+�[m�[32m f"Failed to determine default branch for repository '{repo_path}'"�[m 376: �[32m+�[m�[32m ) from err�[m 377: �[m 378: �[m 379: def _fetch_remote(common: Common, repo_path: Path, logger: Logger) -> None:�[m 380: �[36m@@ -149,7 +152,9 @@�[m �[mdef _update_branch(common: Common, repo_path: Path, branch: str, logger: Logger)�[m 381: common.run(Command("git", "reset", "--hard", f"origin/{branch}"), cwd=repo_path)�[m 382: except RunError as err:�[m 383: logger.fail(f"Failed to update branch '{branch}' for repository '{repo_path}'")�[m 384: �[31m- raise GeneralError(f"Failed to update branch '{branch}' for repository '{repo_path}'") from err�[m 385: �[32m+�[m�[32m raise GeneralError(�[m 386: �[32m+�[m�[32m f"Failed to update branch '{branch}' for repository '{repo_path}'"�[m 387: �[32m+�[m�[32m ) from err�[m 388: �[m 389: �[m 390: def _is_branch_up_to_date(common: Common, repo_path: Path, branch: str) -> bool:�[m 391: �[36m@@ -176,4 +181,3 @@�[m �[mdef _is_branch(common: Common, repo_path: Path, ref: str) -> bool:�[m 392: return True�[m 393: except RunError:�[m 394: return False�[m 395: �[31m-�[m 396: �[1mdiff --git a/tests/unit/test_git_handler.py b/tests/unit/test_git_handler.py�[m 397: �[1mindex d0bc14b..99ecebc 100644�[m 398: �[1m--- a/tests/unit/test_git_handler.py�[m 399: �[1m+++ b/tests/unit/test_git_handler.py�[m 400: �[36m@@ -3,7 +3,7 @@�[m �[mfrom pathlib import Path�[m 401: �[m 402: import pytest�[m 403: import tmt�[m 404: �[31m-from tmt.utils import Command, GeneralError, GitUrlError, RunError�[m 405: �[32m+�[m�[32mfrom tmt.utils import GeneralError, GitUrlError, RunError�[m 406: �[m 407: from tmt_web import settings�[m 408: from tmt_web.utils import git_handler�[m 409: ##[error]Process completed with exit code 1. 410: Post job cleanup.

[codiumai-pr-agent-free]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	✅ Check remote branch existence Suggestion Impact: The commit completely restructured the branch update logic. Instead of having a separate _is_branch_up_to_date function, it integrated the branch comparison directly into the _update_branch function and removed the conditional check in the caller. This addresses the suggestion's concern about handling non-existent remote branches by only attempting the diff if we've already reached the update branch function. code diff: - if _is_branch(common, destination, ref) and not _is_branch_up_to_date(common, destination, ref): + if _is_branch(common, destination, ref): _update_branch(common, destination, ref, logger) return destination @@ -119,7 +119,9 @@ Command("git", "symbolic-ref", "refs/remotes/origin/HEAD"), cwd=repo_path ) if output.stdout: - return output.stdout.strip().removeprefix("refs/remotes/origin/") + match = re.search(r"refs/remotes/origin/(.*)", output.stdout.strip()) + if match: + return match.group(1) logger.fail(f"Failed to determine default branch for repository '{repo_path}'") raise GeneralError(f"Failed to determine default branch for repository '{repo_path}'") @@ -141,27 +143,20 @@ def _update_branch(common: Common, repo_path: Path, branch: str, logger: Logger) -> None: - """Update the specified branch of a Git repository to match the remote counterpart.""" + """Ensure the specified branch is up to date with its remote counterpart.""" 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}'") - raise GeneralError( - f"Failed to update branch '{branch}' for repository '{repo_path}'" - ) from err - - -def _is_branch_up_to_date(common: Common, repo_path: Path, branch: str) -> bool: - """ - Compare the specified branch of a Git repository with its remote counterpart. - - :return: True if the branch is up to date with the remote, False otherwise. - """ - try: + # Check if the branch is already up to date common.run(Command("git", "diff", "--quiet", branch, f"origin/{branch}"), cwd=repo_path) - return True + return except RunError: - return False + # 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}'") + raise GeneralError( + f"Failed to update branch '{branch}' for repository '{repo_path}'" + ) from err The current implementation doesn't handle the case where the remote branch doesn't exist, which would cause the diff command to fail but for a different reason than the branch being out of date. Add a check to verify the remote branch exists. src/tmt_web/utils/git_handler.py [155-165] def _is_branch_up_to_date(common: Common, repo_path: Path, branch: str) -> bool: """ Compare the specified branch of a Git repository with its remote counterpart. :return: True if the branch is up to date with the remote, False otherwise. """ + # First check if remote branch exists + try: + common.run(Command("git", "show-ref", "-q", "--verify", f"refs/remotes/origin/{branch}"), cwd=repo_path) + except RunError: + # Remote branch doesn't exist, can't compare + return True + + # Compare local and remote branches try: common.run(Command("git", "diff", "--quiet", branch, f"origin/{branch}"), cwd=repo_path) return True except RunError: return False [Suggestion processed] Suggestion importance[1-10]: 7 __ Why: This suggestion correctly identifies a scenario where a local branch has no remote counterpart, which would cause an unhandled error, and provides a fix to prevent the application from crashing.	Medium
	Add fallback for default branch The symbolic-ref command will fail if the remote HEAD reference doesn't exist, which happens with fresh clones before fetching. Add a fallback mechanism to handle this case by checking for common default branch names. src/tmt_web/utils/git_handler.py [115-130] 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: output = common.run(Command("git", "symbolic-ref", "refs/remotes/origin/HEAD"), cwd=repo_path) if output.stdout: return output.stdout.strip().removeprefix('refs/remotes/origin/') + # Fallback to common default branch names + for branch in ["main", "master"]: + try: + common.run(Command("git", "show-ref", "-q", "--verify", f"refs/remotes/origin/{branch}"), cwd=repo_path) + logger.debug(f"Using fallback default branch '{branch}'") + return branch + except RunError: + continue + logger.fail(f"Failed to determine default branch for repository '{repo_path}'") raise GeneralError(f"Failed to determine default branch for repository '{repo_path}'") except RunError as err: + # Fallback to common default branch names + for branch in ["main", "master"]: + try: + common.run(Command("git", "show-ref", "-q", "--verify", f"refs/remotes/origin/{branch}"), cwd=repo_path) + logger.debug(f"Using fallback default branch '{branch}'") + return branch + except RunError: + continue + logger.fail(f"Failed to determine default branch for repository '{repo_path}'") raise GeneralError(f"Failed to determine default branch for repository '{repo_path}'") from err Apply / Chat Suggestion importance[1-10]: 6 __ Why: The suggestion correctly identifies that git symbolic-ref can fail if origin/HEAD is not set by the remote, and proposes a robust fallback mechanism, improving the reliability of default branch detection.	Low
Update

[LecrisUT]

Just minor comments. Looks like functionally it would work fine.

[psss]

Thanks for the reviews!