From 2cc8c17d67265f1912b79f8e57fc2718597ca686 Mon Sep 17 00:00:00 2001 From: Mark Mentovai Date: Fri, 23 May 2025 23:40:45 -0400 Subject: [PATCH 1/2] t4129: test that git apply warns for unexpected mode changes There is no test covering what commit 01aff0a (apply: correctly reverse patch's pre- and post-image mode bits, 2023-12-26) addressed. Prior to that commit, git apply was erroneously unaware of a file's expected mode while reverse-patching a file whose mode was not changing. Add the missing test coverage to assure that git apply is aware of the expected mode of a file being patched when the patch does not indicate that the file's mode is changing. This is achieved by arranging a file mode so that it doesn't agree with patch being applied, and checking git apply's output for the warning it's supposed to raise in this situation. Test in both reverse and normal (forward) directions. Signed-off-by: Mark Mentovai Signed-off-by: Junio C Hamano --- t/t4129-apply-samemode.sh | 70 ++++++++++++++++++++++++++++++++++++--- 1 file changed, 65 insertions(+), 5 deletions(-) diff --git a/t/t4129-apply-samemode.sh b/t/t4129-apply-samemode.sh index 2149ad5da4..bf4e7609dc 100755 --- a/t/t4129-apply-samemode.sh +++ b/t/t4129-apply-samemode.sh @@ -102,15 +102,23 @@ test_expect_success POSIXPERM 'do not use core.sharedRepository for working tree ) ' +test_file_mode_staged () { + git ls-files --stage -- "$2" >ls-files-output && + test_grep "^$1 " ls-files-output +} + +test_file_mode_HEAD () { + git ls-tree HEAD -- "$2" >ls-tree-output && + test_grep "^$1 " ls-tree-output +} + test_expect_success 'git apply respects core.fileMode' ' test_config core.fileMode false && echo true >script.sh && git add --chmod=+x script.sh && - git ls-files -s script.sh >ls-files-output && - test_grep "^100755" ls-files-output && + test_file_mode_staged 100755 script.sh && test_tick && git commit -m "Add script" && - git ls-tree -r HEAD script.sh >ls-tree-output && - test_grep "^100755" ls-tree-output && + test_file_mode_HEAD 100755 script.sh && echo true >>script.sh && test_tick && git commit -m "Modify script" script.sh && @@ -126,7 +134,59 @@ test_expect_success 'git apply respects core.fileMode' ' test_grep ! "has type 100644, expected 100755" err && 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 'setup: git apply [--reverse] warns about incorrect file modes' ' + test_config core.fileMode false && + + >mode_test && + git add --chmod=-x mode_test && + test_file_mode_staged 100644 mode_test && + test_tick && git commit -m "add mode_test" && + test_file_mode_HEAD 100644 mode_test && + git tag mode_test_forward_initial && + + echo content >>mode_test && + test_tick && git commit -m "append to mode_test" mode_test && + test_file_mode_HEAD 100644 mode_test && + git tag mode_test_reverse_initial && + + git format-patch -1 --stdout >patch && + test_grep "^index .* 100644$" patch +' + +test_expect_success 'git apply warns about incorrect file modes' ' + test_config core.fileMode false && + git reset --hard mode_test_forward_initial && + + git add --chmod=+x mode_test && + test_file_mode_staged 100755 mode_test && + test_tick && git commit -m "make mode_test executable" && + test_file_mode_HEAD 100755 mode_test && + + git apply --index patch 2>err && + test_grep "has type 100755, expected 100644" err && + test_file_mode_staged 100755 mode_test && + test_tick && git commit -m "redo: append to mode_test" && + test_file_mode_HEAD 100755 mode_test +' + +test_expect_success 'git apply --reverse warns about incorrect file modes' ' + test_config core.fileMode false && + git reset --hard mode_test_reverse_initial && + + git add --chmod=+x mode_test && + test_file_mode_staged 100755 mode_test && + test_tick && git commit -m "make mode_test executable" && + test_file_mode_HEAD 100755 mode_test && + + git apply --index --reverse patch 2>err && + test_grep "has type 100755, expected 100644" err && + test_file_mode_staged 100755 mode_test && + test_tick && git commit -m "undo: append to mode_test" && + test_file_mode_HEAD 100755 mode_test ' test_expect_success POSIXPERM 'patch mode for new file is canonicalized' ' From 1d9a66493be9fef38e531da587dd47f6f17ea483 Mon Sep 17 00:00:00 2001 From: Mark Mentovai Date: Fri, 23 May 2025 23:40:46 -0400 Subject: [PATCH 2/2] apply: set file mode when --reverse creates a deleted file 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 Signed-off-by: Junio C Hamano --- apply.c | 2 +- t/t4129-apply-samemode.sh | 165 +++++++++++++++++++++++++++++++++++++- 2 files changed, 164 insertions(+), 3 deletions(-) diff --git a/apply.c b/apply.c index 381d2e3652..8bbe6ed224 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 bf4e7609dc..1d6317bd71 100755 --- a/t/t4129-apply-samemode.sh +++ b/t/t4129-apply-samemode.sh @@ -102,14 +102,23 @@ test_expect_success POSIXPERM 'do not use core.sharedRepository for working tree ) ' +test_file_mode_common () { + if test "$1" = "000000" + then + test_must_be_empty "$2" + else + test_grep "^$1 " "$2" + fi +} + test_file_mode_staged () { git ls-files --stage -- "$2" >ls-files-output && - test_grep "^$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 "^$1 " ls-tree-output + test_file_mode_common "$1" ls-tree-output } test_expect_success 'git apply respects core.fileMode' ' @@ -189,6 +198,158 @@ test_expect_success 'git apply --reverse warns about incorrect file modes' ' test_file_mode_HEAD 100755 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 100755 change_x_to_notx && + test_tick && git commit -m "add change_x_to_notx as executable" && + test_file_mode_HEAD 100755 change_x_to_notx && + + git add --chmod=-x change_x_to_notx && + test_file_mode_staged 100644 change_x_to_notx && + test_tick && git commit -m "make change_x_to_notx not executable" && + test_file_mode_HEAD 100644 change_x_to_notx && + + git rm change_x_to_notx && + test_file_mode_staged 000000 change_x_to_notx && + test_tick && git commit -m "remove change_x_to_notx" && + test_file_mode_HEAD 000000 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 && + + git tag change_x_to_notx_initial +' + +test_expect_success 'git apply restores file modes (change_x_to_notx)' ' + test_config core.fileMode false && + git reset --hard change_x_to_notx_initial && + + git apply --index change_x_to_notx-0001-create-0755.patch && + test_file_mode_staged 100755 change_x_to_notx && + test_tick && git commit -m "redo: add change_x_to_notx as executable" && + test_file_mode_HEAD 100755 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 100644 change_x_to_notx && + test_tick && git commit -m "redo: make change_x_to_notx not executable" && + test_file_mode_HEAD 100644 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 000000 change_x_to_notx && + test_tick && git commit -m "redo: remove change_notx_to_x" && + test_file_mode_HEAD 000000 change_x_to_notx +' + +test_expect_success 'git apply --reverse restores file modes (change_x_to_notx)' ' + test_config core.fileMode false && + git reset --hard change_x_to_notx_initial && + + git apply --index --reverse change_x_to_notx-0003-delete.patch && + test_file_mode_staged 100644 change_x_to_notx && + test_tick && git commit -m "undo: remove change_x_to_notx" && + test_file_mode_HEAD 100644 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 100755 change_x_to_notx && + test_tick && git commit -m "undo: make change_x_to_notx not executable" && + test_file_mode_HEAD 100755 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 000000 change_x_to_notx && + test_tick && git commit -m "undo: add change_x_to_notx as executable" && + test_file_mode_HEAD 000000 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 100644 change_notx_to_x && + test_tick && git commit -m "add change_notx_to_x as not executable" && + test_file_mode_HEAD 100644 change_notx_to_x && + + git add --chmod=+x change_notx_to_x && + test_file_mode_staged 100755 change_notx_to_x && + test_tick && git commit -m "make change_notx_to_x executable" && + test_file_mode_HEAD 100755 change_notx_to_x && + + git rm change_notx_to_x && + test_file_mode_staged 000000 change_notx_to_x && + test_tick && git commit -m "remove change_notx_to_x" && + test_file_mode_HEAD 000000 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 && + + git tag change_notx_to_x_initial +' + +test_expect_success 'git apply restores file modes (change_notx_to_x)' ' + test_config core.fileMode false && + git reset --hard change_notx_to_x_initial && + + git apply --index change_notx_to_x-0001-create-0644.patch && + test_file_mode_staged 100644 change_notx_to_x && + test_tick && git commit -m "redo: add change_notx_to_x as not executable" && + test_file_mode_HEAD 100644 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 100755 change_notx_to_x && + test_tick && git commit -m "redo: make change_notx_to_x executable" && + test_file_mode_HEAD 100755 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 000000 change_notx_to_x && + test_tick && git commit -m "undo: remove change_notx_to_x" && + test_file_mode_HEAD 000000 change_notx_to_x +' + +test_expect_success 'git apply --reverse restores file modes (change_notx_to_x)' ' + test_config core.fileMode false && + git reset --hard change_notx_to_x_initial && + + git apply --index --reverse change_notx_to_x-0003-delete.patch && + test_file_mode_staged 100755 change_notx_to_x && + test_tick && git commit -m "undo: remove change_notx_to_x" && + test_file_mode_HEAD 100755 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 100644 change_notx_to_x && + test_tick && git commit -m "undo: make change_notx_to_x executable" && + test_file_mode_HEAD 100644 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 000000 change_notx_to_x && + test_tick && git commit -m "undo: add change_notx_to_x as not executable" && + test_file_mode_HEAD 000000 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