git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Kevin Bracey <kevin@bracey.fi>
To: git@vger.kernel.org
Cc: Kevin Bracey <kevin@bracey.fi>, David Aguilar <davvid@gmail.com>,
	Ciaran Jessup <ciaranj@gmail.com>,
	Scott Chacon <schacon@gmail.com>,
	Alex Riesen <raa.lkml@gmail.com>
Subject: [PATCH v2 2/3] mergetools/p4merge: create a base if none available
Date: Sat,  9 Mar 2013 21:20:59 +0200	[thread overview]
Message-ID: <1362856860-15205-3-git-send-email-kevin@bracey.fi> (raw)
In-Reply-To: <1362856860-15205-1-git-send-email-kevin@bracey.fi>

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 with
similar inputs.

Unfortunately this approach produces much worse results 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 3-way diff is not readable,
and you have to invoke "difftool MERGE_HEAD HEAD" manually to get a
useful view.

The original approach appears to have invoked special 2-way merge
behaviour in P4Merge that occurs only if the base filename is "" or
equal to the left input.  You get a good visual comparison, and it does
not auto-resolve differences. (Normally if one branch matched 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. This is the same as the technique used in resolve and octopus
merges, so we relocate that code to a shared function.

Note that if there are no differences at the same location, this
technique can lead to automatic resolution without conflict, combining
everything from the 2 files.  As with the other merges using this
technique, we assume the user will inspect the result before saving.

Signed-off-by: Kevin Bracey <kevin@bracey.fi>
---
 git-merge-one-file.sh | 18 +++++-------------
 git-sh-setup.sh       | 13 +++++++++++++
 mergetools/p4merge    |  6 +++++-
 3 files changed, 23 insertions(+), 14 deletions(-)

diff --git a/git-merge-one-file.sh b/git-merge-one-file.sh
index f612cb8..1236fbf 100755
--- a/git-merge-one-file.sh
+++ b/git-merge-one-file.sh
@@ -104,30 +104,22 @@ case "${1:-.}${2:-.}${3:-.}" in
 		;;
 	esac
 
-	src2=`git-unpack-file $3`
+	src1=$(git-unpack-file $2)
+	src2=$(git-unpack-file $3)
 	case "$1" in
 	'')
 		echo "Added $4 in both, but differently."
-		# This extracts OUR file in $orig, and uses git apply to
-		# remove lines that are unique to ours.
-		orig=`git-unpack-file $2`
-		sz0=`wc -c <"$orig"`
-		@@DIFF@@ -u -La/$orig -Lb/$orig $orig $src2 | git apply --no-add
-		sz1=`wc -c <"$orig"`
-
-		# 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 || : >$orig
+		orig=$(git-unpack-file $2)
+		create_virtual_base "$orig" "$src1" "$src2"
 		;;
 	*)
 		echo "Auto-merging $4"
-		orig=`git-unpack-file $1`
+		orig=$(git-unpack-file $1)
 		;;
 	esac
 
 	# Be careful for funny filename such as "-L" in "$4", which
 	# would confuse "merge" greatly.
-	src1=`git-unpack-file $2`
 	git merge-file "$src1" "$orig" "$src2"
 	ret=$?
 	msg=
diff --git a/git-sh-setup.sh b/git-sh-setup.sh
index 795edd2..aa9a732 100644
--- a/git-sh-setup.sh
+++ b/git-sh-setup.sh
@@ -249,6 +249,19 @@ clear_local_git_env() {
 	unset $(git rev-parse --local-env-vars)
 }
 
+# Generate a virtual base file for a two-file merge. On entry the
+# base file $1 should be a copy of $2. Uses git apply to remove
+# lines from $1 that are not in $3, leaving only common lines.
+create_virtual_base() {
+	sz0=$(wc -c <"$1")
+	@@DIFF@@ -u -La/"$1" -Lb/"$1" "$2" "$3" | git apply --no-add
+	sz1=$(wc -c <"$1")
+
+	# 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 || : >"$1"
+}
+
 
 # Platform specific tweaks to work around some commands
 case $(uname -s) in
