threads / patch / 13242

patch, 5 partsIntroduce fast forward option only

Subject: [PATCH 5/5] Introduce fast forward option only

## tl;dr

4 messages between Apr 24, 2008 and Apr 25, 2008. Diffs are folded; open one to read it.

replies: 3people: 3as markdown or json

Sverre Hvammen Johansen· Apr 24, 2008, 05:53 UTC · lore

This feature is needed for git integration with accurev. See the documentation for an explanation of this feature.

Signed-off-by: Sverre Hvammen Johansen <hvammen@gmail.com>
---
 Documentation/fast-forward-options.txt |    9 ++
 git-merge.sh                           |   12 +-
 git-pull.sh                            |    2 +-
 t/t7601-merge-ff-options.sh            |  214 ++++++++++++++++++++++++++++++++
 4 files changed, 231 insertions(+), 6 deletions(-)
Show changes to 4 files +231 −6

Documentation/fast-forward-options.txt, git-merge.sh, git-pull.sh, t/t7601-merge-ff-options.sh

diff --git a/Documentation/fast-forward-options.txt
b/Documentation/fast-forward-options.txt
index 95d0e6f..4445b0e 100644
--- a/Documentation/fast-forward-options.txt
+++ b/Documentation/fast-forward-options.txt
@@ -12,6 +12,10 @@ never::
        Generate a merge commit even if the merge resolves as a
        fast-forward.  This option is equivalent of '--no-ff'.

+only::
+       Only allow a fast-forward.  The merge will fail unless HEAD is
+       up to date or the merge resolves as a fast-forward.
+
 If your workflow is always to branch from the special branch
 ("master") when working on a topic and merge that back to "master", if
 you happen to have worked only on a single topic and the "master" was
@@ -42,3 +46,8 @@ The first merge of topicA or the only merge of
topicB would have
 resulted in a fast forward without '--ff=never'.  Topic A consist of
 those commits that can be reached from master^2 without passing
 through any of the first-parent ancestries of master.
+
+However, if the workflow require that the branch you are merging with
+is based on the current HEAD you can use "only fast forward" policy to
+enforce fast forward or a failure.  The last merge of topicA in
+the example above would have failed with '--ff=only'.
diff --git a/git-merge.sh b/git-merge.sh
index 7c70c56..68f627d 100755
--- a/git-merge.sh
+++ b/git-merge.sh
@@ -162,21 +162,21 @@ parse_config () {
                        no_commit=t ;;
                --ff)
                        case "$2" in
-                       allow|never)
+                       allow|never|only)
                                fast_forward=$2; shift ;;
                        -*)
                                fast_forward=allow ;;
                        *)
-                               die "Available fast-forward options
are: allow and newer" ;;
+                               die "Available fast-forward options
are: allow, newer, and only" ;;
                        esac
                        ;;
                --ff=*)
                        fast_forward=${1#--ff=}
                        case "$fast_forward" in
-                       allow|never)
+                       allow|never|only)
                                ;;
                        *)
-                               die "Available fast-forward options
are: allow and newer" ;;
+                               die "Available fast-forward options
are: allow, newer, and only" ;;
                        esac
                        ;;
                --no-ff)
@@ -203,7 +203,7 @@ parse_config () {
                shift
        done
        test "$fast_forward" = allow -o "$squash" = "" ||
-               die "You cannot combine --squash with --ff=never"
+               die "You cannot combine --squash with --ff=never or --ff=only."
        args_left=$#
 }

@@ -340,6 +340,8 @@ find_reduced_parents "$@"
 # ff_head may be included here or later in actual parents
 if test -n "$reduced_parents"
 then
+       test $fast_forward = only &&
+               die "--ff=only can not handle more than one real parent"
        test $head = $ff_head ||
                reduced_parents="$ff_head$LF$reduced_parents"
 fi
diff --git a/git-pull.sh b/git-pull.sh
index 2d7293a..5bc84a6 100755
--- a/git-pull.sh
+++ b/git-pull.sh
@@ -41,7 +41,7 @@ do
                no_ff=--ff ;;
        --no-ff)
                no_ff=--no-ff ;;
-       --ff=allow|--ff=never)
+       --ff=allow|--ff=only|--ff=never)
                no_ff=$1 ;;
        -s=*|--s=*|--st=*|--str=*|--stra=*|--strat=*|--strate=*|\
                --strateg=*|--strategy=*|\
diff --git a/t/t7601-merge-ff-options.sh b/t/t7601-merge-ff-options.sh
index 2749f4f..ea36549 100755
--- a/t/t7601-merge-ff-options.sh
+++ b/t/t7601-merge-ff-options.sh
@@ -646,4 +646,218 @@ test_expect_success 'merge c1 with new
repository (pull --ff=never)' '

 test_debug 'gitk --all'

