A simpler, more robust approach to prefer rebase.

diff --git a/server/pre-receive-prefer-rebase b/server/pre-receive-prefer-rebase
index eee74e1583f08b07339aa81aca0b99e3cb2d8286..423f253b5d7c7ba60f81d71994a479b11d8f66cf 100644 (file)
--- a/server/pre-receive-prefer-rebase
+++ b/server/pre-receive-prefer-rebase
@@ -5,12 +5,6 @@ while read oldrev newrev refname ; do
                exit 0
        fi
 
-       # If the new ref is a merge, it'll have spaces in the parents log (ugly, yes)
-       git log -n 1 --pretty=format:%p $newrev | grep " "
-       if [ $? -ne 0 ] ; then
-               exit 0
-       fi
-
        # If they are introducing non-merge commits /and/ merge commits, it could
        # look like one of two ways. This way:
        #
@@ -19,7 +13,14 @@ while read oldrev newrev refname ; do
        #        new --- new --- newrev
        #
        # They basically had an un-shared local dev branch (probably by making a
-       # merge) and instead should have done a rebase.
+       # merge) and instead should have done a rebase. Also, if they did:
+       #
+       # * --- B --- * --- oldrev
+       #        \             \
+       #        new --- new --- new -- newrev
+       #
+       # We should try and catch them--where the merge happened previously to
+       # them doing more work in the newrev commit.
        #
        # But if it looks like:
        #
@@ -29,28 +30,37 @@ while read oldrev newrev refname ; do
        #
        # Then they had a pre-shared branch that cannot be rebased and so they
        # were correct in doing a merge to tie "old" and "oldrev" together.
+       #
+       # Also, we obviously have to be okay with:
+       #
+       # * --- * --- * --- oldrev --- new --- new --- newrev
 
-       # Find the original branch point (B)
-       baserev=$(git merge-base $oldrev $newrev)
-
-       # Before newrev is trying to be pushed, what was the old leg?
-       base_to_old=$(git rev-list $baserev..$oldrev | wc -l)
-
-       # We can now guess the new log by finding both legs old..new...
-       base_to_new=$(git rev-list $baserev..$newrev | wc -l)
-       # And then substract out the old leg
-       new_leg=$(expr $base_to_new - $base_to_old)
-
-       # See post-receive-email for an explanation of this--all new commits
-       all_new=$(git rev-parse --not --branches | grep -v $(git rev-parse $refname) | git rev-list --stdin $oldrev..$newrev | wc -l)
+       git rev-parse --not --branches | grep -v $(git rev-parse $refname) | git rev-list --stdin $oldrev..$newrev | while read commit ; do
+               number_of_parents=$(git rev-list -n 1 --parents $commit | sed 's/ /\n/g' | grep -v $commit | wc -l)
+               if [[ $number_of_parents > 1 ]] ; then
+                       # Find the original branch point (B)
+                       parents=$(git rev-list -n 1 --parents $commit | sed 's/ /\n/g' | grep -v $commit)
+                       baserev=$(git merge-base $parents)
 
-       # If all of their new commits on on their new lew, they didn't have any shared, they should have rebased
-       if [ $all_new -eq $new_leg ] ; then
-               echo "----------------------------------------------------"
-               echo
-               echo "It looks like you should rebase"
-               echo
-               echo "----------------------------------------------------"
+                       # For each parent
+                       git rev-list -n 1 --parents $commit | sed 's/ /\n/g' | grep -v $commit | while read parent ; do
+                               all_commits=$(git rev-list $baserev..$parent | wc -l)
+                               new_commits=$(git rev-parse --not --branches | git rev-list --stdin $baserev..$parent | wc -l)
+                               if [[ $all_commits -eq $new_commits ]] ; then
+                                       echo "----------------------------------------------------"
+                                       echo
+                                       echo "It looks like you should rebase instead of merging $commit"
+                                       echo
+                                       echo "----------------------------------------------------"
+                                       exit 1
+                               fi
+                       done
+                       if [ $? -ne 0 ] ; then
+                               exit 1
+                       fi
+               fi
+       done
+       if [ $? -ne 0 ] ; then
                exit 1
        fi
 done

diff --git a/tests/t2300-server-pre-receive-prefer-rebase.sh b/tests/t2300-server-pre-receive-prefer-rebase.sh
index 8fd15b40e841ac89975025067d0188f0bd6c67c8..e13d15594cbdcb27ba2bfa5575b28906d4382238 100644 (file)
--- a/tests/t2300-server-pre-receive-prefer-rebase.sh
+++ b/tests/t2300-server-pre-receive-prefer-rebase.sh
@@ -35,9 +35,38 @@ test_expect_success 'all local changes do not need a merge' '
        # go back to our client and it will merge in our changes
        cd .. &&
        git pull &&
+       merge=$(git rev-parse HEAD) &&
 
        ! git push 2>push.err &&
-       cat push.err | grep "It looks like you should rebase" &&
+       cat push.err | grep "It looks like you should rebase instead of merging $merge" &&
+       git reset --hard origin/master
+'
+
+test_expect_success 'all local changes do not need a merge even with more commits after' '
+       # server is on "setup"
+
+       # make an outstanding change for us--but do not push
+       echo "$test_name" >a.client1 &&
+       git add a.client1 &&
+       git commit -m "$test_name on client1" &&
+
+       # have another client commit (in this case, it is the server, but close enough)
+       cd server &&
+       echo "$test_name" >a.client2 &&
+       git add a.client2 &&
+       git commit -m "$test_name on client2" &&
+
+       # go back to our client and it will merge in our changes
+       cd .. &&
+       git pull &&
+       merge=$(git rev-parse HEAD) &&
+
+       # To complicate things, have them add another change
+       echo "$test_name again" >a.client1 &&
+       git commit -a -m "$test_name on client1 again" &&
+
+       ! git push 2>push.err &&
+       cat push.err | grep "It looks like you should rebase instead of merging $merge" &&
        git reset --hard origin/master
 '