Subject: [PATCH] apply: set file mode when --reverse creates a deleted file

From: Mark Mentovai <[email-masked]>
Date: Thu, 22 May 2025 18:02:35 -0400

Commit 01aff0a (apply: correctly reverse patch's pre- and post-image
mode bits; 2023-12-26) revised reverse_patches() to maintain the desired
property that when only one of patch::old_mode and patch::new_mode is
set, the mode will be carried in old_mode. That property is generally
correct, with one notable notable exception: when creating a file, only
new_mode will be set. Since reversing a deletion results in a creation,
new_mode must be set in that case.

Omitting handling for this case meant that reversing a patch that
removed an executable file would not result in the executable permission
being set on the re-created file.

When git apply --reverse is used, reverse_patches() will now additionaly
swap old_mode and new_mode for what's represented in the patch as a file
deletion, as it is transformed into a file creation under reversal.

Tests are added that ensure that git apply sets file modes correctly on
file creation, both in the normal (forward) and reverse direction.
Existing test coverage for file modes focused only on mode changes of
existing files, and only in the forward direction.

Signed-off-by: Mark Mentovai <[email-masked]>
---
 apply.c                   |   2 +-
 t/t4129-apply-samemode.sh | 117 +++++++++++++++++++++++++++++++++++++-
 2 files changed, 117 insertions(+), 2 deletions(-)

diff --git a/apply.c b/apply.c
index f274a3794877..bd4571f89358 100644
--- a/apply.c
+++ b/apply.c
@@ -2219,7 +2219,7 @@ static void reverse_patches(struct patch *p)
 		struct fragment *frag = p->fragments;
 
 		SWAP(p->new_name, p->old_name);
-		if (p->new_mode)
+		if (p->new_mode || p->is_delete)
 			SWAP(p->new_mode, p->old_mode);
 		SWAP(p->is_new, p->is_delete);
 		SWAP(p->lines_added, p->lines_deleted);
diff --git a/t/t4129-apply-samemode.sh b/t/t4129-apply-samemode.sh
index 2149ad5da44c..036613ad8fed 100755
--- a/t/t4129-apply-samemode.sh
+++ b/t/t4129-apply-samemode.sh
@@ -124,9 +124,124 @@ test_expect_success 'git apply respects core.fileMode' '
 
 	git apply patch 2>err &&
 	test_grep ! "has type 100644, expected 100755" err &&
+	git reset --hard &&
 
 	git apply --cached patch 2>err &&
