git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v2 2/2] apply: set file mode when --reverse creates a deleted file

From
Mark Mentovai <mark@chromium.org>
Date
May 23, 2025, 17:21 UTC
Message-ID
<20250523172154.93810-3-mark@chromium.org>
In-Reply-To
<20250523172154.93810-1-mark@chromium.org>

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 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 means that reversing a patch that removes an executable file will not result in the executable permission being set on the re-created file. Existing test coverage for file modes focuses only on mode changes of existing files.

Swap old_mode and new_mode in reverse_patches() for what's represented in the patch as a file deletion, as it is transformed into a file creation under reversal. This causes git apply --reverse to set the executable permission properly when re-creating a deleted executable file.

Add tests ensuring that git apply sets file modes correctly on file creation, both in the forward and reverse directions.

Signed-off-by: Mark Mentovai <mark@chromium.org>
---
 apply.c                   |   2 +-
 t/t4129-apply-samemode.sh | 152 +++++++++++++++++++++++++++++++++++++-
 2 files changed, 151 insertions(+), 3 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 082e56db651e..4b64b7213f76 100755
--- a/t/t4129-apply-samemode.sh
+++ b/t/t4129-apply-samemode.sh
@@ -102,14 +102,18 @@ test_expect_success POSIXPERM 'do not use core.sharedRepository for working tree
 	)
 '
 
+test_file_mode_common() {
+	test -n "$1" && test_grep "^10$1 " "$2" || test_must_be_empty "$2"
+}
+
 test_file_mode_staged () {
 	git ls-files --stage -- "$2" >ls-files-output &&
-	test_grep "^10$1 " ls-files-output
+	test_file_mode_common "$1" ls-files-output
 }
 
 test_file_mode_HEAD () {
 	git ls-tree HEAD -- "$2" >ls-tree-output &&
-	test_grep "^10$1 " ls-tree-output
+	test_file_mode_common "$1" ls-tree-output
 }
 
 test_expect_success 'git apply respects core.fileMode' '
@@ -180,6 +184,150 @@ test_expect_success 'git apply warns about incorrect file modes' '
 	test_file_mode_HEAD 0755 mode_test
 '
 
