Re: [RFC/PATCH 2/4] Restructuring git-merge.sh

for preparation of new feature:

   Head reduction before selecting merge strategy

Signed-off-by: Sverre Hvammen Johansen <redacted>
---
 git-merge.sh |  166 ++++++++++++++++++++++++++++++----------------------------
 1 files changed, 85 insertions(+), 81 deletions(-)

diff --git a/git-merge.sh b/git-merge.sh
index 17f40f2..2acd2cc 100755
--- a/git-merge.sh
+++ b/git-merge.sh

@@ -207,6 +207,29 @@ parse_config () {
        args_left=$#
 }

+# Find real parents
+# Set the following variables as followd:
+#   real_parents: The parents specified on the command line
+#   common:       All common ancestors or not_queried
+#   ff_head:      Fast forward of head
+find_real_parents () {
+       real_parents=$(git rev-parse "$@")
+       real_parents=${real_parents#$LF}
+       if test $# = 1
+       then
+               common=$(git merge-base --all $head "$@")
+               if test "$common" = $head
+               then
+                       ff_head=$1
+               else
+                       ff_head=$head
+               fi
+       else
+               common=not_queried
+               ff_head=$head
+       fi
+}
+
 test $# != 0 || usage

 have_message=

@@ -294,24 +317,26 @@ do
 done
 set x $remoteheads ; shift

+find_real_parents "$@"
+
 case "$use_strategies" in
 '')
-       case "$#" in
-       1)
-               var="`git config --get pull.twohead`"
+       case "$real_parents" in
+       ?*"$LF"?*)
+               var="`git config --get pull.octopus`"
                if test -n "$var"
                then
                        use_strategies="$var"
                else
-                       use_strategies="$default_twohead_strategies"
+                       use_strategies="$default_octopus_strategies"
                fi ;;
        *)
-               var="`git config --get pull.octopus`"
+               var="`git config --get pull.twohead`"
                if test -n "$var"
                then
                        use_strategies="$var"
                else
-                       use_strategies="$default_octopus_strategies"
+                       use_strategies="$default_twohead_strategies"
                fi ;;
        esac
        ;;

@@ -339,87 +364,66 @@ do
        done
 done

-case "$#" in
-1)
-       common=$(git merge-base --all $head "$@")
-       ;;
-*)
-       common=$(git show-branch --merge-base $head "$@")
-       ;;
-esac
 echo "$head" >"$GIT_DIR/ORIG_HEAD"

-case "$fast_forward,$#,$common,$no_commit" in
-*,*,'',*)
-       # No common ancestors found. We need a real merge.
-       ;;
-*,1,"$1",*)
-       # If head can reach all the merge then we are up to date.
-       # but first the most common case of merging one remote.
-       finish_up_to_date "Already up-to-date."
-       exit 0
-       ;;
-allow,1,"$head",*)
-       # Again the most common case of merging one remote.
-       echo "Updating $(git rev-parse --short $head)..$(git rev-parse
--short $1)"
-       git update-index --refresh 2>/dev/null
-       msg="Fast forward"
-       if test -n "$have_message"
+if true
+then
+       if test $head = $ff_head -a "$common" = "$real_parents"
        then
-               msg="$msg (no commit created; -m option ignored)"
-       fi
-       new_head=$(git rev-parse --verify "$1^0") &&
-       git read-tree -v -m -u --exclude-per-directory=.gitignore

$head "$new_head" &&
-       finish "$new_head" "$msg" || exit
-       dropsave
-       exit 0
-       ;;
-*,1,?*"$LF"?*,*)
-       # We are not doing octopus and not fast forward.  Need a
-       # real merge.
-       ;;
-*,1,*,)
-       # We are not doing octopus, not fast forward, and have only
-       # one common.
-       git update-index --refresh 2>/dev/null
-       case "$allow_trivial_merge" in
-       t)
-               # See if it is really trivial.
-               git var GIT_COMMITTER_IDENT >/dev/null || exit
-               echo "Trying really trivial in-index merge..."
-               if git read-tree --trivial -m -u -v $common $head "$1" &&
-                  result_tree=$(git write-tree)
-               then
-                       echo "Wonderful."
-                       result_commit=$(
-                               printf '%s\n' "$merge_msg" |
-                               git commit-tree $result_tree -p HEAD -p "$1"
-                       ) || exit
-                       finish "$result_commit" "In-index merge"
-                       dropsave
-                       exit 0
-               fi
-               echo "Nope."
-       esac
-       ;;
-*)
-       # An octopus.  If we can reach all the remote we are up to date.
-       up_to_date=t
-       for remote
-       do
-               common_one=$(git merge-base --all $head $remote)
-               if test "$common_one" != "$remote"
+               finish_up_to_date "Already up-to-date."
+               exit 0
+       elif test $fast_forward != never -a $ff_head = "$real_parents"
+       then
+               echo "Updating $(git rev-parse --short $head)..$(git
rev-parse --short $ff_head)"
+               git update-index --refresh 2>/dev/null
+               msg="Fast forward"
+               if test -n "$have_message"
                then
