Skip to content

Commit 1d9a664

Browse files
markmentovaigitster
markmentovai
authored and
gitster
committed
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 <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 2cc8c17 commit 1d9a664

File tree

2 files changed

+164
-3
lines changed

2 files changed

+164
-3
lines changed

‎apply.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2219,7 +2219,7 @@ static void reverse_patches(struct patch *p)
22192219
struct fragment *frag = p->fragments;
22202220

22212221
SWAP(p->new_name, p->old_name);
2222-
if (p->new_mode)
2222+
if (p->new_mode || p->is_delete)
22232223
SWAP(p->new_mode, p->old_mode);
22242224
SWAP(p->is_new, p->is_delete);
22252225
SWAP(p->lines_added, p->lines_deleted);

‎t/t4129-apply-samemode.sh‎

Lines changed: 163 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -102,14 +102,23 @@ test_expect_success POSIXPERM 'do not use core.sharedRepository for working tree
102102
)
103103
'
104104

105+
test_file_mode_common () {
106+
if test "$1" = "000000"
107+
then
108+
test_must_be_empty "$2"
109+
else
110+
test_grep "^$1 " "$2"
111+
fi
112+
}
113+
105114
test_file_mode_staged () {
106115
git ls-files --stage -- "$2" >ls-files-output &&
107-
test_grep "^$1 " ls-files-output
116+
test_file_mode_common "$1" ls-files-output
108117
}
109118

110119
test_file_mode_HEAD () {
111120
git ls-tree HEAD -- "$2" >ls-tree-output &&
112-
test_grep "^$1 " ls-tree-output
121+
test_file_mode_common "$1" ls-tree-output
113122
}
114123

115124
test_expect_success 'git apply respects core.fileMode' '
@@ -189,6 +198,158 @@ test_expect_success 'git apply --reverse warns about incorrect file modes' '
189198
test_file_mode_HEAD 100755 mode_test
190199
'
191200

