git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH v2] apply: resolve trivial merge without hitting ll-merge with "--3way"

From
Elijah Newren <newren@gmail.com>
Date
Sep 6, 2021, 18:57 UTC
Message-ID
<CABPp-BGrg7eBCeq7SLvx3N5p7HyKGwS7qwTe=+En6OfiKhiXPQ@mail.gmail.com>
In-Reply-To
<20210905190657.2906699-1-gitster@pobox.com>
On Sun, Sep 5, 2021 at 12:07 PM Junio C Hamano <gitster@pobox.com> wrote:
Show 133 quoted lines
>
> The ll_binary_merge() function assumes that the ancestor blob is
> different from either side of the new versions, and always fails
> the merge in conflict, unless -Xours or -Xtheirs is in effect.
>
> The normal "merge" machineries all resolve the trivial cases
> (e.g. if our side changed while their side did not, the result
> is ours) without triggering the file-level merge drivers, so the
> assumption is warranted.
>
> The code path in "git apply --3way", however, does not check for
> the trivial three-way merge situation and always calls the
> file-level merge drivers.  This used to be perfectly OK back
> when we always first attempted a straight patch application and
> used the three-way code path only as a fallback.  Any binary
> patch that can be applied as a trivial three-way merge (e.g. the
> patch is based exactly on the version we happen to have) would
> always cleanly apply, so the ll_binary_merge() that is not
> prepared to see the trivial case would not have to handle such a
> case.
>
> This no longer is true after we made "--3way" to mean "first try
> three-way and then fall back to straight application", and made
> "git apply -3" on a binary patch that is based on the current
> version no longer apply.
>
> Teach "git apply -3" to first check for the trivial merge cases
> and resolve them without hitting the file-level merge drivers.
>
> Signed-off-by: Jerry Zhang <jerry@skydio.com>
> [jc: stolen tests from Jerry's patch]
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>
>  apply.c                   | 21 ++++++++++++++++++
>  t/t4108-apply-threeway.sh | 45 +++++++++++++++++++++++++++++++++++++++
>  2 files changed, 66 insertions(+)
>
> diff --git a/apply.c b/apply.c
> index 44bc31d6eb..c9f9503e90 100644
> --- a/apply.c
> +++ b/apply.c
> @@ -3467,6 +3467,21 @@ static int load_preimage(struct apply_state *state,
>         return 0;
>  }
>
> +static int resolve_to(struct image *image, const struct object_id *result_id)
> +{
> +       unsigned long size;
> +       enum object_type type;
> +
> +       clear_image(image);
> +
> +       image->buf = read_object_file(result_id, &type, &size);
> +       if (!image->buf || type != OBJ_BLOB)
> +               die("unable to read blob object %s", oid_to_hex(result_id));
> +       image->len = size;
> +
> +       return 0;
> +}
> +
>  static int three_way_merge(struct apply_state *state,
>                            struct image *image,
>                            char *path,
> @@ -3478,6 +3493,12 @@ static int three_way_merge(struct apply_state *state,
>         mmbuffer_t result = { NULL };
>         int status;
>
> +       /* resolve trivial cases first */
> +       if (oideq(base, ours))
> +               return resolve_to(image, theirs);
> +       else if (oideq(base, theirs) || oideq(ours, theirs))
> +               return resolve_to(image, ours);
> +
>         read_mmblob(&base_file, base);
>         read_mmblob(&our_file, ours);
>         read_mmblob(&their_file, theirs);
> diff --git a/t/t4108-apply-threeway.sh b/t/t4108-apply-threeway.sh
> index 65147efdea..cc3aa3314a 100755
> --- a/t/t4108-apply-threeway.sh
> +++ b/t/t4108-apply-threeway.sh
> @@ -230,4 +230,49 @@ test_expect_success 'apply with --3way --cached and conflicts' '
>         test_cmp expect.diff actual.diff
>  '
>
> +test_expect_success 'apply binary file patch' '
> +       git reset --hard main &&
> +       cp "$TEST_DIRECTORY/test-binary-1.png" bin.png &&
> +       git add bin.png &&
> +       git commit -m "add binary file" &&
> +
> +       cp "$TEST_DIRECTORY/test-binary-2.png" bin.png &&
> +
> +       git diff --binary >bin.diff &&
> +       git reset --hard &&
> +
> +       # Apply must succeed.
> +       git apply bin.diff
> +'
> +
> +test_expect_success 'apply binary file patch with 3way' '
> +       git reset --hard main &&
> +       cp "$TEST_DIRECTORY/test-binary-1.png" bin.png &&
> +       git add bin.png &&
> +       git commit -m "add binary file" &&
> +
> +       cp "$TEST_DIRECTORY/test-binary-2.png" bin.png &&
> +
> +       git diff --binary >bin.diff &&
> +       git reset --hard &&
> +
> +       # Apply must succeed.
> +       git apply --3way --index bin.diff
> +'
> +
> +test_expect_success 'apply full-index patch with 3way' '
> +       git reset --hard main &&
> +       cp "$TEST_DIRECTORY/test-binary-1.png" bin.png &&
> +       git add bin.png &&
> +       git commit -m "add binary file" &&
> +
> +       cp "$TEST_DIRECTORY/test-binary-2.png" bin.png &&
> +
> +       git diff --full-index >bin.diff &&
> +       git reset --hard &&
> +
> +       # Apply must succeed.
> +       git apply --3way --index bin.diff
> +'
> +
>  test_done
> --
> 2.33.0-408-g8e1aa136b3
Reviewed-by: Elijah Newren <newren@gmail.com>
Previous: Junio C HamanoNext: Ævar Arnfjörð Bjarmason
Message 8 of 16 in “git-apply: fix --3way with binary patch”
  1. git-apply: fix --3way with binary patchJerry Zhang, Jul 28, 2021
  2. Junio C HamanoJul 28, 2021
  3. Junio C HamanoJul 28, 2021
  4. ll-merge: teach ll_binary_merge() a trivial three-way mergeJunio C Hamano, Jul 28, 2021
  5. Elijah NewrenJul 28, 2021
  6. Junio C HamanoJul 29, 2021
  7. apply: resolve trivial merge without hitting ll-merge with "--3way"Junio C Hamano, Sep 5, 2021
  8. Elijah NewrenSep 6, 2021
  9. Ævar Arnfjörð BjarmasonSep 6, 2021
  10. Junio C HamanoSep 7, 2021
  11. Junio C HamanoSep 7, 2021
  12. Jerry ZhangJul 28, 2021
  13. Jerry ZhangJul 28, 2021
  14. Junio C HamanoJul 28, 2021
  15. Jerry ZhangJul 28, 2021
  16. Junio C HamanoJul 28, 2021

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.