5

So at work we're implementing a new, nice Git branching strategy - Awesome!

In order to preserve the new (and nice) structure of our repository, we would like all merges to be done with the --no-ff flag (and the --no-commit flag to allow for better merge commit messages). However, it seems that just asking everyone to remember it, is a bit unreliable. Is there any way to enforce that every developer must merge with the aforementioned flags?

As far as I can tell, it's not possible to check this with a hook (as git does not reliably store any information about the fast-forward). I know it's possible to set configuration on each developers machine (by running git config --global merge.ff no). If this is the solution, how do I make sure that every developer has this configuration set?

Snapcount
  • 73
  • 7

3 Answers3

3

I believe you can check this in pre-receive hook, on the server, but it's tricky to define just what is allowed, and it could make pushes more difficult for those doing them. Moreover, it only gives those who have done the wrong thing an error: it's up to them to fix it, which may be a bit complicated for some users.

Moreover, you (and developers) might not want to force --no-ff on certain merges: in particular, if you have no commits on branch X and you fetch new origin/X commits, you might want to fast-forward your X to origin/X. (On the other hand, you/they can always use git rebase for such cases.)

That said, let's see if we can define the correct behavior.

First, we may note that "fast forward" is actually a property of a label move, not of a merge. A label moves in fast-forward fashion when its previous commit is an ancestor of its new commit. (Git thus uses the term "fast forward merge" to refer to something that is in fact not a merge at all.) The default test that git uses for any branch push update is that the label update must be a fast-forward operation unless the force flag is set.

We don't want to reject label fast-forwarding since that's the normal case for extending a branch, with or without merges:

...--o--o--n--n--n   <-- br1

In this diagram we represent a proposed update (as seen in a pre-receive hook) with existing commits written as o, and new ones written as n. The label br1 (more precisely, refs/heads/br1) used to point to the tip-most o and now will point to the tip-most n.