diff --git a/mergetools/p4merge b/mergetools/p4merge
index 46b3a5a..16ae0cc 100644
--- a/mergetools/p4merge
+++ b/mergetools/p4merge
@@ -21,7 +21,11 @@ diff_cmd () {
 
 merge_cmd () {
 	touch "$BACKUP"
-	$base_present || >"$BASE"
+	if ! $base_present
+	then
+		cp -- "$LOCAL" "$BASE"
+		create_virtual_base "$BASE" "$LOCAL" "$REMOTE"
+	fi
 	"$merge_tool_path" "$BASE" "$REMOTE" "$LOCAL" "$MERGED"
 	check_unchanged
 }
-- 
1.8.2.rc3.7.g77aeedb

  parent reply	other threads:[~2013-03-10  0:09 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-03-06 20:32 [PATCH 0/2] Improve P4Merge mergetool invocation Kevin Bracey
2013-03-06 20:32 ` [PATCH 1/2] p4merge: swap LOCAL and REMOTE for mergetool Kevin Bracey
2013-03-07  0:36   ` Junio C Hamano
2013-03-07  6:16     ` Kevin Bracey
2013-03-07  7:23       ` Junio C Hamano
2013-03-07 17:14         ` Kevin Bracey
2013-03-07 19:10           ` Junio C Hamano
2013-03-07 19:50             ` David Aguilar
2013-03-07 20:31               ` Junio C Hamano
2013-03-06 20:32 ` [PATCH 2/2] p4merge: create a virtual base if none available Kevin Bracey
2013-03-07  2:23   ` David Aguilar
2013-03-07  6:28     ` Kevin Bracey
2013-03-07  7:25     ` Junio C Hamano
2013-03-07  3:33   ` David Aguilar
2013-03-09 19:20 ` [PATCH v2 0/3] Improve P4Merge mergetool invocation Kevin Bracey
2013-03-09 19:20   ` [PATCH v2 1/3] mergetools/p4merge: swap LOCAL and REMOTE Kevin Bracey
2013-03-09 19:20   ` Kevin Bracey [this message]
2013-03-10  4:55     ` [PATCH v2 2/3] mergetools/p4merge: create a base if none available Junio C Hamano
2013-03-09 19:21   ` [PATCH v2 3/3] git-merge-one-file: revise merge error reporting Kevin Bracey
2013-03-13  1:12 ` [PATCH v3 1/3] mergetools/p4merge: swap LOCAL and REMOTE Kevin Bracey
2013-03-13  1:12   ` [PATCH v3 2/3] mergetools/p4merge: create a base if none available Kevin Bracey
2013-03-13  1:12   ` [PATCH v3 3/3] git-merge-one-file: revise merge error reporting Kevin Bracey
2013-03-13  9:03     ` David Aguilar
2013-03-24 12:26       ` [PATCH v2 0/3] git-merge-one-file " Kevin Bracey
2013-03-24 12:26         ` [PATCH v2 1/3] git-merge-one-file: style cleanup Kevin Bracey
2013-03-24 12:26         ` [PATCH v2 2/3] git-merge-one-file: send "ERROR:" messages to stderr Kevin Bracey
2013-03-24 12:26         ` [PATCH v2 3/3] git-merge-one-file: revise merge error reporting Kevin Bracey
2013-03-25 17:04           ` Junio C Hamano
2013-03-25 17:17             ` Junio C Hamano
2013-03-25 17:20               ` Junio C Hamano
2013-03-25 19:24               ` Eric Sunshine
2013-03-13 17:57     ` [PATCH v3 " Junio C Hamano
2013-03-14  6:27       ` Kevin Bracey
2013-03-14 14:56         ` Junio C Hamano
2013-03-14 17:31           ` Kevin Bracey
2013-03-14 17:39             ` Kevin Bracey
2013-03-13  2:05   ` [PATCH v3 1/3] mergetools/p4merge: swap LOCAL and REMOTE David Aguilar
2013-03-24 11:54   ` [PATCH v4 1/2] " Kevin Bracey
2013-03-24 11:54     ` [PATCH v4 2/2] mergetools/p4merge: create a base if none available Kevin Bracey
2013-03-25 17:47       ` 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

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

  git send-email \
    --in-reply-to=1362856860-15205-3-git-send-email-kevin@bracey.fi \
    --to=kevin@bracey.fi \
    --cc=ciaranj@gmail.com \
    --cc=davvid@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=raa.lkml@gmail.com \
    --cc=schacon@gmail.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.
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).