archive mirror
 help / color / mirror / Atom feed
* [PATCH] apply: when -R, also reverse list of sections
@ 2020-09-28 21:20 Jonathan Tan
  2020-09-28 22:07 ` Junio C Hamano
  0 siblings, 1 reply; 8+ messages in thread
From: Jonathan Tan @ 2020-09-28 21:20 UTC (permalink / raw)
  To: git; +Cc: Jonathan Tan

A patch changing a symlink into a file is written with 2 sections (in
the code, represented as "struct patch"): firstly, the deletion of the
symlink, and secondly, the creation of the file. When applying that
patch with -R, the sections are reversed, so we get:

 (1) creation of a symlink, then
 (2) deletion of a file.

This causes an issue when the "deletion of a file" section is checked,
because Git observes that the so-called file is not a file but a
symlink, resulting in a "wrong type" error message.

What we want is:

 (1) deletion of a file, then
 (2) creation of a symlink.

In the code, this is reflected in the behavior of previous_patch() when
invoked from check_preimage() when the deletion is checked. Creation
then deletion means that when the deletion is checked, previous_patch()
returns the creation section, triggering a mode conflict resulting in
the "wrong type" error message. But deletion then creation means that
when the deletion is checked, previous_patch() returns NULL, so the
deletion mode is checked against lstat, which is what we want.

Therefore, when building the list of sections, build them in reverse
order (by adding to the front of the list instead of the back) when -R
is passed.

Signed-off-by: Jonathan Tan <>
I traced the different behavior in previous_patch() to what the
invocation of to_be_deleted() returns, but I couldn't figure out why it
returns false in the current-but-wrong-order case but true in the
future-and-correct-order case. I see that prepare_fn_table() sets
PATH_TO_BE_DELETED for deletions, but couldn't figure out where and when
it is set to something else. Further compounding my confusion,
conceptually, at the point of checking the deletion, both patches are
(in theory) "to be deleted". Any help in this is appreciated.
 apply.c                     | 9 +++++++--
 t/ | 7 +++++++
 2 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/apply.c b/apply.c
index 76dba93c97..359ceb632c 100644
--- a/apply.c
+++ b/apply.c
@@ -4699,8 +4699,13 @@ static int apply_patch(struct apply_state *state,
 		if (use_patch(state, patch)) {
 			patch_stats(state, patch);
-			*listp = patch;
-			listp = &patch->next;
+			if (!list || !state->apply_in_reverse) {
+				*listp = patch;
+				listp = &patch->next;
+			} else {
+				patch->next = list;
+				list = patch;
+			}
 			if ((patch->new_name &&
diff --git a/t/ b/t/
index ebadbc347f..da3e64f811 100755
--- a/t/
+++ b/t/
@@ -88,6 +88,13 @@ test_expect_success 'symlink becomes file' '
 test_debug 'cat patch'
+test_expect_success 'symlink becomes file, in reverse' '
+	git checkout -f foo-symlinked-to-bar &&
+	git diff-tree -p HEAD foo-back-to-file > patch &&
+	git checkout foo-back-to-file &&
+	git apply -R --index < patch
+	'
 test_expect_success 'binary file becomes symlink' '
 	git checkout -f foo-becomes-binary &&
 	git diff-tree -p --binary HEAD foo-symlinked-to-bar > patch &&

^ permalink raw reply related	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2020-10-20 22:04 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-28 21:20 [PATCH] apply: when -R, also reverse list of sections Jonathan Tan
2020-09-28 22:07 ` Junio C Hamano
2020-10-20 19:12   ` Jonathan Tan
2020-10-20 20:06     ` Junio C Hamano
2020-10-20 20:50       ` Junio C Hamano
2020-10-20 21:36         ` Jonathan Tan
2020-10-20 21:48           ` Junio C Hamano
2020-10-20 22:04             ` [PATCH v2] " Jonathan Tan

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).