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

2013-03-25 Thread Junio C Hamano
Kevin Bracey ke...@bracey.fi writes:

 Minor change from v3: that version moved initialisation of src1 higher up,
 detaching it from its associated comment. This move was only required by
 earlier versions, so v4 leaves src1 in its original position.

The funny filename comment was from b539c5e8fbd3 (git-merge-one:
new merge world order., 2005-12-07) where the removed code just
before that new comment ended with:

merge $4 $orig $src2

(yes, we used to use merge program from the RCS suite).  The
comment refers to one of the bad side effect the old code used to
have and warns against such a practice, i.e. it was talking about
the code that no longer existed.

I think the two-line comment should simply go.

Given that, I _think_ it is OK to move the initialization of src1
next to that of src2; that may make the result easier to read.

Thanks.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v4 2/2] mergetools/p4merge: create a base if none available

2013-03-24 Thread Kevin Bracey
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 ke...@bracey.fi
Reviewed-by: David Aguilar dav...@gmail.com
---
Minor change from v3: that version moved initialisation of src1 higher up,
detaching it from its associated comment. This move was only required by
earlier versions, so v4 leaves src1 in its original position.

Added Reviewed-by footer.

 Documentation/git-sh-setup.txt |  6 ++
 git-merge-one-file.sh  | 18 +-
 git-sh-setup.sh| 12 
 mergetools/p4merge |  6 +-
 4 files changed, 28 insertions(+), 14 deletions(-)

diff --git a/Documentation/git-sh-setup.txt b/Documentation/git-sh-setup.txt
index 6a9f66d..5d709d0 100644
--- a/Documentation/git-sh-setup.txt
+++ b/Documentation/git-sh-setup.txt
@@ -82,6 +82,12 @@ get_author_ident_from_commit::
outputs code for use with eval to set the GIT_AUTHOR_NAME,
GIT_AUTHOR_EMAIL and GIT_AUTHOR_DATE variables for a given commit.
 
+create_virtual_base::
+   modifies the first file so only lines in common with the
+   second file remain. If there is insufficient common material,
+   then the first file is left empty. The result is suitable
+   as a virtual base input for a 3-way merge.
+
 GIT
 ---
 Part of the linkgit:git[1] suite
diff --git a/git-merge-one-file.sh b/git-merge-one-file.sh
index 3373c04..255c07a 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`
+   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 $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`
+   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 9cfbe7f..2f78359 100644
--- a/git-sh-setup.sh
+++ b/git-sh-setup.sh
@@ -249,6 +249,18 @@ clear_local_git_env() {
unset $(git rev-parse --local-env-vars)
 }
 
+# Generate a virtual base file for a two-file merge. Uses git apply to
+# remove lines from $1 that are not in $2, leaving only common lines.
+create_virtual_base() {
+   sz0=$(wc -c $1)
+   @@DIFF@@ -u -La/$1 -Lb/$1 $1 $2 | git apply --no-add
+   sz1=$(wc -c $1)
+
+   # If we do not have enough common material, it