-                       up_to_date=f
-                       break
+                       msg="$msg (no commit created; -m option ignored)"
                fi
-       done
-       if test "$up_to_date" = t
-       then
-               finish_up_to_date "Already up-to-date. Yeeah!"
+               new_head=$(git rev-parse --verify "$ff_head^0") &&
+               git read-tree -v -m -u
--exclude-per-directory=.gitignore $head "$new_head" &&
+               finish "$new_head" "$msg" || exit
+               dropsave
                exit 0
        fi
+fi
+
+case "$real_parents" in
+?*"$LF"?*)
+       # We have more than one parent
+       common=$(git show-branch --merge-base $head $real_parents)
        ;;
+*)
+       # We have exactly one parent
+       test "$common" != not_queried || common=$(git merge-base --all
$head $real_parents)
+       case "$common" in
+       ?*"$LF"?*)
+               # We are not doing octopus and not fast forward.  Need a
+               # real merge.
+               ;;
+       *)
+               git update-index --refresh 2>/dev/null
+               if test "$allow_trivial_merge" = t
+               then
+                       # See if it is really trivial.
+                       git var GIT_COMMITTER_IDENT >/dev/null || exit
+                       echo "Trying really trivial in-index merge..."
+                       if git read-tree --trivial -m -u -v $common
$head $real_parents &&
+                               result_tree=$(git write-tree)
+                       then
+                               echo "Wonderful."
+                               result_commit=$(
+                                       printf '%s\n' "$merge_msg" |
+                                       git commit-tree $result_tree
-p HEAD -p $real_parents
+                               ) || exit
+                               finish "$result_commit" "In-index merge"
+                               dropsave
+                               exit 0
+                       fi
+                       echo "Nope."
+               fi ;;
+       esac ;;
 esac

 # We are going to make a new commit.

@@ -460,7 +464,7 @@ do
     # Remember which strategy left the state in the working tree
     wt_strategy=$strategy

-    git-merge-$strategy $common -- "$head_arg" "$@"
+    git-merge-$strategy $common -- "$head_arg" $real_parents
     exit=$?
     if test "$no_commit" = t && test "$exit" = 0
     then

@@ -530,7 +534,7 @@ case "$best_strategy" in
        echo "Rewinding the tree to pristine..."
        restorestate
        echo "Using the $best_strategy to prepare resolving by hand."
-       git-merge-$best_strategy $common -- "$head_arg" "$@"
+       git-merge-$best_strategy $common -- "$head_arg" $real_parents
        ;;
 esac

-- 
Sverre Hvammen Johansen

"Sverre Hvammen Johansen" [off-list ref] writes:

for preparation of new feature:

A bit of nit-picking: I'd like to have full sentences in commit
message (in the commit description); continuing first line isn't the
best style, IMHO.  I'd write instead the following:

  Restructure git-merge.sh, adding find_real_parents() function,
  in preparation for new feature:
 

Not specified_parents, or arg_parents?

+#   common:       All common ancestors or not_queried
+#   ff_head:      Fast forward of head
+find_real_parents () {

Very nice.

+       real_parents=$(git rev-parse "$@")
+       real_parents=${real_parents#$LF}

Why this trick with adding $LF?

-- 
Jakub Narebski
Poland
ShadeHawk on #git

"Sverre Hvammen Johansen" [off-list ref] writes:

for preparation of new feature:

Full sentence, please?

"Fast forward of head"?  Puzzled, and sorry I cannot offer a better
rewrite for this one as I do not quite get what you are trying to say
here.

+find_real_parents () {
+       real_parents=$(git rev-parse "$@")
+       real_parents=${real_parents#$LF}

What is this "#$LF" for?  rev-parse begins its output with an empty line
and you want to strip it?

+       if test $# = 1
+       then
+               common=$(git merge-base --all $head "$@")
+               if test "$common" = $head
+               then
+                       ff_head=$1
+               else
+                       ff_head=$head
+               fi

So for a single-remote merge, merge-base is run and common is set.

This caught my attention before looking at 3/4 ;-).

-       # An octopus.  If we can reach all the remote we are up to date.
-       up_to_date=t
-       for remote
-       do
-               common_one=$(git merge-base --all $head $remote)
-               if test "$common_one" != "$remote"
                then
-                       up_to_date=f
-                       break
                fi
-       done
-       if test "$up_to_date" = t
-       then
-               finish_up_to_date "Already up-to-date. Yeeah!"
                exit 0
        fi

This optimization seems to have been lost, even though it was supposed to
be just code restructuring.

+fi
+
+case "$real_parents" in
+?*"$LF"?*)
+       # We have more than one parent
+       common=$(git show-branch --merge-base $head $real_parents)
        ;;
+*)
+       # We have exactly one parent
+       test "$common" != not_queried || common=$(git merge-base --all
$head $real_parents)

How can "not_queried" be possible here?