git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jonathan Tan <jonathantanmy@google.com>
To: git@vger.kernel.org
Cc: Jonathan Tan <jonathantanmy@google.com>,
	Junio C Hamano <gitster@pobox.com>
Subject: [PATCH v2] apply: when -R, also reverse list of sections
Date: Tue, 20 Oct 2020 15:04:52 -0700	[thread overview]
Message-ID: <20201020220452.697760-1-jonathantanmy@google.com> (raw)
In-Reply-To: <xmqqblgwa7s5.fsf@gitster.c.googlers.com>

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.

There are also other ways a patch can contain 2 sections referencing the
same file, for example, in 7a07841c0b ("git-apply: handle a patch that
touches the same path more than once better", 2008-06-27). "git apply
-R" fails in the same way, and this commit makes this case succeed.

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.

Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
---
OK - updated the commit message and added a test for the use case Junio
described.
---
 apply.c                     | 9 +++++++--
 t/t4114-apply-typechange.sh | 7 +++++++
 t/t4127-apply-same-fn.sh    | 9 +++++++++
 3 files changed, 23 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,
 			reverse_patches(patch);
 		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 &&
 			     ends_with_path_components(patch->new_name,
diff --git a/t/t4114-apply-typechange.sh b/t/t4114-apply-typechange.sh
index ebadbc347f..da3e64f811 100755
--- a/t/t4114-apply-typechange.sh
+++ b/t/t4114-apply-typechange.sh
@@ -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 &&
diff --git a/t/t4127-apply-same-fn.sh b/t/t4127-apply-same-fn.sh
index 972946c174..305b7e649e 100755
--- a/t/t4127-apply-same-fn.sh
+++ b/t/t4127-apply-same-fn.sh
@@ -32,6 +32,10 @@ test_expect_success 'apply same filename with independent changes' '
 
 test_expect_success 'apply same filename with overlapping changes' '
 	git reset --hard &&
+
+	# Store same_fn so that we can check apply -R in next test
+	cp same_fn same_fn1 &&
+
 	modify "s/^d/z/" same_fn &&
 	git diff > patch0 &&
 	git add same_fn &&
@@ -43,6 +47,11 @@ test_expect_success 'apply same filename with overlapping changes' '
 	test_cmp same_fn same_fn2
 '
 
+test_expect_success 'apply same filename with overlapping changes, in reverse' '
+	git apply -R patch0 &&
+	test_cmp same_fn same_fn1
+'
+
 test_expect_success 'apply same new filename after rename' '
 	git reset --hard &&
 	git mv same_fn new_fn &&
-- 
2.29.0.rc1.297.gfa9743e501-goog


      reply	other threads:[~2020-10-20 22:04 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-28 21:20 [PATCH] " 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             ` Jonathan Tan [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20201020220452.697760-1-jonathantanmy@google.com \
    --to=jonathantanmy@google.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --subject='Re: [PATCH v2] apply: when -R, also reverse list of sections' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

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).