Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions pr_split/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
materialize_group_files,
merge_chain_assignments,
parse_diff,
target_file_modes,
)
from .exceptions import (
ErrorMsg,
Expand Down Expand Up @@ -357,12 +358,17 @@ def _create_single_branch_and_commit(
add_worktree(worktree_path, branch_name, start_point or merge_base_ref)
try:
materialized = materialize_group_files(parsed_diff, group, merge_base_ref)
modes = target_file_modes(parsed_diff, group)
for file_path, content in materialized.items():
p = Path(worktree_path) / file_path
if content is not None:
p.parent.mkdir(parents=True, exist_ok=True)
# newline="" keeps CRLF from the reconstructed content intact.
p.write_text(content, encoding="utf-8", errors="surrogateescape", newline="")
if file_path in modes:
# Git only tracks the executable bit; apply the diff's
# target mode so chmod changes reach the sub-PR.
p.chmod(modes[file_path] & 0o777)
elif p.exists():
p.unlink()

Expand Down
3 changes: 2 additions & 1 deletion pr_split/diff_ops/__init__.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
from .parser import ParsedDiff, extract_diff, parse_diff
from .reconstructor import materialize_group_files, merge_chain_assignments
from .reconstructor import materialize_group_files, merge_chain_assignments, target_file_modes

__all__ = [
"ParsedDiff",
"extract_diff",
"materialize_group_files",
"merge_chain_assignments",
"parse_diff",
"target_file_modes",
]
31 changes: 31 additions & 0 deletions pr_split/diff_ops/reconstructor.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
from __future__ import annotations

import re
import subprocess

from loguru import logger
Expand Down Expand Up @@ -164,3 +165,33 @@ def materialize_group_files(
base_content = _get_base_file_content(file_path, ref)
result[file_path] = apply_hunks(base_content, patch_file, indices)
return result


_TARGET_MODE_RE = re.compile(r"^(?:new file mode|new mode) (\d{6})$", re.MULTILINE)


def target_file_modes(parsed_diff: ParsedDiff, group: Group) -> dict[str, int]:
"""Map each file the group materializes to the mode the diff gives it.

unidiff parses ``old mode``/``new mode``/``new file mode`` headers into
``patch_info`` only; nothing else applies them, so without this a
``chmod +x`` that comes with a content change silently loses the bit.
"""
wanted = {assignment.file_path for assignment in group.assignments}
modes: dict[str, int] = {}
for patch_file in parsed_diff.patch_set:
# target_file, not is_removed_file: the latter is also true for a file
# truncated to empty, which still needs its mode applied.
if patch_file.path not in wanted or patch_file.target_file == "/dev/null":
continue
header = "".join(str(line) for line in patch_file.patch_info or [])
match = _TARGET_MODE_RE.search(header)
if not match:
continue
mode = int(match.group(1), 8)
# Only regular files (100644 / 100755): a symlink's 120000 would mask
# to 000 and make the written file unreadable. Symlinks are written as
# plain files by the reconstructor, unchanged from before.
if mode & 0o170000 == 0o100000:
modes[patch_file.path] = mode
return modes
31 changes: 31 additions & 0 deletions tests/test_cli_helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -892,3 +892,34 @@ def test_oversized_ids_are_recorded(self) -> None:

groups = [self._sized("pr-1", 876), self._sized("pr-2", 400)]
assert _oversized_group_ids(groups, 400) == ["pr-1"]


class TestWorktreeAppliesFileModes:
@patch("pr_split.cli.commit_files_in_dir", return_value="sha1")
@patch("pr_split.cli.target_file_modes", return_value={"run.sh": 0o100755})
@patch("pr_split.cli.materialize_group_files", return_value={"run.sh": "x\n", "a.py": "y\n"})
@patch("pr_split.cli.remove_worktree")
@patch("pr_split.cli.add_worktree")
def test_executable_bit_applied_only_where_diff_says(
self,
mock_add: MagicMock,
mock_remove: MagicMock,
mock_mat: MagicMock,
mock_modes: MagicMock,
mock_commit: MagicMock,
) -> None:
import os
import stat
from pathlib import Path

modes: dict[str, int] = {}

def capture(cwd: str, file_paths: list[str], message: str, **kwargs: object) -> str:
for file_path in file_paths:
modes[file_path] = stat.S_IMODE(os.stat(Path(cwd) / file_path).st_mode)
return "sha1"

mock_commit.side_effect = capture
_create_branches_and_commits([_group("pr-1", "t")], MagicMock(), "main", "sha", "ns")
assert modes["run.sh"] & stat.S_IXUSR
assert not modes["a.py"] & stat.S_IXUSR
66 changes: 66 additions & 0 deletions tests/test_reconstructor.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
materialize_group_files,
merge_chain_assignments,
split_git_lines,
target_file_modes,
)
from pr_split.exceptions import GitOperationError
from pr_split.schemas import Group, GroupAssignment
Expand Down Expand Up @@ -659,3 +660,68 @@ def test_real_deletion_still_removes(self) -> None:
],
)
assert materialize_group_files(parsed, group, "main") == {"a.txt": None}


