CM-51935 - Fix pre-receive hook for bare repositories (#345)

Author: MarshalX

cycode/cli/files_collector/commit_range_documents.py

@@ -212,24 +212,33 @@ def parse_pre_receive_input() -> str:
 
 
 def get_diff_file_path(diff: 'Diff', relative: bool = False, repo: Optional['Repo'] = None) -> Optional[str]:
-    if relative:
-        # relative to the repository root
-        return diff.b_path if diff.b_path else diff.a_path
-
-    # Try blob-based paths first (most reliable when available)
-    if diff.b_blob:
-        return diff.b_blob.abspath
-    if diff.a_blob:
-        return diff.a_blob.abspath
-
-    # Fallback: construct an absolute path from a relative path
-    # This handles renames and other cases where blobs might be None
-    if repo and repo.working_tree_dir:
-        target_path = diff.b_path if diff.b_path else diff.a_path
-        if target_path:
-            return os.path.abspath(os.path.join(repo.working_tree_dir, target_path))
-
-    return None
+    """Get the file path from a git Diff object.
+
+    Args:
+        diff: Git Diff object representing the file change
+        relative: If True, return the path relative to the repository root;
+            otherwise, return an absolute path IF possible
+        repo: Optional Git Repo object, used to resolve absolute paths
+
+    Note:
+        It tries to get the absolute path, falling back to relative paths. `relative` flag forces relative paths.
+
+        One case of relative paths is when the repository is bare and does not have a working tree directory.
+    """
+    # try blob-based paths first (most reliable when available)
+    blob = diff.b_blob if diff.b_blob else diff.a_blob
+    if blob:
+        if relative:
+            return blob.path
+        if repo and repo.working_tree_dir:
+            return blob.abspath
+
+    path = diff.b_path if diff.b_path else diff.a_path  # relative path within the repo
+    if not relative and path and repo and repo.working_tree_dir:
+        # convert to the absolute path using the repo's working tree directory
+        path = os.path.join(repo.working_tree_dir, path)
+
+    return path
 
 
 def get_diff_file_content(diff: 'Diff') -> str:

tests/cli/files_collector/test_commit_range_documents.py

@@ -155,3 +155,184 @@ def test_git_mv_pre_commit_scan() -> None:
         for diff in diff_index:
             file_path = get_path_by_os(get_diff_file_path(diff, repo=repo))
             assert file_path == renamed_path
+
+
+class TestGetDiffFilePath:
+    """Test the get_diff_file_path function with various diff scenarios."""
+
+    def test_diff_with_b_blob_and_working_tree(self) -> None:
+        """Test that blob.abspath is returned when b_blob is available and repo has a working tree."""
+        with temporary_git_repository() as (temp_dir, repo):
+            test_file = os.path.join(temp_dir, 'test_file.py')
+            with open(test_file, 'w') as f:
+                f.write("print('original content')")
+
+            repo.index.add(['test_file.py'])
+            repo.index.commit('Initial commit')
+
+            with open(test_file, 'w') as f:
+                f.write("print('modified content')")
+
+            repo.index.add(['test_file.py'])
+
+            # Get diff of staged changes
+            head_ref = get_safe_head_reference_for_diff(repo)
+            diff_index = repo.index.diff(head_ref)
+
+            assert len(diff_index) == 1
+
+            result = get_diff_file_path(diff_index[0], repo=repo)
+
+            assert result == test_file
+            assert os.path.isabs(result)
+
+    def test_diff_with_a_blob_only_and_working_tree(self) -> None:
+        """Test that a_blob.abspath is used when b_blob is None but a_blob exists."""
+        with temporary_git_repository() as (temp_dir, repo):
+            test_file = os.path.join(temp_dir, 'to_delete.py')
+            with open(test_file, 'w') as f:
+                f.write("print('will be deleted')")
+
+            repo.index.add(['to_delete.py'])
+            repo.index.commit('Initial commit')
+
+            os.remove(test_file)
+            repo.index.remove(['to_delete.py'])
+
+            # Get diff of staged changes
+            head_ref = get_safe_head_reference_for_diff(repo)
+            diff_index = repo.index.diff(head_ref)
+
+            assert len(diff_index) == 1
+
+            result = get_diff_file_path(diff_index[0], repo=repo)
+
+            assert result == test_file
+            assert os.path.isabs(result)
+
+    def test_diff_with_b_path_fallback(self) -> None:
+        """Test that b_path is used with working_tree_dir when blob is not available."""
+        with temporary_git_repository() as (temp_dir, repo):
+            test_file = os.path.join(temp_dir, 'new_file.py')
+            with open(test_file, 'w') as f:
+                f.write("print('new file')")
+
+            repo.index.add(['new_file.py'])
+
+            # for new files, there's no a_blob
+            head_ref = get_safe_head_reference_for_diff(repo)
+            diff_index = repo.index.diff(head_ref)
+            diff = diff_index[0]
+
+            assert len(diff_index) == 1
+
+            result = get_diff_file_path(diff, repo=repo)
+            assert result == test_file
+            assert os.path.isabs(result)
+
+            result = get_diff_file_path(diff, relative=True, repo=repo)
+            assert test_file.endswith(result)
+            assert not os.path.isabs(result)
+
+    def test_diff_with_a_path_fallback(self) -> None:
+        """Test that a_path is used when b_path is None."""
+        with temporary_git_repository() as (temp_dir, repo):
+            test_file = os.path.join(temp_dir, 'deleted_file.py')
+            with open(test_file, 'w') as f:
+                f.write("print('will be deleted')")
+
+            repo.index.add(['deleted_file.py'])
+            repo.index.commit('Initial commit')
+
+            # for deleted files, b_path might be None, so a_path should be used
+            os.remove(test_file)
+            repo.index.remove(['deleted_file.py'])
+
+            head_ref = get_safe_head_reference_for_diff(repo)
+            diff_index = repo.index.diff(head_ref)
+
+            assert len(diff_index) == 1
+            diff = diff_index[0]
+
+            result = get_diff_file_path(diff, repo=repo)
+            assert result == test_file
+            assert os.path.isabs(result)
+
+            result = get_diff_file_path(diff, relative=True, repo=repo)
+            assert test_file.endswith(result)
+            assert not os.path.isabs(result)
+
+    def test_diff_without_repo(self) -> None:
+        """Test behavior when repo is None."""
+        with temporary_git_repository() as (temp_dir, repo):
+            test_file = os.path.join(temp_dir, 'test.py')
+            with open(test_file, 'w') as f:
+                f.write("print('test')")
+
+            repo.index.add(['test.py'])
+            head_ref = get_safe_head_reference_for_diff(repo)
+            diff_index = repo.index.diff(head_ref)
+
+            assert len(diff_index) == 1
+            diff = diff_index[0]
+
+            result = get_diff_file_path(diff, repo=None)
+
+            expected_path = diff.b_path or diff.a_path
+            assert result == expected_path
+            assert not os.path.isabs(result)
+
+    def test_diff_with_bare_repository(self) -> None:
+        """Test behavior when the repository has no working tree directory."""
+        with tempfile.TemporaryDirectory() as temp_dir:
+            bare_repo = Repo.init(temp_dir, bare=True)
+
+            try:
+                # Create a regular repo to push to the bare repo
+                with tempfile.TemporaryDirectory() as work_dir:
+                    work_repo = Repo.init(work_dir, b='main')
+                    try:
+                        test_file = os.path.join(work_dir, 'test.py')
+                        with open(test_file, 'w') as f:
+                            f.write("print('test')")
+
+                        work_repo.index.add(['test.py'])
+                        work_repo.index.commit('Initial commit')
+
+                        work_repo.create_remote('origin', temp_dir)
+                        work_repo.remotes.origin.push('main:main')
+
+                        with open(test_file, 'w') as f:
+                            f.write("print('modified')")
+                        work_repo.index.add(['test.py'])
+
+                        # Get diff
+                        diff_index = work_repo.index.diff('HEAD')
+                        assert len(diff_index) == 1
+                        diff = diff_index[0]
+
+                        # Test with bare repo (no working_tree_dir)
+                        result = get_diff_file_path(diff, repo=bare_repo)
+
+                        # Should return a relative path since bare repo has no working tree
+                        expected_path = diff.b_path or diff.a_path
+                        assert result == expected_path
+                        assert not os.path.isabs(result)
+                    finally:
+                        work_repo.close()
+            finally:
+                bare_repo.close()
+
+    def test_diff_with_no_paths(self) -> None:
+        """Test behavior when the diff has neither a_path nor b_path."""
+        with temporary_git_repository() as (temp_dir, repo):
+
+            class MockDiff:
+                def __init__(self) -> None:
+                    self.a_path = None
+                    self.b_path = None
+                    self.a_blob = None
+                    self.b_blob = None
+
+            result = get_diff_file_path(MockDiff(), repo=repo)
+            assert result is None