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

Re: [PATCH 2/2] p4merge: create a virtual base if none available

From
David Aguilar <davvid@gmail.com>
Date
Mar 7, 2013, 02:23 UTC
Message-ID
<CAJDDKr6+VRnc-HK52woHHLtAqXau=76Gc+Ag=keiMGffuco64A@mail.gmail.com>
In-Reply-To
<1362601978-16911-3-git-send-email-kevin@bracey.fi>
On Wed, Mar 6, 2013 at 12:32 PM, Kevin Bracey <kevin@bracey.fi> wrote:
Show 49 quoted lines
> Originally, with no base, Git gave P4Merge $LOCAL as a dummy base:
>
>    p4merge "$LOCAL" "$LOCAL" "$REMOTE" "$MERGED"
>
> Commit 0a0ec7bd changed this to:
>
>    p4merge "empty file" "$LOCAL" "$REMOTE" "$MERGED"
>
> to avoid the problem of being unable to save in some circumstances.
>
> Unfortunately this approach does not produce good results at all on
> differing inputs. P4Merge really regards the blank file as the base, and
> once you have just a couple of differences between the two branches you
> end up with one a massive full-file conflict. The diff is not readable,
> and you have to invoke "difftool MERGE_HEAD HEAD" manually to see a
> 2-way diff.
>
> The original form appears to have invoked special 2-way comparison
> behaviour that occurs only if the base filename is "" or equal to the
> left input.  You get a good diff, and it does not auto-resolve in one
> direction or the other. (Normally if one branch equals the base, it
> would autoresolve to the other branch).
>
> But there appears to be no way of getting this 2-way behaviour and being
> able to reliably save. Having base=left appears to be triggering other
> assumptions. There are tricks the user can use to force the save icon
> on, but it's not intuitive.
>
> So we now follow a suggestion given in the original patch's discussion:
> generate a virtual base, consisting of the lines common to the two
> branches. It produces a much nicer 3-way diff view than either of the
> original forms, and than I suspect other mergetools are managing.
>
> Signed-off-by: Kevin Bracey <kevin@bracey.fi>
> ---
>  git-mergetool--lib.sh | 14 ++++++++++++++
>  mergetools/p4merge    |  2 +-
>  2 files changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
> index e338be5..5b60cf5 100644
> --- a/git-mergetool--lib.sh
> +++ b/git-mergetool--lib.sh
> @@ -108,6 +108,20 @@ check_unchanged () {
>         fi
>  }
>
> +make_virtual_base() {
> +               # Copied from git-merge-one-file.sh.
I think the reasoning behind these patches is good.

How do we feel about this duplication? Should we make a common function in the git-sh-setup.sh, or is it okay to have a slightly modified version of this function in two places?

Show 31 quoted lines
> +               # This starts with $LOCAL, and uses git apply to
> +               # remove lines that are not in $REMOTE.
> +               cp -- "$LOCAL" "$BASE"
> +               sz0=`wc -c <"$BASE"`
> +               @@DIFF@@ -u -L"a/$BASE" -L"b/$BASE" "$BASE" "$REMOTE" | git apply --no-add
> +               sz1=`wc -c <"$BASE"`
> +
> +               # If we do not have enough common material, it is not
> +               # worth trying two-file merge using common subsections.
> +               expr $sz0 \< $sz1 \* 2 >/dev/null || : >"$BASE"
> +}
> +
>  valid_tool () {
>         setup_tool "$1" && return 0
>         cmd=$(get_merge_tool_cmd "$1")
> diff --git a/mergetools/p4merge b/mergetools/p4merge
> index 46b3a5a..f0a893b 100644
> --- a/mergetools/p4merge
> +++ b/mergetools/p4merge
> @@ -21,7 +21,7 @@ diff_cmd () {
>
>  merge_cmd () {
>         touch "$BACKUP"
> -       $base_present || >"$BASE"
> +       $base_present || make_virtual_base
>         "$merge_tool_path" "$BASE" "$REMOTE" "$LOCAL" "$MERGED"
>         check_unchanged
>  }
> --
> 1.8.2.rc2.5.g1a80410.dirty
>
-- 
David
Previous: Kevin BraceyNext: Kevin Bracey
Message 11 of 40 in “Improve P4Merge mergetool invocation”
  1. 0/2 Improve P4Merge mergetool invocationKevin Bracey, Mar 6, 2013
  2. 1/2 p4merge: swap LOCAL and REMOTE for mergetoolKevin Bracey, Mar 6, 2013
  3. Junio C HamanoMar 7, 2013
  4. Kevin BraceyMar 7, 2013
  5. Junio C HamanoMar 7, 2013
  6. Kevin BraceyMar 7, 2013
  7. Junio C HamanoMar 7, 2013
  8. David AguilarMar 7, 2013
  9. Junio C HamanoMar 7, 2013
  10. 2/2 p4merge: create a virtual base if none availableKevin Bracey, Mar 6, 2013
  11. David AguilarMar 7, 2013
  12. Kevin BraceyMar 7, 2013
  13. Junio C HamanoMar 7, 2013
  14. David AguilarMar 7, 2013
  15. 0/3 Improve P4Merge mergetool invocationKevin Bracey, Mar 9, 2013
  16. 1/3 mergetools/p4merge: swap LOCAL and REMOTEKevin Bracey, Mar 9, 2013
  17. 2/3 mergetools/p4merge: create a base if none availableKevin Bracey, Mar 9, 2013
  18. Junio C HamanoMar 10, 2013
  19. 3/3 git-merge-one-file: revise merge error reportingKevin Bracey, Mar 9, 2013
  20. 1/3 mergetools/p4merge: swap LOCAL and REMOTEKevin Bracey, Mar 13, 2013
  21. 2/3 mergetools/p4merge: create a base if none availableKevin Bracey, Mar 13, 2013
  22. 3/3 git-merge-one-file: revise merge error reportingKevin Bracey, Mar 13, 2013
  23. David AguilarMar 13, 2013
  24. 0/3 git-merge-one-file error reportingKevin Bracey, Mar 24, 2013
  25. 1/3 git-merge-one-file: style cleanupKevin Bracey, Mar 24, 2013
  26. 2/3 git-merge-one-file: send "ERROR:" messages to stderrKevin Bracey, Mar 24, 2013
  27. 3/3 git-merge-one-file: revise merge error reportingKevin Bracey, Mar 24, 2013
  28. Junio C HamanoMar 25, 2013
  29. Junio C HamanoMar 25, 2013
  30. Junio C HamanoMar 25, 2013
  31. Eric SunshineMar 25, 2013
  32. Junio C HamanoMar 13, 2013
  33. Kevin BraceyMar 14, 2013
  34. Junio C HamanoMar 14, 2013
  35. Kevin BraceyMar 14, 2013
  36. Kevin BraceyMar 14, 2013
  37. David AguilarMar 13, 2013
  38. 1/2 mergetools/p4merge: swap LOCAL and REMOTEKevin Bracey, Mar 24, 2013
  39. 2/2 mergetools/p4merge: create a base if none availableKevin Bracey, Mar 24, 2013
  40. Junio C HamanoMar 25, 2013

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.