201+
test_expect_success 'setup: git apply [--reverse] restores file modes (change_x_to_notx)' '
202+
test_config core.fileMode false &&
203+
204+
touch change_x_to_notx &&
205+
git add --chmod=+x change_x_to_notx &&
206+
test_file_mode_staged 100755 change_x_to_notx &&
207+
test_tick && git commit -m "add change_x_to_notx as executable" &&
208+
test_file_mode_HEAD 100755 change_x_to_notx &&
209+
210+
git add --chmod=-x change_x_to_notx &&
211+
test_file_mode_staged 100644 change_x_to_notx &&
212+
test_tick && git commit -m "make change_x_to_notx not executable" &&
213+
test_file_mode_HEAD 100644 change_x_to_notx &&
214+
215+
git rm change_x_to_notx &&
216+
test_file_mode_staged 000000 change_x_to_notx &&
217+
test_tick && git commit -m "remove change_x_to_notx" &&
218+
test_file_mode_HEAD 000000 change_x_to_notx &&
219+
220+
git format-patch -o patches -3 &&
221+
mv patches/0001-* change_x_to_notx-0001-create-0755.patch &&
222+
mv patches/0002-* change_x_to_notx-0002-chmod-0644.patch &&
223+
mv patches/0003-* change_x_to_notx-0003-delete.patch &&
224+
225+
test_grep "^new file mode 100755$" change_x_to_notx-0001-create-0755.patch &&
226+
test_grep "^old mode 100755$" change_x_to_notx-0002-chmod-0644.patch &&
227+
test_grep "^new mode 100644$" change_x_to_notx-0002-chmod-0644.patch &&
228+
test_grep "^deleted file mode 100644$" change_x_to_notx-0003-delete.patch &&
229+
230+
git tag change_x_to_notx_initial
231+
'
232+
233+
test_expect_success 'git apply restores file modes (change_x_to_notx)' '
234+
test_config core.fileMode false &&
235+
git reset --hard change_x_to_notx_initial &&
236+
237+
git apply --index change_x_to_notx-0001-create-0755.patch &&
238+
test_file_mode_staged 100755 change_x_to_notx &&
239+
test_tick && git commit -m "redo: add change_x_to_notx as executable" &&
240+
test_file_mode_HEAD 100755 change_x_to_notx &&
241+
242+
git apply --index change_x_to_notx-0002-chmod-0644.patch 2>err &&
243+
test_grep ! "has type 100.*, expected 100.*" err &&
244+
test_file_mode_staged 100644 change_x_to_notx &&
245+
test_tick && git commit -m "redo: make change_x_to_notx not executable" &&
246+
test_file_mode_HEAD 100644 change_x_to_notx &&
247+
248+
git apply --index change_x_to_notx-0003-delete.patch 2>err &&
249+
test_grep ! "has type 100.*, expected 100.*" err &&
250+
test_file_mode_staged 000000 change_x_to_notx &&
251+
test_tick && git commit -m "redo: remove change_notx_to_x" &&
252+
test_file_mode_HEAD 000000 change_x_to_notx
253+
'
254+
255+
test_expect_success 'git apply --reverse restores file modes (change_x_to_notx)' '
256+
test_config core.fileMode false &&
257+
git reset --hard change_x_to_notx_initial &&
258+
259+
git apply --index --reverse change_x_to_notx-0003-delete.patch &&
260+
test_file_mode_staged 100644 change_x_to_notx &&
261+
test_tick && git commit -m "undo: remove change_x_to_notx" &&
262+
test_file_mode_HEAD 100644 change_x_to_notx &&
263+
264+
git apply --index --reverse change_x_to_notx-0002-chmod-0644.patch 2>err &&
265+
test_grep ! "has type 100.*, expected 100.*" err &&
266+
test_file_mode_staged 100755 change_x_to_notx &&
267+
test_tick && git commit -m "undo: make change_x_to_notx not executable" &&
268+
test_file_mode_HEAD 100755 change_x_to_notx &&
269+
270+
git apply --index --reverse change_x_to_notx-0001-create-0755.patch 2>err &&
271+
test_grep ! "has type 100.*, expected 100.*" err &&
272+
test_file_mode_staged 000000 change_x_to_notx &&
273+
test_tick && git commit -m "undo: add change_x_to_notx as executable" &&
274+
test_file_mode_HEAD 000000 change_x_to_notx
275+
'
276+
277+
test_expect_success 'setup: git apply [--reverse] restores file modes (change_notx_to_x)' '
278+
test_config core.fileMode false &&
279+
280+
touch change_notx_to_x &&
281+
git add --chmod=-x change_notx_to_x &&
282+
test_file_mode_staged 100644 change_notx_to_x &&
283+
test_tick && git commit -m "add change_notx_to_x as not executable" &&
284+
test_file_mode_HEAD 100644 change_notx_to_x &&
285+
286+
git add --chmod=+x change_notx_to_x &&
287+
test_file_mode_staged 100755 change_notx_to_x &&
288+
test_tick && git commit -m "make change_notx_to_x executable" &&
289+
test_file_mode_HEAD 100755 change_notx_to_x &&
290+
291+
git rm change_notx_to_x &&
292+
test_file_mode_staged 000000 change_notx_to_x &&
293+
test_tick && git commit -m "remove change_notx_to_x" &&
294+
test_file_mode_HEAD 000000 change_notx_to_x &&
295+
296+
git format-patch -o patches -3 &&
297+
mv patches/0001-* change_notx_to_x-0001-create-0644.patch &&
298+
mv patches/0002-* change_notx_to_x-0002-chmod-0755.patch &&
299+
mv patches/0003-* change_notx_to_x-0003-delete.patch &&
300+
301+
test_grep "^new file mode 100644$" change_notx_to_x-0001-create-0644.patch &&
302+
test_grep "^old mode 100644$" change_notx_to_x-0002-chmod-0755.patch &&
303+
test_grep "^new mode 100755$" change_notx_to_x-0002-chmod-0755.patch &&
304+
test_grep "^deleted file mode 100755$" change_notx_to_x-0003-delete.patch &&
305+
306+
git tag change_notx_to_x_initial
307+
'
308+
309+
test_expect_success 'git apply restores file modes (change_notx_to_x)' '
310+
test_config core.fileMode false &&
311+
git reset --hard change_notx_to_x_initial &&
312+
313+
git apply --index change_notx_to_x-0001-create-0644.patch &&
314+
test_file_mode_staged 100644 change_notx_to_x &&
315+
test_tick && git commit -m "redo: add change_notx_to_x as not executable" &&
316+
test_file_mode_HEAD 100644 change_notx_to_x &&
317+
318+
git apply --index change_notx_to_x-0002-chmod-0755.patch 2>err &&
319+
test_grep ! "has type 100.*, expected 100.*" err &&
320+
test_file_mode_staged 100755 change_notx_to_x &&
321+
test_tick && git commit -m "redo: make change_notx_to_x executable" &&
322+
test_file_mode_HEAD 100755 change_notx_to_x &&
323+
324+
git apply --index change_notx_to_x-0003-delete.patch &&
325+
test_grep ! "has type 100.*, expected 100.*" err &&
326+
test_file_mode_staged 000000 change_notx_to_x &&
327+
test_tick && git commit -m "undo: remove change_notx_to_x" &&
328+
test_file_mode_HEAD 000000 change_notx_to_x
329+
'
330+
331+
test_expect_success 'git apply --reverse restores file modes (change_notx_to_x)' '
332+
test_config core.fileMode false &&
333+
git reset --hard change_notx_to_x_initial &&
334+
335+
git apply --index --reverse change_notx_to_x-0003-delete.patch &&
336+
test_file_mode_staged 100755 change_notx_to_x &&
337+
test_tick && git commit -m "undo: remove change_notx_to_x" &&
338+
test_file_mode_HEAD 100755 change_notx_to_x &&
339+
340+
git apply --index --reverse change_notx_to_x-0002-chmod-0755.patch 2>err &&
341+
test_grep ! "has type 100.*, expected 100.*" err &&
342+
test_file_mode_staged 100644 change_notx_to_x &&
343+
test_tick && git commit -m "undo: make change_notx_to_x executable" &&
344+
test_file_mode_HEAD 100644 change_notx_to_x &&
345+
346+
git apply --index --reverse change_notx_to_x-0001-create-0644.patch 2>err &&
347+
test_grep ! "has type 100.*, expected 100.*" err &&
348+
test_file_mode_staged 000000 change_notx_to_x &&
349+
test_tick && git commit -m "undo: add change_notx_to_x as not executable" &&
350+
test_file_mode_HEAD 000000 change_notx_to_x
351+
'
352+
192353
test_expect_success POSIXPERM 'patch mode for new file is canonicalized' '
193354
cat >patch <<-\EOF &&
194355
diff --git a/non-canon b/non-canon

0 commit comments

Comments
 (0)