+test_expect_success 'merge c0 with c1 (--ff=only overrides --no-ff)' '
+       git reset --hard c0 &&
+       git config branch.master.mergeoptions "--no-ff" &&
+       git merge --ff=only c1 &&
+       verify_merge file result.1 &&
+       verify_head $c1
+'
+
+test_debug 'gitk --all'
+
+test_expect_success 'merge c0 with c1 (--ff=only in config)' '
+       git reset --hard c0 &&
+       git config branch.master.mergeoptions "--ff=only" &&
+       git merge c1 &&
+       test_tick &&
+       verify_merge file result.1 &&
+       verify_head $c1
+'
+
+test_debug 'gitk --all'
+
+test_expect_success 'merge c1 with c0 (--ff=only in config)' '
+       git reset --hard c1 &&
+       git config branch.master.mergeoptions "--ff=only" &&
+       git merge c0 &&
+       verify_merge file result.1 &&
+       verify_head $c1
+'
+
+test_debug 'gitk --all'
+
+test_expect_success 'merge c1 with c2 (--ff=only in config)' '
+       git reset --hard c1 &&
+       test_tick &&
+       git config branch.master.mergeoptions "--ff=only" &&
+       test_must_fail git merge c2 &&
+       verify_merge file result.1 &&
+       verify_head $c1
+'
+
+test_debug 'gitk --all'
+
+test_expect_success 'merge c0 with c1 (--ff=only)' '
+       git reset --hard c0 &&
+       test_tick &&
+       git merge --ff=only c1 &&
+       verify_merge file result.1 &&
+       verify_head $c1
+'
+
+test_debug 'gitk --all'
+
+test_expect_success 'merge c1 with c0 (--ff=only)' '
+       git reset --hard c1 &&
+       test_tick &&
+       git merge --ff=only c0 &&
+       verify_merge file result.1 &&
+       verify_head $c1
+'
+
+test_debug 'gitk --all'
+
+test_expect_success 'merge c0 with c1 and c2 (--ff=only)' '
+       git reset --hard c0 &&
+       test_must_fail git merge --ff=only c1 c2 &&
+       verify_merge file result.0 &&
+       verify_head $c0
+'
+
+test_debug 'gitk --all'
+
+test_expect_success 'merge c1 with c0 (--ff=only)' '
+       git reset --hard c1 &&
+       test_tick &&
+       git merge --ff=only c0 &&
+       verify_merge file result.1 &&
+       verify_head $c1
+'
+
+test_debug 'gitk --all'
+
+test_expect_success 'merge c1 with c2 (--ff=only overrides --no-ff)' '
+       git reset --hard c1 &&
+       git config branch.master.mergeoptions "--no-ff" &&
+       test_tick &&
+       test_must_fail git merge c2 --ff=only &&
+       verify_merge file result.1 &&
+       verify_head $c1
+'
+
+test_debug 'gitk --all'
+
+test_expect_success 'merge c0 with c1 (--no-ff overrides --ff=only)' '
+       git reset --hard c0 &&
+       git config branch.master.mergeoptions "--ff=only" &&
+       test_tick &&
+       git merge --no-ff c1 &&
+       verify_merge file result.1 &&
+       verify_parents $c0 $c1
+'
+
+test_debug 'gitk --all'
+
+test_expect_success 'merge c1 with c2 (--ff owerrides --ff=only)' '
+       git reset --hard c1 &&
+       git config branch.master.mergeoptions "--ff=only" &&
+       test_tick &&
+       git merge --ff c2 &&
+       verify_merge file result.1-5 &&
+       verify_parents $c1 $c2
+'
+
+test_debug 'gitk --all'
+
+test_expect_success 'merge c1 with x0 (--squash combined with --ff=only)' '
+       git reset --hard c1 &&
+       git config branch.master.mergeoptions "" &&
+       test_tick &&
+       test_must_fail git merge x0 --squash --ff=only &&
+       verify_merge file result.1 &&
+       verify_head $c1
+'
+
+test_debug 'gitk --all'
+
+
+test_expect_success 'merge x0 with c1 (--squash combined with --ff=only)' '
+       git reset --hard x0 &&
+       git config branch.master.mergeoptions "" &&
+       test_tick &&
+       test_must_fail git merge c1 --squash --ff=only &&
+       verify_merge file result.1-5 &&
+       verify_head $x0
+'
+
+test_debug 'gitk --all'
+
+
+test_expect_success 'merge c1 with c2 (--squash combined with --ff=only)' '
+       git reset --hard c1 &&
+       git config branch.master.mergeoptions "" &&
+       test_tick &&
+       test_must_fail git merge c2 --squash --ff=only &&
+       verify_merge file result.1 &&
+       verify_head $c1
+'
+
+test_debug 'gitk --all'
+
+
+test_expect_success 'merge c1 with x0 (--no-commit combined with --ff=only)' '
+       git reset --hard c1 &&
+       git config branch.master.mergeoptions "" &&
+       test_tick &&
+       git merge x0 --no-commit --ff=only &&
+       verify_merge file result.1-5 &&
+       verify_head $x0
+'
+
+test_debug 'gitk --all'
+
+
+test_expect_success 'merge x0 with c1 (--no-commit combined with --ff=only)' '
+       git reset --hard x0 &&
+       git config branch.master.mergeoptions "" &&
+       test_tick &&
+       git merge c1 --no-commit --ff=only &&
+       verify_merge file result.1-5 &&
+       verify_head $x0
+'
+
+test_debug 'gitk --all'
+
+test_expect_success 'merge c1 with c2 (--no-commit combined with --ff=only)' '
+       git reset --hard c1 &&
+       git config branch.master.mergeoptions "" &&
+       test_tick &&
+       test_must_fail git merge c2 --no-commit --ff=only &&
+       verify_merge file result.1 &&
+       verify_head $c1
+'
+
+test_debug 'gitk --all'
+
+test_expect_success 'merge c1 with x1 (pull --ff=only)' '
+       git reset --hard c1 &&
+       test_tick &&
+       git pull --ff=only clone refs/heads/master &&
+       verify_merge file result.1-13 &&
+       verify_head $x1
+'
+
+test_debug 'gitk --all'
+
+test_expect_success 'merge x2 with x1 (pull --ff=only)' '
+       git reset --hard x2 &&
+       test_tick &&
+       test_must_fail git pull --ff=only clone refs/heads/master &&
+       verify_merge file result.5-13 &&
+       verify_head $x2
+'
+
+test_debug 'gitk --all'
+
+test_expect_success 'merge c1 with new repository (pull --ff=only)' '
+       git reset --hard c1 &&
+       test_tick &&
+       test_must_fail git pull --ff=only new refs/heads/master &&
+       verify_merge file result.1 &&
+       verify_head $c1
+'
+
+test_debug 'gitk --all'
+
 test_done

