Skip to content

Commit 7a10c59

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 67f27b3 commit 7a10c59

File tree

2 files changed

+151
-3
lines changed

2 files changed

+151
-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: 150 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -102,14 +102,18 @@ test_expect_success POSIXPERM 'do not use core.sharedRepository for working tree
102102
)
103103
'
104104

105+
test_file_mode_common() {
106+
test -n "$1" && test_grep "^10$1 " "$2" || test_must_be_empty "$2"
107+
}
108+
105109
test_file_mode_staged () {
106110
git ls-files --stage -- "$2" >ls-files-output &&
107-
test_grep "^10$1 " ls-files-output
111+
test_file_mode_common "$1" ls-files-output
108112
}
109113

110114
test_file_mode_HEAD () {
111115
git ls-tree HEAD -- "$2" >ls-tree-output &&
112-
test_grep "^10$1 " ls-tree-output
116+
test_file_mode_common "$1" ls-tree-output
113117
}
114118

115119
test_expect_success 'git apply respects core.fileMode' '
@@ -180,6 +184,150 @@ test_expect_success 'git apply warns about incorrect file modes' '
180184
test_file_mode_HEAD 0755 mode_test
181185
'
182186

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

0 commit comments

Comments
 (0)