From c699a7ccdcca832464d51bff8937cd0bffa1fd9e Mon Sep 17 00:00:00 2001 From: Kevin Bracey Date: Wed, 13 Mar 2013 03:12:20 +0200 Subject: mergetools/p4merge: swap LOCAL and REMOTE Reverse LOCAL and REMOTE when invoking P4Merge as a mergetool, so that the incoming branch is now in the left-hand, blue triangle pane, and the current branch is in the right-hand, green circle pane. This change makes use of P4Merge consistent with its built-in help, its reference documentation, and Perforce itself. But most importantly, it makes merge results clearer. P4Merge is not totally symmetrical between left and right; despite changing a few text labels from "theirs/ours" to "left/right" when invoked manually, it still retains its original Perforce "theirs/ours" viewpoint. Most obviously, in the result pane P4Merge shows changes that are common to both branches in green. This is on the basis of the current branch being green, as it is when invoked from Perforce; it means that lines in the result are blue if and only if they are being changed by the merge, making the resulting diff clearer. Note that P4Merge now shows "ours" on the right for both diff and merge, unlike other diff/mergetools, which always have REMOTE on the right. But observe that REMOTE is the working tree (ie "ours") for a diff, while it's another branch (ie "theirs") for a merge. Ours and theirs are reversed for a rebase - see "git help rebase". However, this does produce the desired "show the results of this commit" effect in P4Merge - changes that remain in the rebased commit (in your branch, but not in the new base) appear in blue; changes that do not appear in the rebased commit (from the new base, or common to both) are in green. If Perforce had rebase, they'd probably not swap ours/theirs, but make P4Merge show common changes in blue, picking out our changes in green. We can't do that, so this is next best. Signed-off-by: Kevin Bracey Reviewed-by: David Aguilar Signed-off-by: Junio C Hamano --- mergetools/p4merge | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mergetools/p4merge b/mergetools/p4merge index 8a3691656..46b3a5a4f 100644 --- a/mergetools/p4merge +++ b/mergetools/p4merge @@ -22,7 +22,7 @@ diff_cmd () { merge_cmd () { touch "$BACKUP" $base_present || >"$BASE" - "$merge_tool_path" "$BASE" "$LOCAL" "$REMOTE" "$MERGED" + "$merge_tool_path" "$BASE" "$REMOTE" "$LOCAL" "$MERGED" check_unchanged } -- cgit v1.2.1 From 4549162e8d623d69ea48745fce3709e5133ce043 Mon Sep 17 00:00:00 2001 From: Kevin Bracey Date: Wed, 13 Mar 2013 03:12:21 +0200 Subject: mergetools/p4merge: create a base if none available 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 Reviewed-by: David Aguilar Signed-off-by: Junio C Hamano --- 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 6a9f66d1d..5d709d02c 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 f612cb847..0f164e54c 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" "$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 795edd285..349a5d44e 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 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 46b3a5a4f..5a608abf9 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" "$REMOTE" + fi "$merge_tool_path" "$BASE" "$REMOTE" "$LOCAL" "$MERGED" check_unchanged } -- cgit v1.2.1 From 530333cfe851b68a3635ee5bcfe707799dab9b83 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Mon, 25 Mar 2013 10:48:24 -0700 Subject: merge-one-file: remove stale comment 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. Signed-off-by: Junio C Hamano --- git-merge-one-file.sh | 2 -- 1 file changed, 2 deletions(-) diff --git a/git-merge-one-file.sh b/git-merge-one-file.sh index 0f164e54c..57fad733b 100755 --- a/git-merge-one-file.sh +++ b/git-merge-one-file.sh @@ -118,8 +118,6 @@ case "${1:-.}${2:-.}${3:-.}" in ;; esac - # Be careful for funny filename such as "-L" in "$4", which - # would confuse "merge" greatly. git merge-file "$src1" "$orig" "$src2" ret=$? msg= -- cgit v1.2.1 From 333ea38db95c77fad8b18daa80bbf0f96fee556c Mon Sep 17 00:00:00 2001 From: Kevin Bracey Date: Sun, 24 Mar 2013 14:26:23 +0200 Subject: git-merge-one-file: style cleanup Update style to match Documentation/CodingGuidelines. Signed-off-by: Kevin Bracey Signed-off-by: Junio C Hamano --- git-merge-one-file.sh | 26 +++++++++++++++++--------- 1 file changed, 17 insertions(+), 9 deletions(-) diff --git a/git-merge-one-file.sh b/git-merge-one-file.sh index 57fad733b..c90b9ffd3 100755 --- a/git-merge-one-file.sh +++ b/git-merge-one-file.sh @@ -27,7 +27,7 @@ SUBDIRECTORY_OK=Yes cd_to_toplevel require_work_tree -if ! test "$#" -eq 7 +if test $# != 7 then echo "$LONG_USAGE" exit 1 @@ -38,7 +38,8 @@ case "${1:-.}${2:-.}${3:-.}" in # Deleted in both or deleted in one and unchanged in the other # "$1.." | "$1.$1" | "$1$1.") - if [ "$2" ]; then + if test -n "$2" + then echo "Removing $4" else # read-tree checked that index matches HEAD already, @@ -48,7 +49,8 @@ case "${1:-.}${2:-.}${3:-.}" in # we do not have it in the index, though. exec git update-index --remove -- "$4" fi - if test -f "$4"; then + if test -f "$4" + then rm -f -- "$4" && rmdir -p "$(expr "z$4" : 'z\(.*\)/')" 2>/dev/null || : fi && @@ -78,7 +80,8 @@ case "${1:-.}${2:-.}${3:-.}" in # Added in both, identically (check for same permissions). # ".$3$2") - if [ "$6" != "$7" ]; then + if test "$6" != "$7" + then echo "ERROR: File $4 added identically in both branches," echo "ERROR: but permissions conflict $6->$7." exit 1 @@ -121,7 +124,8 @@ case "${1:-.}${2:-.}${3:-.}" in git merge-file "$src1" "$orig" "$src2" ret=$? msg= - if [ $ret -ne 0 ]; then + if test $ret != 0 + then msg='content conflict' fi @@ -130,18 +134,22 @@ case "${1:-.}${2:-.}${3:-.}" in git checkout-index -f --stage=2 -- "$4" && cat "$src1" >"$4" || exit 1 rm -f -- "$orig" "$src1" "$src2" - if [ "$6" != "$7" ]; then - if [ -n "$msg" ]; then + if test "$6" != "$7" + then + if test -n "$msg" + then msg="$msg, " fi msg="${msg}permissions conflict: $5->$6,$7" ret=1 fi - if [ "$1" = '' ]; then + if test -z "$1" + then ret=1 fi - if [ $ret -ne 0 ]; then + if test $ret != 0 + then echo "ERROR: $msg in $4" exit 1 fi -- cgit v1.2.1 From d401acf7036cf01d93d138239edf87a59f9627b4 Mon Sep 17 00:00:00 2001 From: Kevin Bracey Date: Sun, 24 Mar 2013 14:26:24 +0200 Subject: git-merge-one-file: send "ERROR:" messages to stderr Signed-off-by: Kevin Bracey Signed-off-by: Junio C Hamano --- git-merge-one-file.sh | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/git-merge-one-file.sh b/git-merge-one-file.sh index c90b9ffd3..a93d0b4cd 100755 --- a/git-merge-one-file.sh +++ b/git-merge-one-file.sh @@ -69,7 +69,7 @@ case "${1:-.}${2:-.}${3:-.}" in echo "Adding $4" if test -f "$4" then - echo "ERROR: untracked $4 is overwritten by the merge." + echo "ERROR: untracked $4 is overwritten by the merge." >&2 exit 1 fi git update-index --add --cacheinfo "$7" "$3" "$4" && @@ -82,8 +82,8 @@ case "${1:-.}${2:-.}${3:-.}" in ".$3$2") if test "$6" != "$7" then - echo "ERROR: File $4 added identically in both branches," - echo "ERROR: but permissions conflict $6->$7." + echo "ERROR: File $4 added identically in both branches," >&2 + echo "ERROR: but permissions conflict $6->$7." >&2 exit 1 fi echo "Adding $4" @@ -98,11 +98,11 @@ case "${1:-.}${2:-.}${3:-.}" in case ",$6,$7," in *,120000,*) - echo "ERROR: $4: Not merging symbolic link changes." + echo "ERROR: $4: Not merging symbolic link changes." >&2 exit 1 ;; *,160000,*) - echo "ERROR: $4: Not merging conflicting submodule changes." + echo "ERROR: $4: Not merging conflicting submodule changes." >&2 exit 1 ;; esac @@ -150,14 +150,14 @@ case "${1:-.}${2:-.}${3:-.}" in if test $ret != 0 then - echo "ERROR: $msg in $4" + echo "ERROR: $msg in $4" >&2 exit 1 fi exec git update-index -- "$4" ;; *) - echo "ERROR: $4: Not handling case $1 -> $2 -> $3" + echo "ERROR: $4: Not handling case $1 -> $2 -> $3" >&2 ;; esac exit 1 -- cgit v1.2.1 From 4fa5c0591a7b5c71c8ccf71d8401c3337b7867ad Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Mon, 25 Mar 2013 10:05:13 -0700 Subject: merge-one-file: force content conflict for "both sides added" case Historically, we tried to be lenient to "both sides added, slightly differently" case and as long as the files can be merged using a made-up common ancestor cleanly, since f7d24bbefb06 (merge with /dev/null as base, instead of punting O==empty case, 2005-11-07). This was later further refined to use a better made-up common file with fd66dbf5297a (merge-one-file: use empty- or common-base condintionally in two-stage merge., 2005-11-10), but the spirit has been the same. But the original fix in f7d24bbefb06 to avoid punting on "both sides added" case had a code to unconditionally error out the merge. When this triggers, even though the content-level merge can be done cleanly, we end up not saying "content conflict" in the message, but still issue the error message, showing "ERROR: in ". Move that "always fail for add/add conflict" logic a bit higher to fix this. Signed-off-by: Junio C Hamano --- git-merge-one-file.sh | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/git-merge-one-file.sh b/git-merge-one-file.sh index a93d0b4cd..7e82facf9 100755 --- a/git-merge-one-file.sh +++ b/git-merge-one-file.sh @@ -124,9 +124,10 @@ case "${1:-.}${2:-.}${3:-.}" in git merge-file "$src1" "$orig" "$src2" ret=$? msg= - if test $ret != 0 + if test $ret != 0 || test -z "$1" then msg='content conflict' + ret=1 fi # Create the working tree file, using "our tree" version from the @@ -143,10 +144,6 @@ case "${1:-.}${2:-.}${3:-.}" in msg="${msg}permissions conflict: $5->$6,$7" ret=1 fi - if test -z "$1" - then - ret=1 - fi if test $ret != 0 then -- cgit v1.2.1