Hey David,

I ran the following in each repository:

$ sudo git config hooks.denybadwhitespace true
$ sudo git config hooks.denymerge.master true
sudo ln -sfv /usr/share/git-core/custom-hooks/update hooks/update

Where /usr/share/git-core/custom-hooks/update contains some backported work that Jim Meyering did for the gnulib git repository¹.

Let me know if this doesn't work as expected in any way.

¹ http://lists.gnu.org/archive/html/bug-gnulib/2008-10/msg00221.html

When I try to push now I get the error:

{{{
*** Project description file hasn't been set
error: hooks/update exited with error code 1
error: hook declined to update refs/heads/master
To ssh://git.fedorahosted.org/git/deltacloud/core.git
! [remote rejected] master -> master (hook declined)
}}}

Can you change .git/description to

{{{
Deltacloud core API and drivers
}}}

(I don't think there's a way for me to do that remotely, or is there ?)

Unfortunately, I don't think that is something you can change remotely. I've updated the description file. Let me know if you still get errors when you push.

Something's not quite right with the hook. Somehow, merge commit 371034f9 got through even with the hook.

Yuck. I seems like if the merge commit isn't the last commit being pushed it slips through. I'm not sure why yet. I spent a little time trying to figure it out yesterday but didn't come up with any solution. I'll try to look into it more soon. If not, we can ping Jim Meyerying, as the author of the hooks. He might notice a problem -- perhaps I've not backported his work to the git on fedorahosted properly.

Replying to [comment:5 tmz]:

Yuck. I seems like if the merge commit isn't the last commit being pushed it slips through. I'm not sure why yet. I spent a little time trying to figure it out yesterday but didn't come up with any solution. I'll try to look into it more soon. If not, we can ping Jim Meyerying, as the author of the hooks. He might notice a problem -- perhaps I've not backported his work to the git on fedorahosted properly.

is this issue still running?

I've not found a fix, so I would assume so. Adding Jim Meyering to CC (Hi Jim, hope that's alright.) The update hook we have on hosted1 is at:

http://tmz.fedorapeople.org/tmp/update.hosted

Anyone notice anything broken about that (particularly as compared to the gnulib update hook from the URL in comment 1)? Testing on hosted1, rev-parse in git-1.5.5.6 does do the right thing when checking for merge commits, exiting 0 when one is present and 1 when not.

I think I've either missed something obvious in merging Jim's update hook changes into the git-1.5.5.6 hook or we're running into something that older git just doesn't do properly.

Hi David,

The script you referenced is out of date -- and buggy in that it failed to detect some types of merges. The latest is here:

http://git.et.redhat.com/?p=ovirt-server.git;a=blob_plain;f=git-hook/update;hb=refs/heads/vcs-admin

Jim, thanks for the update. Amusingly, I was just about to post a patch doing the 'for rev in ...' almost identical to your current hook. I'll merge this into the hook we deploy on hosted1. It should allow us to close this and #2013.

Thanks for the ping Xavier.

Jim, one issue I noticed that may well affect hosted repos more than it ever will on ovirt is that the update hook fails to deny merges when pushing into an empty repository. The rev-list command doesn't take kindly to a range like ^0000. :)

For us, we may well get a request to create a repository and enable the denymerge setting. If we did, the initial push would succeed, but spit out an error from rev-list like:

fatal: Invalid revision range 0000000000000000000000000000000000000000..15ec79a055d7a446fcebbb110347693a89ba38a8

It's perhaps better to catch this and at least let the user know that the denymerge setting was ineffective. Alternately, we could just /dev/null the error output from rev-list. I've done the former in the hook we'll deploy to hosted. A patch against the ovirt-server hook is here, if you're interested:

http://tmz.fedorapeople.org/patches/0001-update-hook-Avoid-errors-on-initial-push-in-denymerg.patch

If there is a way to get at the list of revisions being pushed, that would be even better. I just don't know what it is currently.

attachment
patch

Thanks Todd,
I've just attached the change you suggested.

I put it here, too:
http://git.et.redhat.com/?p=ovirt-server.git;a=commitdiff;h=a401269404d1cffd54cf78403a635b91a0ce0865

Jim,

Nice! I didn't think about the objects being added to the repository prior to the update hook being called. When I looked at your version, I was sure that it would fail on an empty repo. Actual testing proved me wrong and left me humbled and impressed. :)

I've rolled your excellent improvement into the hook on fedorahosted.org. Thanks again. I can now have a drink for the day, having learned something new and gained even more appreciation for empiricism. :)

Is this covered now, and can be closed?

Yes, this is all fine now.