+test_expect_success 'setup: git apply [--reverse] restores file modes (change_x_to_notx)' '
+	test_config core.fileMode false &&
+
+	touch change_x_to_notx &&
+	git add --chmod=+x change_x_to_notx &&
+	test_file_mode_staged 0755 change_x_to_notx &&
+	test_tick && git commit -m "add change_x_to_notx as executable" &&
+	test_file_mode_HEAD 0755 change_x_to_notx &&
+
+	git add --chmod=-x change_x_to_notx &&
+	test_file_mode_staged 0644 change_x_to_notx &&
+	test_tick && git commit -m "make change_x_to_notx not executable" &&
+	test_file_mode_HEAD 0644 change_x_to_notx &&
+
+	git rm change_x_to_notx &&
+	test_file_mode_staged "" change_x_to_notx &&
+	test_tick && git commit -m "remove change_x_to_notx" &&
+	test_file_mode_HEAD "" change_x_to_notx &&
+
+	git format-patch -o patches -3 &&
+	mv patches/0001-* change_x_to_notx-0001-create-0755.patch &&
+	mv patches/0002-* change_x_to_notx-0002-chmod-0644.patch &&
+	mv patches/0003-* change_x_to_notx-0003-delete.patch &&
+
+	test_grep "^new file mode 100755$" change_x_to_notx-0001-create-0755.patch &&
+	test_grep "^old mode 100755$" change_x_to_notx-0002-chmod-0644.patch &&
+	test_grep "^new mode 100644$" change_x_to_notx-0002-chmod-0644.patch &&
+	test_grep "^deleted file mode 100644$" change_x_to_notx-0003-delete.patch
+'
+
+test_expect_success 'git apply restores file modes (change_x_to_notx)' '
+	test_config core.fileMode false &&
+
+	git apply --index change_x_to_notx-0001-create-0755.patch &&
+	test_file_mode_staged 0755 change_x_to_notx &&
+	test_tick && git commit -m "redo: add change_x_to_notx as executable" &&
+	test_file_mode_HEAD 0755 change_x_to_notx &&
+
+	git apply --index change_x_to_notx-0002-chmod-0644.patch 2>err &&
+	test_grep ! "has type 100.*, expected 100.*" err &&
+	test_file_mode_staged 0644 change_x_to_notx &&
+	test_tick && git commit -m "redo: make change_x_to_notx not executable" &&
+	test_file_mode_HEAD 0644 change_x_to_notx &&
+
+	git apply --index change_x_to_notx-0003-delete.patch 2>err &&
+	test_grep ! "has type 100.*, expected 100.*" err &&
+	test_file_mode_staged "" change_x_to_notx &&
+	test_tick && git commit -m "redo: remove change_notx_to_x" &&
+	test_file_mode_HEAD "" change_x_to_notx
+'
+
+test_expect_success 'git apply --reverse restores file modes (change_x_to_notx)' '
+	test_config core.fileMode false &&
+
+	git apply --index --reverse change_x_to_notx-0003-delete.patch &&
+	test_file_mode_staged 0644 change_x_to_notx &&
+	test_tick && git commit -m "undo: remove change_x_to_notx" &&
+	test_file_mode_HEAD 0644 change_x_to_notx &&
+
+	git apply --index --reverse change_x_to_notx-0002-chmod-0644.patch 2>err &&
+	test_grep ! "has type 100.*, expected 100.*" err &&
+	test_file_mode_staged 0755 change_x_to_notx &&
+	test_tick && git commit -m "undo: make change_x_to_notx not executable" &&
+	test_file_mode_HEAD 0755 change_x_to_notx &&
+
+	git apply --index --reverse change_x_to_notx-0001-create-0755.patch 2>err &&
+	test_grep ! "has type 100.*, expected 100.*" err &&
+	test_file_mode_staged "" change_x_to_notx &&
+	test_tick && git commit -m "undo: add change_x_to_notx as executable" &&
+	test_file_mode_HEAD "" change_x_to_notx
+'
+
+test_expect_success 'setup: git apply [--reverse] restores file modes (change_notx_to_x)' '
+	test_config core.fileMode false &&
+
+	touch change_notx_to_x &&
+	git add --chmod=-x change_notx_to_x &&
+	test_file_mode_staged 0644 change_notx_to_x &&
+	test_tick && git commit -m "add change_notx_to_x as not executable" &&
+	test_file_mode_HEAD 0644 change_notx_to_x &&
+
+	git add --chmod=+x change_notx_to_x &&
+	test_file_mode_staged 0755 change_notx_to_x &&
+	test_tick && git commit -m "make change_notx_to_x executable" &&
+	test_file_mode_HEAD 0755 change_notx_to_x &&
+
+	git rm change_notx_to_x &&
+	test_file_mode_staged "" change_notx_to_x &&
+	test_tick && git commit -m "remove change_notx_to_x" &&
+	test_file_mode_HEAD "" change_notx_to_x &&
+
+	git format-patch -o patches -3 &&
+	mv patches/0001-* change_notx_to_x-0001-create-0644.patch &&
+	mv patches/0002-* change_notx_to_x-0002-chmod-0755.patch &&
+	mv patches/0003-* change_notx_to_x-0003-delete.patch &&
+
+	test_grep "^new file mode 100644$" change_notx_to_x-0001-create-0644.patch &&
+	test_grep "^old mode 100644$" change_notx_to_x-0002-chmod-0755.patch &&
+	test_grep "^new mode 100755$" change_notx_to_x-0002-chmod-0755.patch &&
+	test_grep "^deleted file mode 100755$" change_notx_to_x-0003-delete.patch
+'
+
+test_expect_success 'git apply restores file modes (change_notx_to_x)' '
+	test_config core.fileMode false &&
+
+	git apply --index change_notx_to_x-0001-create-0644.patch &&
+	test_file_mode_staged 0644 change_notx_to_x &&
+	test_tick && git commit -m "redo: add change_notx_to_x as not executable" &&
+	test_file_mode_HEAD 0644 change_notx_to_x &&
+
+	git apply --index change_notx_to_x-0002-chmod-0755.patch 2>err &&
+	test_grep ! "has type 100.*, expected 100.*" err &&
+	test_file_mode_staged 0755 change_notx_to_x &&
+	test_tick && git commit -m "redo: make change_notx_to_x executable" &&
+	test_file_mode_HEAD 0755 change_notx_to_x &&
+
+	git apply --index change_notx_to_x-0003-delete.patch &&
+	test_grep ! "has type 100.*, expected 100.*" err &&
+	test_file_mode_staged "" change_notx_to_x &&
+	test_tick && git commit -m "undo: remove change_notx_to_x" &&
+	test_file_mode_HEAD "" change_notx_to_x
+'
+
+test_expect_success 'git apply --reverse restores file modes (change_notx_to_x)' '
+	test_config core.fileMode false &&
+
+	git apply --index --reverse change_notx_to_x-0003-delete.patch &&
+	test_file_mode_staged 0755 change_notx_to_x &&
+	test_tick && git commit -m "undo: remove change_notx_to_x" &&
+	test_file_mode_HEAD 0755 change_notx_to_x &&
+
+	git apply --index --reverse change_notx_to_x-0002-chmod-0755.patch 2>err &&
+	test_grep ! "has type 100.*, expected 100.*" err &&
+	test_file_mode_staged 0644 change_notx_to_x &&
+	test_tick && git commit -m "undo: make change_notx_to_x executable" &&
+	test_file_mode_HEAD 0644 change_notx_to_x &&
+
+	git apply --index --reverse change_notx_to_x-0001-create-0644.patch 2>err &&
+	test_grep ! "has type 100.*, expected 100.*" err &&
+	test_file_mode_staged "" change_notx_to_x &&
+	test_tick && git commit -m "undo: add change_notx_to_x as not executable" &&
+	test_file_mode_HEAD "" change_notx_to_x
+'
+
 test_expect_success POSIXPERM 'patch mode for new file is canonicalized' '
 	cat >patch <<-\EOF &&
 	diff --git a/non-canon b/non-canon

