[PATCH v3 0/2] apply: set file mode when --reverse creates a deleted file
- From
Mark Mentovai <mark@chromium.org>
- Date
- May 24, 2025, 03:40 UTC
- Message-ID
- <20250524034046.2619-1-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.
Changes from v2: * test_file_mode_common correctly fails when requested to verify the presence of a file (by nonzero mode argument) that is actually absent. * Test setup establishes tags at appropriate checkpoints with git tag, and tests restore those checkpoints with git reset --hard, making tests less sensitive to execution order and to the success of previous tests. * test_file_mode_common accepts full 6-digit file modes instead of 4-digit ones, and uses 000000 for absent files (as git diff --raw does) instead of an empty string. * File presence is assured with >f as opposed to touch f. Minor style * fix at the point of test_file_mode_common's declaration.
Changes from v1: * Reword the commit message as requested. * Refactor the new test in accordance with review feedback, splitting it up and increasing coverage. * Introduce another new test for the specific problem that commit 01aff0a (apply: correctly reverse patch's pre- and post-image mode bits, 2023-12-26) fixed.
Mark Mentovai (2): t4129: test that git apply warns for unexpected mode changes apply: set file mode when --reverse creates a deleted file
apply.c | 2 +- t/t4129-apply-samemode.sh | 231 +++++++++++++++++++++++++++++++++++++- 2 files changed, 227 insertions(+), 6 deletions(-)
Range-diff against v2:
1: a0af8d15f664 ! 1: 5ee7d4efe53e t4129: test that git apply warns for unexpected mode changes
@@ t/t4129-apply-samemode.sh: test_expect_success POSIXPERM 'do not use core.shared
+test_file_mode_staged () {
+ git ls-files --stage -- "$2" >ls-files-output &&
-+ test_grep "^10$1 " ls-files-output
++ test_grep "^$1 " ls-files-output
+}
+
+test_file_mode_HEAD () {
+ git ls-tree HEAD -- "$2" >ls-tree-output &&
-+ test_grep "^10$1 " ls-tree-output
++ test_grep "^$1 " ls-tree-output
+}
+
test_expect_success 'git apply respects core.fileMode' '
@@ t/t4129-apply-samemode.sh: test_expect_success POSIXPERM 'do not use core.shared
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 0755 script.sh &&
++ 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 0755 script.sh &&
++ test_file_mode_HEAD 100755 script.sh &&
echo true >>script.sh &&
test_tick && git commit -m "Modify script" script.sh &&
@@ t/t4129-apply-samemode.sh: test_expect_success 'git apply respects core.fileMode
+test_expect_success 'setup: git apply [--reverse] warns about incorrect file modes' '
+ test_config core.fileMode false &&
+
-+ touch mode_test &&
++ >mode_test &&
+ git add --chmod=-x mode_test &&
-+ test_file_mode_staged 0644 mode_test &&
++ test_file_mode_staged 100644 mode_test &&
+ test_tick && git commit -m "add mode_test" &&
-+ test_file_mode_HEAD 0644 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 0644 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 &&
-+
-+ git add --chmod=+x mode_test &&
-+ test_file_mode_staged 0755 mode_test &&
-+ test_tick && git commit -m "make mode_test executable" &&
-+ test_file_mode_HEAD 0755 mode_test
++ test_grep "^index .* 100644$" patch
+'
+
-+test_expect_success 'git apply --reverse warns about incorrect file modes' '
++test_expect_success 'git apply warns about incorrect file modes' '
+ test_config core.fileMode false &&
++ git reset --hard mode_test_forward_initial &&
+
-+ git apply --index --reverse patch 2>err &&
++ 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 0755 mode_test &&
-+ test_tick && git commit -m "undo append" &&
-+ test_file_mode_HEAD 0755 mode_test
++ 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 warns about incorrect file modes' '
++test_expect_success 'git apply --reverse warns about incorrect file modes' '
+ test_config core.fileMode false &&
++ git reset --hard mode_test_reverse_initial &&
+
-+ git apply --index patch 2>err &&
++ 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 0755 mode_test &&
-+ test_tick && git commit -m "redo append" &&
-+ test_file_mode_HEAD 0755 mode_test
++ 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' '
2: 00760c9ec492 ! 2: cc6f4b4af3b7 apply: set file mode when --reverse creates a deleted file
@@ t/t4129-apply-samemode.sh: test_expect_success POSIXPERM 'do not use core.shared
)
'
-+test_file_mode_common() {
-+ test -n "$1" && test_grep "^10$1 " "$2" || test_must_be_empty "$2"
++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 "^10$1 " 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 "^10$1 " 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' '
-@@ t/t4129-apply-samemode.sh: test_expect_success 'git apply warns about incorrect file modes' '
- test_file_mode_HEAD 0755 mode_test
+@@ t/t4129-apply-samemode.sh: 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)' '
@@ t/t4129-apply-samemode.sh: test_expect_success 'git apply warns about incorrect
+
+ touch change_x_to_notx &&
+ git add --chmod=+x change_x_to_notx &&
-+ test_file_mode_staged 0755 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 0755 change_x_to_notx &&
++ test_file_mode_HEAD 100755 change_x_to_notx &&
+
+ git add --chmod=-x change_x_to_notx &&
-+ test_file_mode_staged 0644 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 0644 change_x_to_notx &&
++ test_file_mode_HEAD 100644 change_x_to_notx &&
+
+ git rm change_x_to_notx &&
-+ test_file_mode_staged "" 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 "" 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 &&
@@ t/t4129-apply-samemode.sh: test_expect_success 'git apply warns about incorrect
+ 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_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 0755 change_x_to_notx &&
++ 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 0755 change_x_to_notx &&
++ 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 0644 change_x_to_notx &&
++ 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 0644 change_x_to_notx &&
++ 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 "" change_x_to_notx &&
++ test_file_mode_staged 000000 change_x_to_notx &&
+ test_tick && git commit -m "redo: remove change_notx_to_x" &&
-+ test_file_mode_HEAD "" change_x_to_notx
++ 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 0644 change_x_to_notx &&
++ test_file_mode_staged 100644 change_x_to_notx &&
+ test_tick && git commit -m "undo: remove change_x_to_notx" &&
-+ test_file_mode_HEAD 0644 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 0755 change_x_to_notx &&
++ 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 0755 change_x_to_notx &&
++ 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 "" change_x_to_notx &&
++ 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 "" change_x_to_notx
++ test_file_mode_HEAD 000000 change_x_to_notx
+'
+
+test_expect_success 'setup: git apply [--reverse] restores file modes (change_notx_to_x)' '
@@ t/t4129-apply-samemode.sh: test_expect_success 'git apply warns about incorrect
+
+ touch change_notx_to_x &&
+ git add --chmod=-x change_notx_to_x &&
-+ test_file_mode_staged 0644 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 0644 change_notx_to_x &&
++ test_file_mode_HEAD 100644 change_notx_to_x &&
+
+ git add --chmod=+x change_notx_to_x &&
-+ test_file_mode_staged 0755 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 0755 change_notx_to_x &&
++ test_file_mode_HEAD 100755 change_notx_to_x &&
+
+ git rm change_notx_to_x &&
-+ test_file_mode_staged "" 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 "" 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 &&
@@ t/t4129-apply-samemode.sh: test_expect_success 'git apply warns about incorrect
+ 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_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 0644 change_notx_to_x &&
++ 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 0644 change_notx_to_x &&
++ 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 0755 change_notx_to_x &&
++ 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 0755 change_notx_to_x &&
++ 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 "" change_notx_to_x &&
++ test_file_mode_staged 000000 change_notx_to_x &&
+ test_tick && git commit -m "undo: remove change_notx_to_x" &&
-+ test_file_mode_HEAD "" 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 0755 change_notx_to_x &&
++ test_file_mode_staged 100755 change_notx_to_x &&
+ test_tick && git commit -m "undo: remove change_notx_to_x" &&
-+ test_file_mode_HEAD 0755 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 0644 change_notx_to_x &&
++ 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 0644 change_notx_to_x &&
++ 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 "" change_notx_to_x &&
++ 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 "" change_notx_to_x
++ test_file_mode_HEAD 000000 change_notx_to_x
+'
+
test_expect_success POSIXPERM 'patch mode for new file is canonicalized' '-- 2.49.0