From d644a344104f68b810295a4b32cf46eb5deb9310 Mon Sep 17 00:00:00 2001 From: Ilya Siamionau Date: Thu, 11 Sep 2025 17:00:10 +0200 Subject: [PATCH 1/2] CM-51935 - Fix pre-receive hook for bare repositories --- .../files_collector/commit_range_documents.py | 45 +++-- .../test_commit_range_documents.py | 181 ++++++++++++++++++ 2 files changed, 208 insertions(+), 18 deletions(-) diff --git a/cycode/cli/files_collector/commit_range_documents.py b/cycode/cli/files_collector/commit_range_documents.py index 572d0cc2..6bb4f365 100644 --- a/cycode/cli/files_collector/commit_range_documents.py +++ b/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 related 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: diff --git a/tests/cli/files_collector/test_commit_range_documents.py b/tests/cli/files_collector/test_commit_range_documents.py index c51db5ad..c092d4c4 100644 --- a/tests/cli/files_collector/test_commit_range_documents.py +++ b/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 From 2a93d8a94fc834118169504d0f23d91c31a56345 Mon Sep 17 00:00:00 2001 From: Ilya Siamionau Date: Thu, 11 Sep 2025 17:04:40 +0200 Subject: [PATCH 2/2] fix typo Co-authored-by: elsapet --- cycode/cli/files_collector/commit_range_documents.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cycode/cli/files_collector/commit_range_documents.py b/cycode/cli/files_collector/commit_range_documents.py index 6bb4f365..65cf8506 100644 --- a/cycode/cli/files_collector/commit_range_documents.py +++ b/cycode/cli/files_collector/commit_range_documents.py @@ -223,7 +223,7 @@ def get_diff_file_path(diff: 'Diff', relative: bool = False, repo: Optional['Rep Note: It tries to get the absolute path, falling back to relative paths. `relative` flag forces relative paths. - One case of related paths is when the repository is bare and does not have a working tree directory. + 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