-- 
Sverre Hvammen Johansen
Jakub Narebski· Apr 25, 2008, 10:46 UTC · re: Sverre Hvammen Johansen · lore

Re: [PATCH 5/5] Introduce fast forward option only

"Sverre Hvammen Johansen" <hvammen@gmail.com> writes:
> This feature is needed for git integration with accurev.
> See the documentation for an explanation of this feature.

I'd rather have description of the change in the commit message itself. You can add "See the documentation for details", but the basic description, like:

   This commit introcudes fast forward option 'only'.  With --ff=only
   merge succeeds only if it resolves to fast-forward merge.
 
It would be nice to have described either in commit message, or in the
documentation, _why_ it is needed for integration with AccuRev,
i.e. what features of AccuRev or of integration requires this.
-- 
Jakub Narebski
Poland
ShadeHawk on #git
Junio C Hamano· Apr 25, 2008, 18:59 UTC · re: Jakub Narebski · lore

Re: [PATCH 5/5] Introduce fast forward option only

Jakub Narebski <jnareb@gmail.com> writes:
Show 15 quoted lines
> "Sverre Hvammen Johansen" <hvammen@gmail.com> writes:
>
>> This feature is needed for git integration with accurev.
>> See the documentation for an explanation of this feature.
>
> I'd rather have description of the change in the commit message
> itself.  You can add "See the documentation for details", but the
> basic description, like:
>
>    This commit introcudes fast forward option 'only'.  With --ff=only
>    merge succeeds only if it resolves to fast-forward merge.
>  
> It would be nice to have described either in commit message, or in the
> documentation, _why_ it is needed for integration with AccuRev,
> i.e. what features of AccuRev or of integration requires this.

Including this message, I agree with what Jakub said as comments to this series. Readers of "git log" 6 months from now should not have to suffer "See Documentation", and reviewers of this series right now should not have to guess why it is a good idea and is worth reviewing by actually reading the patch.

Sverre Hvammen Johansen· Apr 25, 2008, 21:00 UTC · re: Junio C Hamano · lore

Re: [PATCH 5/5] Introduce fast forward option only

It will probably take more than a week before I have time to look into this as I am out of town this weekend. I will then rebase the patches on top of 1.5.5.1, test them, and send them as attachments to one mail.

-- 
Sverre Hvammen Johansen

← back to recent threads