-- 
2.49.0
Previous: Mark MentovaiNext: Junio C Hamano
Message 8 of 17 in “apply: set file mode when --reverse creates a deleted file”
  1. apply: set file mode when --reverse creates a deleted fileMark Mentovai, May 22, 2025
  2. Kristoffer HaugsbakkMay 22, 2025
  3. Junio C HamanoMay 22, 2025
  4. Junio C HamanoMay 22, 2025
  5. Mark MentovaiMay 22, 2025
  6. Junio C HamanoMay 23, 2025
  7. 0/2 apply: set file mode when --reverse creates a deleted fileMark Mentovai, May 23, 2025
  8. 2/2 apply: set file mode when --reverse creates a deleted fileMark Mentovai, May 23, 2025
  9. Junio C HamanoMay 23, 2025
  10. Mark MentovaiMay 24, 2025
  11. 1/2 t4129: test that git apply warns for unexpected mode changesMark Mentovai, May 23, 2025
  12. Junio C HamanoMay 23, 2025
  13. Mark MentovaiMay 24, 2025
  14. 0/2 apply: set file mode when --reverse creates a deleted fileMark Mentovai, May 24, 2025
  15. 1/2 t4129: test that git apply warns for unexpected mode changesMark Mentovai, May 24, 2025
  16. 2/2 apply: set file mode when --reverse creates a deleted fileMark Mentovai, May 24, 2025
  17. Junio C HamanoMay 27, 2025

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.