From: Jerry Zhang <jerry@skydio.com>
To: git@vger.kernel.org, newren@gmail.com, gitster@pobox.com
Cc: ross@skydio.com, abe@skydio.com, brian.kubisiak@skydio.com,
Jerry Zhang <jerry@skydio.com>
Subject: [PATCH] git-apply: try threeway first when "--3way" is used
Date: Tue, 6 Apr 2021 16:25:32 -0700 [thread overview]
Message-ID: <20210406232532.3543-1-jerry@skydio.com> (raw)
In-Reply-To: <20210406025551.25213-1-jerry@skydio.com>
The apply_fragments() method of "git apply"
can silently apply patches incorrectly if
a file has repeating contents. In these
cases a three-way merge is capable of applying
it correctly in more situations, and will
show a conflict rather than applying it
incorrectly. However, because the patches
apply "successfully" using apply_fragments(),
git will never fall back to the merge, even
if the "--3way" flag is used, and the user has
no way to ensure correctness by forcing the
three-way merge method.
Change the behavior so that when "--3way" is used,
git will always try the three-way merge first and
will only fall back to apply_fragments() in cases
where blobs are not available or some other error
(but not in the case of a merge conflict).
Since user-facing results will be different,
this has backwards compatibility implications
for users depending on the old behavior. In
addition, the three-way merge will be slower
than direct patch application.
Signed-off-by: Jerry Zhang <jerry@skydio.com>
---
Documentation/git-apply.txt | 5 ++---
apply.c | 13 ++++++-------
t/t4108-apply-threeway.sh | 20 ++++++++++++++++++++
3 files changed, 28 insertions(+), 10 deletions(-)
diff --git a/Documentation/git-apply.txt b/Documentation/git-apply.txt
index 91d9a8601c8c316d4649c405af42e531c39991a8..9144575299c264dd299b542b7b5948eef35f211c 100644
--- a/Documentation/git-apply.txt
+++ b/Documentation/git-apply.txt
@@ -84,9 +84,8 @@ OPTIONS
-3::
--3way::
- When the patch does not apply cleanly, fall back on 3-way merge if
- the patch records the identity of blobs it is supposed to apply to,
- and we have those blobs available locally, possibly leaving the
+ Attempt 3-way merge if the patch records the identity of blobs it is supposed
+ to apply to and we have those blobs available locally, possibly leaving the
conflict markers in the files in the working tree for the user to
resolve. This option implies the `--index` option, and is incompatible
with the `--reject` and the `--cached` options.
diff --git a/apply.c b/apply.c
index 6695a931e979a968b28af88d425d0c76ba17d0d4..9bd4efcbced842d2c5c030a0f2178ddb36114600 100644
--- a/apply.c
+++ b/apply.c
@@ -3569,10 +3569,10 @@ static int try_threeway(struct apply_state *state,
write_object_file("", 0, blob_type, &pre_oid);
else if (get_oid(patch->old_oid_prefix, &pre_oid) ||
read_blob_object(&buf, &pre_oid, patch->old_mode))
- return error(_("repository lacks the necessary blob to fall back on 3-way merge."));
+ return error(_("repository lacks the necessary blob to perform 3-way merge."));
if (state->apply_verbosity > verbosity_silent)
- fprintf(stderr, _("Falling back to three-way merge...\n"));
+ fprintf(stderr, _("Performing three-way merge...\n"));
img = strbuf_detach(&buf, &len);
prepare_image(&tmp_image, img, len, 1);
@@ -3604,7 +3604,7 @@ static int try_threeway(struct apply_state *state,
if (status < 0) {
if (state->apply_verbosity > verbosity_silent)
fprintf(stderr,
- _("Failed to fall back on three-way merge...\n"));
+ _("Failed to perform three-way merge...\n"));
return status;
}
@@ -3637,10 +3637,9 @@ static int apply_data(struct apply_state *state, struct patch *patch,
if (load_preimage(state, &image, patch, st, ce) < 0)
return -1;
- if (patch->direct_to_threeway ||
- apply_fragments(state, &image, patch) < 0) {
+ if (!state->threeway || try_threeway(state, &image, patch, st, ce) < 0) {
/* Note: with --reject, apply_fragments() returns 0 */
- if (!state->threeway || try_threeway(state, &image, patch, st, ce) < 0)
+ if (patch->direct_to_threeway || apply_fragments(state, &image, patch) < 0)
return -1;
}
patch->result = image.buf;
@@ -5017,7 +5016,7 @@ int apply_parse_options(int argc, const char **argv,
OPT_BOOL(0, "apply", force_apply,
N_("also apply the patch (use with --stat/--summary/--check)")),
OPT_BOOL('3', "3way", &state->threeway,
- N_( "attempt three-way merge if a patch does not apply")),
+ N_( "attempt three-way merge, fall back on normal patch if that fails")),
OPT_FILENAME(0, "build-fake-ancestor", &state->fake_ancestor,
N_("build a temporary index based on embedded index information")),
/* Think twice before adding "--nul" synonym to this */
diff --git a/t/t4108-apply-threeway.sh b/t/t4108-apply-threeway.sh
index d62db3fbe16f35a625a4a14eebb70034f695d3eb..9ff313f976422f9c12dc8032d14567b54cfe3765 100755
--- a/t/t4108-apply-threeway.sh
+++ b/t/t4108-apply-threeway.sh
@@ -160,4 +160,24 @@ test_expect_success 'apply -3 with add/add conflict (dirty working tree)' '
test_cmp three.save three
'
+test_expect_success 'apply -3 with ambiguous repeating file' '
+ git reset --hard &&
+ test_write_lines 1 2 1 2 1 2 1 2 1 2 1 >one_two_repeat &&
+ git add one_two_repeat &&
+ git commit -m "init one" &&
+ test_write_lines 1 2 1 2 1 2 1 2 one 2 1 >one_two_repeat &&
+ git commit -a -m "change one" &&
+
+ git diff HEAD~ >Repeat.diff &&
+ git reset --hard HEAD~ &&
+
+ test_write_lines 1 2 1 2 1 2 one 2 1 2 one >one_two_repeat &&
+ git commit -a -m "change surrounding one" &&
+
+ git apply --index --3way Repeat.diff &&
+ test_write_lines 1 2 1 2 1 2 one 2 one 2 one >expect &&
+
+ test_cmp expect one_two_repeat
+'
+
test_done
--
2.29.0
next prev parent reply other threads:[~2021-04-06 23:25 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-04-06 2:55 [PATCH] git-apply: try threeway first when "--3way" is used Jerry Zhang
2021-04-06 6:13 ` Junio C Hamano
2021-04-06 23:13 ` Junio C Hamano
2021-04-06 6:14 ` Junio C Hamano
2021-04-06 23:25 ` Jerry Zhang [this message]
2021-04-07 0:19 ` Junio C Hamano
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
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210406232532.3543-1-jerry@skydio.com \
--to=jerry@skydio.com \
--cc=abe@skydio.com \
--cc=brian.kubisiak@skydio.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=newren@gmail.com \
--cc=ross@skydio.com \
/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
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
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 read-only IMAP folder(s) and NNTP newsgroup(s).