-
-
Notifications
You must be signed in to change notification settings - Fork 281
fix: Add git worktree support for Docker environments #6456
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
- Add _is_git_worktree() method to detect worktree environments - Wrap git fetch in try-catch to handle GitCommandError gracefully - Add informative warning messages for worktree scenarios - Update exception handling to catch both InvalidGitRepositoryError and GitCommandError - Add comprehensive unit tests for worktree detection and error handling Fixes #6455
- Loading branch information
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -701,7 +701,7 @@ def collect_files(self): | |
| # List files using git diff | ||
| try: | ||
| all_files = self.list_files_git_diff() | ||
| except git.InvalidGitRepositoryError as git_err: | ||
| except (git.InvalidGitRepositoryError, git.exc.GitCommandError) as git_err: | ||
| logging.warning( | ||
| "Unable to list updated files from git diff. Switch to VALIDATE_ALL_CODE_BASE=true" | ||
| ) | ||
|
|
@@ -788,12 +788,45 @@ def collect_files(self): | |
| if len(linter.files) == 0 and linter.lint_all_files is False: | ||
| linter.is_active = False | ||
|
|
||
| def _is_git_worktree(self, repo): | ||
| """ | ||
| Detect if the current git repository is a worktree. | ||
|
|
||
| In a worktree, the .git directory is actually a file containing | ||
| a gitdir reference to the main repository's worktrees directory. | ||
|
|
||
| Args: | ||
| repo: GitPython Repo object | ||
|
|
||
| Returns: | ||
| bool: True if this is a worktree, False otherwise | ||
| """ | ||
| try: | ||
| git_dir = repo.git_dir | ||
| # Check if .git is a file (worktree) or directory (regular repo) | ||
| git_path = os.path.join(repo.working_dir, '.git') | ||
| if os.path.isfile(git_path): | ||
| # It's a worktree - .git is a file containing gitdir reference | ||
| return True | ||
| # Also check if git_dir contains 'worktrees' in the path | ||
| if 'worktrees' in git_dir: | ||
| return True | ||
| except Exception as e: | ||
| logging.debug(f"Error checking for worktree: {str(e)}") | ||
| return False | ||
|
|
||
| def list_files_git_diff(self): | ||
| # List all updated files from git | ||
| logging.info( | ||
| "Listing updated files in [" + self.github_workspace + "] using git diff." | ||
| ) | ||
| repo = git.Repo(os.path.realpath(self.github_workspace)) | ||
|
|
||
| # Check if we're in a git worktree | ||
| is_worktree = self._is_git_worktree(repo) | ||
| if is_worktree: | ||
| logging.info("Detected git worktree environment") | ||
|
|
||
| # Add auth header if necessary | ||
| if config.get(self.request_id, "GIT_AUTHORIZATION_BEARER", "") != "": | ||
| auth_bearer = "Authorization: Bearer " + config.get( | ||
|
|
@@ -810,7 +843,25 @@ def list_files_git_diff(self): | |
| ) | ||
| local_ref = f"refs/remotes/{default_branch_remote}" | ||
| # Try to fetch default_branch from origin, because it isn't cached locally. | ||
| repo.git.fetch("origin", f"{remote_ref}:{local_ref}") | ||
| try: | ||
| repo.git.fetch("origin", f"{remote_ref}:{local_ref}") | ||
| except git.exc.GitCommandError as fetch_err: | ||
| # Handle worktree and other fetch errors gracefully | ||
| logging.warning( | ||
| f"Unable to fetch {remote_ref} from origin: {str(fetch_err)}" | ||
| ) | ||
| if is_worktree: | ||
| logging.warning( | ||
| "Git worktree detected - this is a known issue when running in Docker. " | ||
| "The worktree's .git file contains an absolute path that is invalid inside the container. " | ||
| "Continuing without fetch - ensure your repository is up-to-date before running MegaLinter." | ||
| ) | ||
| else: | ||
| logging.warning( | ||
| "Continuing without fetch - this may result in comparing against a stale branch. " | ||
| "Consider setting VALIDATE_ALL_CODEBASE=true to avoid git operations." | ||
| ) | ||
| # Continue without the fetch - use whatever refs are available | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bug: BugWhen git fetch fails (caught at line 848), the code continues to execute and attempts to use |
||
| # Make git diff to list files (and exclude symlinks) | ||
| try: | ||
| # Use optimized way from https://github.com/oxsecurity/megalinter/pull/3472 | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,166 @@ | ||
| #!/usr/bin/env python3 | ||
| """ | ||
| Unit tests for Git worktree handling in MegaLinter | ||
|
|
||
| """ | ||
| import os | ||
| import tempfile | ||
| import unittest | ||
| from unittest.mock import MagicMock, Mock, patch | ||
|
|
||
| import git | ||
|
|
||
| from megalinter import Megalinter | ||
|
|
||
|
|
||
| class worktree_test(unittest.TestCase): | ||
| """Test Git worktree detection and error handling""" | ||
|
|
||
| def test_is_git_worktree_detection_for_regular_repo(self): | ||
| """Test that a regular repository is NOT detected as a worktree""" | ||
| # Create a mock repo that represents a regular repository | ||
| mock_repo = Mock() | ||
| mock_repo.git_dir = "/path/to/repo/.git" | ||
| mock_repo.working_dir = "/path/to/repo" | ||
|
|
||
| # Create a temporary directory to simulate a regular repo | ||
| with tempfile.TemporaryDirectory() as tmpdir: | ||
| git_dir = os.path.join(tmpdir, ".git") | ||
| os.makedirs(git_dir) | ||
|
|
||
| mock_repo.working_dir = tmpdir | ||
| mock_repo.git_dir = git_dir | ||
|
|
||
| # Create MegaLinter instance | ||
| megalinter = Megalinter({"workspace": tmpdir, "cli": False}) | ||
|
|
||
| # Test worktree detection | ||
| is_worktree = megalinter._is_git_worktree(mock_repo) | ||
|
|
||
| self.assertFalse( | ||
| is_worktree, | ||
| "Regular repository should NOT be detected as a worktree" | ||
| ) | ||
|
|
||
| def test_is_git_worktree_detection_for_worktree_file(self): | ||
| """Test that a worktree (with .git as a file) IS detected""" | ||
| # Create a mock repo that represents a worktree | ||
| mock_repo = Mock() | ||
| mock_repo.git_dir = "/path/to/repo/.git/worktrees/my-worktree" | ||
|
|
||
| # Create a temporary directory to simulate a worktree | ||
| with tempfile.TemporaryDirectory() as tmpdir: | ||
| # Create .git as a FILE (worktree indicator) | ||
| git_file = os.path.join(tmpdir, ".git") | ||
| with open(git_file, "w") as f: | ||
| f.write("gitdir: /path/to/repo/.git/worktrees/my-worktree\n") | ||
|
|
||
| mock_repo.working_dir = tmpdir | ||
|
|
||
| # Create MegaLinter instance | ||
| megalinter = Megalinter({"workspace": tmpdir, "cli": False}) | ||
|
|
||
| # Test worktree detection | ||
| is_worktree = megalinter._is_git_worktree(mock_repo) | ||
|
|
||
| self.assertTrue( | ||
| is_worktree, | ||
| "Worktree (with .git file) should be detected as a worktree" | ||
| ) | ||
|
|
||
| def test_is_git_worktree_detection_by_path(self): | ||
| """Test that a worktree is detected by 'worktrees' in git_dir path""" | ||
| # Create a mock repo with 'worktrees' in the path | ||
| mock_repo = Mock() | ||
| mock_repo.git_dir = "/path/to/repo/.git/worktrees/my-worktree" | ||
| mock_repo.working_dir = "/path/to/worktree" | ||
|
|
||
| # Create a temporary directory | ||
| with tempfile.TemporaryDirectory() as tmpdir: | ||
| mock_repo.working_dir = tmpdir | ||
| git_dir_path = os.path.join(tmpdir, ".git") | ||
| os.makedirs(git_dir_path) | ||
|
|
||
| # Override git_dir to have 'worktrees' in path | ||
| mock_repo.git_dir = "/main/.git/worktrees/test" | ||
|
|
||
| # Create MegaLinter instance | ||
| megalinter = Megalinter({"workspace": tmpdir, "cli": False}) | ||
|
|
||
| # Test worktree detection | ||
| is_worktree = megalinter._is_git_worktree(mock_repo) | ||
|
|
||
| self.assertTrue( | ||
| is_worktree, | ||
| "Worktree should be detected by 'worktrees' in git_dir path" | ||
| ) | ||
|
|
||
| @patch("git.Repo") | ||
| def test_git_fetch_error_handling_in_worktree(self, mock_repo_class): | ||
| """Test that git fetch errors are properly handled in worktrees""" | ||
| # Create a temporary directory | ||
| with tempfile.TemporaryDirectory() as tmpdir: | ||
| # Create .git as a file to simulate worktree | ||
| git_file = os.path.join(tmpdir, ".git") | ||
| with open(git_file, "w") as f: | ||
| f.write("gitdir: /main/.git/worktrees/test\n") | ||
|
|
||
| # Mock the Repo object | ||
| mock_repo_instance = Mock() | ||
| mock_repo_instance.git_dir = "/main/.git/worktrees/test" | ||
| mock_repo_instance.working_dir = tmpdir | ||
| mock_repo_instance.refs = [] | ||
|
|
||
| # Mock git.fetch to raise GitCommandError | ||
| mock_repo_instance.git.fetch.side_effect = git.exc.GitCommandError( | ||
| "git fetch", | ||
| 128, | ||
| stderr="fatal: not a git repository: /host/path/.git/worktrees/test" | ||
| ) | ||
|
|
||
| mock_repo_class.return_value = mock_repo_instance | ||
|
|
||
| # Create MegaLinter instance | ||
| megalinter = Megalinter({"workspace": tmpdir, "cli": False}) | ||
|
|
||
| # Try to list files using git diff - should not raise an exception | ||
| try: | ||
| # This should handle the error gracefully | ||
| files = megalinter.list_files_git_diff() | ||
| # If we get here, the error was handled | ||
| self.assertTrue( | ||
| True, | ||
| "Git fetch error should be caught and handled" | ||
| ) | ||
| except git.exc.GitCommandError: | ||
| self.fail( | ||
| "GitCommandError should be caught and handled, not raised" | ||
| ) | ||
|
|
||
| def test_worktree_detection_handles_exceptions(self): | ||
| """Test that worktree detection handles exceptions gracefully""" | ||
| # Create a mock repo that raises an exception | ||
| mock_repo = Mock() | ||
| mock_repo.git_dir = Mock(side_effect=Exception("Test exception")) | ||
|
|
||
| with tempfile.TemporaryDirectory() as tmpdir: | ||
| # Create MegaLinter instance | ||
| megalinter = Megalinter({"workspace": tmpdir, "cli": False}) | ||
|
|
||
| # Test worktree detection - should not raise exception | ||
| try: | ||
| is_worktree = megalinter._is_git_worktree(mock_repo) | ||
| # Should return False when exception occurs | ||
| self.assertFalse( | ||
| is_worktree, | ||
| "Should return False when exception occurs during detection" | ||
| ) | ||
| except Exception: | ||
| self.fail( | ||
| "Worktree detection should handle exceptions gracefully" | ||
| ) | ||
|
|
||
|
|
||
| if __name__ == "__main__": | ||
| unittest.main() | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Fix overly broad worktree directory detection
The worktree detection check
if 'worktrees' in git_dir:is too broad and will produce false positives. It will incorrectly identify regular git repositories as worktrees if their path happens to contain the substring 'worktrees' anywhere (e.g.,/home/user/my_worktrees_project/.gitor/path/to/worktrees/.git). The check should be more specific, such as checking if the path contains/.git/worktrees/or matches the actual worktree directory structure pattern.