class TestTargetFileModes:
def _group(self, *paths: str) -> Group:
return Group(
id="pr-1",
title="t",
description="d",
assignments=[
GroupAssignment(
file_path=p, assignment_type=AssignmentType.WHOLE_FILE, hunk_indices=[0]
)
for p in paths
],
)

def test_mode_change_with_content_is_reported(self) -> None:
parsed = parse_diff(
"diff --git a/run.sh b/run.sh\nold mode 100644\nnew mode 100755\n"
"index 422c2b7..55dce13\n--- a/run.sh\n+++ b/run.sh\n@@ -1,2 +1,2 @@\n a\n-b\n+B\n"
)
assert target_file_modes(parsed, self._group("run.sh")) == {"run.sh": 0o100755}

def test_new_executable_file_is_reported(self) -> None:
parsed = parse_diff(
"diff --git a/n.sh b/n.sh\nnew file mode 100755\n--- /dev/null\n+++ b/n.sh\n"
"@@ -0,0 +1 @@\n+x\n"
)
assert target_file_modes(parsed, self._group("n.sh")) == {"n.sh": 0o100755}

def test_plain_change_and_unassigned_files_are_ignored(self) -> None:
parsed = parse_diff(
"diff --git a/a.py b/a.py\n--- a/a.py\n+++ b/a.py\n@@ -1 +1 @@\n-x\n+y\n"
"diff --git a/b.sh b/b.sh\nold mode 100644\nnew mode 100755\n"
"--- a/b.sh\n+++ b/b.sh\n@@ -1 +1 @@\n-x\n+y\n"
)
assert target_file_modes(parsed, self._group("a.py")) == {}

def test_removed_file_is_ignored(self) -> None:
parsed = parse_diff(
"diff --git a/d.sh b/d.sh\ndeleted file mode 100755\n--- a/d.sh\n+++ /dev/null\n"
"@@ -1 +0,0 @@\n-x\n"
)
assert target_file_modes(parsed, self._group("d.sh")) == {}

def test_symlink_mode_is_not_reported(self) -> None:
parsed = parse_diff(
"diff --git a/link b/link\nnew file mode 120000\n--- /dev/null\n+++ b/link\n"
"@@ -0,0 +1 @@\n+other.py\n\\ No newline at end of file\n"
)
assert target_file_modes(parsed, self._group("link")) == {}

def test_mode_change_back_to_non_executable_is_reported(self) -> None:
parsed = parse_diff(
"diff --git a/run.sh b/run.sh\nold mode 100755\nnew mode 100644\n"
"--- a/run.sh\n+++ b/run.sh\n@@ -1 +1 @@\n-x\n+y\n"
)
assert target_file_modes(parsed, self._group("run.sh")) == {"run.sh": 0o100644}

def test_file_truncated_to_empty_keeps_its_new_mode(self) -> None:
parsed = parse_diff(
"diff --git a/a.sh b/a.sh\nold mode 100644\nnew mode 100755\n"
"--- a/a.sh\n+++ b/a.sh\n@@ -1,2 +0,0 @@\n-x\n-y\n"
)
assert target_file_modes(parsed, self._group("a.sh")) == {"a.sh": 0o100755}
Loading