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 <mark@chromium.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Mark Mentovai committed
May 23, 2025 at 23:40 UTC
1d9a66493be9fef38e531da587dd47f6f17ea483
2 files changed
+164
-3
apply.c
+1
-1
@@ -2219,7 +2219,7 @@ static void reverse_patches(struct patch *p)
2219
struct fragment *frag = p->fragments;
2220
2221
SWAP(p->new_name, p->old_name);
2222
- if (p->new_mode)
2222
+ if (p->new_mode || p->is_delete)
2223
SWAP(p->new_mode, p->old_mode);
2224
SWAP(p->is_new, p->is_delete);
2225
SWAP(p->lines_added, p->lines_deleted);
t/t4129-apply-samemode.sh
+163
-2
@@ -102,14 +102,23 @@ test_expect_success POSIXPERM 'do not use core.sharedRepository for working tree
102
)
103
'
104
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
+
114
test_file_mode_staged () {
115
git ls-files --stage -- "$2" >ls-files-output &&
107
- test_grep "^$1 " ls-files-output
116
+ test_file_mode_common "$1" ls-files-output
117
}
118
119
test_file_mode_HEAD () {
120
git ls-tree HEAD -- "$2" >ls-tree-output &&
112
- test_grep "^$1 " ls-tree-output
121
+ test_file_mode_common "$1" ls-tree-output
122
}
123
124
test_expect_success 'git apply respects core.fileMode' '
@@ -189,6 +198,158 @@ test_expect_success 'git apply --reverse warns about incorrect file modes' '
198
test_file_mode_HEAD 100755 mode_test
199
'
200
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
+
353
test_expect_success POSIXPERM 'patch mode for new file is canonicalized' '
354
cat >patch <<-\EOF &&
355
diff --git a/non-canon b/non-canon