-	test_grep ! "has type 100644, expected 100755" err
+	test_grep ! "has type 100644, expected 100755" err &&
+	git reset --hard
+'
+
+test_expect_success 'git apply restores file modes' '
+	test_config core.fileMode false &&
+	echo "This is data, do not execute!" >data.txt &&
+	git add --chmod=+x data.txt &&
+	git ls-files -s data.txt >ls-files-output &&
+	test_grep "^100755" ls-files-output &&
+	test_tick && git commit -m "Add data" &&
+	git ls-tree -r HEAD data.txt >ls-tree-output &&
+	test_grep "^100755" ls-tree-output &&
+	git checkout -- data.txt &&
+
+	git add --chmod=-x data.txt &&
+	git ls-files -s data.txt >ls-files-output &&
+	test_grep "^100644" ls-files-output &&
+	test_tick && git commit -m "Make data non-executable" &&
+	git ls-tree -r HEAD data.txt >ls-tree-output &&
+	test_grep "^100644" ls-tree-output &&
+	git checkout -- data.txt &&
+
+	git rm data.txt &&
+	git ls-files -s data.txt >ls-files-output &&
+	test_must_be_empty ls-files-output &&
+	test_tick && git commit -m "Remove data" &&
+	git ls-tree -r HEAD data.txt >ls-tree-output &&
+	test_must_be_empty ls-tree-output &&
+
+	git format-patch HEAD~3..HEAD~2 --stdout >patch &&
+	test_grep "^new file mode 100755$" patch &&
+	git apply --index patch &&
+	git ls-files -s data.txt >ls-files-output &&
+	test_grep "^100755" ls-files-output &&
+	test_tick && git commit -m "Re-add data" &&
+	git ls-tree -r HEAD data.txt >ls-tree-output &&
+	test_grep "^100755" ls-tree-output &&
+
+	git format-patch HEAD~3..HEAD~2 --stdout >patch &&
+	test_grep "^old mode 100755$" patch &&
+	test_grep "^new mode 100644$" patch &&
+	git apply --index patch 2>err &&
+	test_grep ! "has type 100644, expected 100755" err &&
+	git ls-files -s data.txt >ls-files-output &&
+	test_grep "^100644" ls-files-output &&
+	test_tick && git commit -m "Redo data mode change" &&
+	git ls-tree -r HEAD data.txt >ls-tree-output &&
+	test_grep "^100644" ls-tree-output &&
+
+	git format-patch HEAD~3..HEAD~2 --stdout >patch &&
+	test_grep "^deleted file mode 100644$" patch &&
+	git apply --index patch 2>err &&
+	test_grep ! "has type 100755, expected 100644" err &&
+	git ls-files -s data.txt >ls-files-output &&
+	test_must_be_empty ls-files-output &&
+	test_tick && git commit -m "Redo data removal" &&
+	git ls-tree -r HEAD tool.sh >ls-tree-output &&
+	test_must_be_empty ls-tree-output
+'
+
+test_expect_success 'git apply --reverse restores file modes' '
+	test_config core.fileMode false &&
+	echo true >tool.sh &&
+	git add --chmod=-x tool.sh &&
+	git ls-files -s tool.sh >ls-files-output &&
+	test_grep "^100644" ls-files-output &&
+	test_tick && git commit -m "Add tool" &&
+	git ls-tree -r HEAD tool.sh >ls-tree-output &&
+	test_grep "^100644" ls-tree-output &&
+
+	git add --chmod=+x tool.sh &&
+	git ls-files -s tool.sh >ls-files-output &&
+	test_grep "^100755" ls-files-output &&
+	test_tick && git commit -m "Make tool executable" &&
+	git ls-tree -r HEAD tool.sh >ls-tree-output &&
+	test_grep "^100755" ls-tree-output &&
+	git checkout -- tool.sh &&
+
+	git rm tool.sh &&
+	git ls-files -s tool.sh >ls-files-output &&
+	test_must_be_empty ls-files-output &&
+	test_tick && git commit -m "Remove tool" &&
+	git ls-tree -r HEAD tool.sh >ls-tree-output &&
+	test_must_be_empty ls-tree-output &&
+
+	git format-patch -1 --stdout >patch &&
+	test_grep "^deleted file mode 100755$" patch &&
+	git apply --index --reverse patch &&
+	git ls-files -s tool.sh >ls-files-output &&
+	test_grep "^100755" ls-files-output &&
+	test_tick && git commit -m "Undo tool removal" &&
+	git ls-tree -r HEAD tool.sh >ls-tree-output &&
+	test_grep "^100755" ls-tree-output &&
+
+	git format-patch HEAD~3..HEAD~2 --stdout >patch &&
+	test_grep "^old mode 100644$" patch &&
+	test_grep "^new mode 100755$" patch &&
+	git apply --index --reverse patch 2>err &&
+	test_grep ! "has type 100644, expected 100755" err &&
+	git ls-files -s tool.sh >ls-files-output &&
+	test_grep "^100644" ls-files-output &&
+	test_tick && git commit -m "Undo tool mode change" &&
+	git ls-tree -r HEAD tool.sh >ls-tree-output &&
+	test_grep "^100644" ls-tree-output &&
+
+	git format-patch HEAD~5..HEAD~4 --stdout >patch &&
+	test_grep "^new file mode 100644$" patch &&
+	git apply --index --reverse patch 2>err &&
+	test_grep ! "has type 100755, expected 100644" err &&
+	git ls-files -s tool.sh >ls-files-output &&
+	test_must_be_empty ls-files-output &&
+	test_tick && git commit -m "Undo tool addition" &&
+	git ls-tree -r HEAD tool.sh >ls-tree-output &&
+	test_must_be_empty ls-tree-output
 '
 
 test_expect_success POSIXPERM 'patch mode for new file is canonicalized' '
-- 
2.49.0