In your pre-receive hook, what happens is that the repository has actually already been updated with new commits (merges or not), and git simply hands to you every reference-update-request, in the form of <old-hash, new-hash, name> tuples. In other words, given the compressed br1 update diagram above, we might rewrite this using upper and lower case (I can't do color, alas) with uppercase representing the incoming old-hash and new-hash values, giving:

...--o--O--n--n--N   <-- br1

If you reject the push, git winds up garbage-collecting the new commits, since you told it not to allow any reference updates (including branch names), so br1 winds up still pointing to commit O.

Now, merges in general are fine in your system, but what you want is to make sure that when br1 acquires a merge commit that will later be on br2, branch br2 will not move to include that merge commit directly. That is, this is OK:

...--o--O--n--n--N   <-- br1
...             /
...---o--O--n--N     <-- br2

and even this is OK (maybe—we might assume that you'll get a br2 update on a later push, and we will check that part then; we cannot do it yet because we have not gotten the br2 update):

...--o--O--n--n--N   <-- br1
...             /
...         n--n
...        /
...----o--o          <-- br2 (you get no update since br2 did not move)

but this is to be rejected:

...--o--O--n--n--N   <-- br1, br2
...             /
...---o--O--n--n

and so is this:

...--o--O--n--n--N     <-- br1
...             / \
...---o--O--n--n   N   <-- br2

On the other hand, this is OK (though we might want to constrain which parent we allow this to go through on br2; presumably in these diagrams, the lines leading straight to the left are all --first-parent links):

...--o--O--n--n--N     <-- br1
...             / \
...---o--O--n--n---N   <-- br2

Moreover, it's OK to get additional non-merge commits following a merge:

...--o--O--n--n--n--N   <-- br1
...             / \
...---o--O--n--n---N    <-- br2

(and likewise on br2). We must, however, check every merge as this is not OK:

...--o--O--n--n--n--n--n---N   <-- br1
...             / \     \ /
...---o--O--n--n   n--n--N     <-- br2

(here someone did git merge br2 while on br1, then did git merge br1 while on br2 getting a fast-forward, then made two commits on br2; they also made two commits on br1; then they merged br1 into br2 again, and then merged br2 into br1 as a --no-ff merge; then pushed both br1 and br2 in one git push).

So: what rule, precisely, should we enforce? We can, I think, make this easier and better by enforcing rules about --first-parent as we traverse merge commits. In particular, what we want is:

  • given a branch update (not a create or delete)
  • do a --first-parent traversal of old-hash..new-hash, in graph order (git rev-list --topo-order)
  • require that the earliest commit in the resulting list have, as its first parent, the old hash.

There are a number of ways to write this, and it's tempting to try to use --boundary, but this does not work right as the boundary commits shown for a merge include all of its parents, even when using --first-parent. So let's go for the most straightforward:

# Operation must be a fast-forward.  This ensures that
# $oldsha is an ancestor of (and thus related to) $newsha,
# and thus we are not discarding any commits.
if ! git merge-base --is-ancestor $oldsha $newsha; then
    ... reject as non-fast-forward
fi
edge=$(git rev-list --topo-order --first-parent $oldsha..$newsha | tail -1)
# If the rev-list is empty then $oldsha is not related to $newsha.
# However, we checked that first.  (The only other case where this
# can occur is if $oldsha equals $newsha, which is not an update,
# so we won't be running this code at all.)
#
# The listed edge commit may, however, be a root commit (have
# no parent).  We must reject this case as well as "parent is
# not $oldsha".  Fortunately that happens automatically since
# the empty string does not match a valid hash; we just need
# to be sure to quote "$parent".
parent=$(git rev-parse -q --verify $edge^)
if [ "$parent" = $oldsha ]; then
    ... update is OK
else
    ... reject as containing a bogus merge
fi

Note that this also rejects "foxtrot merges" since the first-parent rev-list does not lead back to the original hash.

(I have not tested any of this.)

Michael
  • 8,362
  • 6
  • 61
  • 88
torek
  • 448,244
  • 59
  • 642
  • 775
  • First of all: i'm sorry for the late response; i got caught up in easter :-) Secondly: Your answer does make a lot of sense at first glance. I did not think of the --first-parent approach. However, I don't see how we differentiate ordinary pushes to `br1`, and pushes which were fast-forwarded from a local branch. I tried your code on a few examples, and I could not get it to work. (As I understand it, it should be an update-hook, but for good measure I tried to implement it as a pre-receive hook as well, but to no avail). Am I just not understanding it correctly? – Snapcount Mar 29 '16 at 10:36
  • I was thinking pre-receive (which allows you to implement more global tests, of all labels to be updated) but the fragments above are intended to work in an update hook as well. For actual testing of real scenarios, though, you'd need to provide scripts to create those scenarios: git init, make commits on branches, make merges of various sorts, and attempt pushes. The script would need to create and set up the remote repository as well. (An example hook with tracing / logging code would help, too). – torek Mar 29 '16 at 16:25
0

Here i a sample hook code to do it:

#!/bin/sh

# for details see here, 
# http://git-scm.com/book/en/Customizing-Git-An-Example-Git-Enforced-Policy
# it seems that git on Windows doesn't support ruby, so use bash instead
# to function, put it into remote hook dir
# to disable, rename or delete file in remote hook dir

refname=$1
oldrev=$2
newrev=$3


# enforces fast-forward only pushes
check_fast_forward ()
{
  all_refs=`git rev-list ${oldrev}..${newrev} | wc  -l`
  single_parent_refs=`git rev-list ${oldrev}..${newrev} --max-parents=1 | wc  -l `
  if [ $all_refs -eq $single_parent_refs ]; then
    echo "This is the section for fast-forward commits ..."
    exit 0
  fi
}

check_fast_forward

Set it according to your needs:

-eq is equal to

if [ "$a" -eq "$b" ]

 

-ne is not equal to

if [ "$a" -ne "$b" ]
CodeWizard
  • 128,036
  • 21
  • 144
  • 167
  • That hook **enforces** fast-forward pushes - I would like to **prevent** fast-forwards (but only when merging) – Snapcount Mar 23 '16 at 08:13
  • Right now it is what you wanted. change the comparison flags according to your needs – CodeWizard Mar 23 '16 at 08:28
  • I may be wrong here, but I don't think this is a symmetric problem. Correct me if t'm wrong: With the new test, we check if all refs **don't** share a single parent. In other words, if I make a change to a single branch (i.e no merges are involved) the hook will reject the push. This would not be desirable for obvious reasons. – Snapcount Mar 23 '16 at 09:00
  • Okay, I've tested different configurations of your code. I could get it to reject all merges (regardless of the `--no-ff` flag), accept everything I could think to throw at it, or reject all fast-forwards (including regular pushes without merging). Am i just being stupid here? I appreciate your help :-) – Snapcount Mar 23 '16 at 09:36
0

Since just about any option would involve the team member doing an extra step as part of their git setup, wouldn't it be best to just have each individual set their global config to default to --no-ff?

The only ones using ff would be the ones explicitly stating it then, rather than the ones forgetting to use --no-ff.

Kindread
  • 926
  • 4
  • 12
  • I would prefer to enforce the policy somehow - it could involve setting local configuration, but I would like a more... controlled way of doing it, than just asking people nicely. Do you know of a way to "push" configuration to developers? – Snapcount Aug 17 '16 at 10:46