| Issue 925: | pre-receive and update hooks | |
| 21 people starred this issue and may be notified of changes. | Back to list |
Gerrit doesn't offer any way to trigger "pre-receive" or "update" hooks as documented here: http://www.kernel.org/pub/software/scm/git/docs/githooks.html#pre-receive JGit supports pre-receive hooks, via the org.eclipse.jgit.transport.PreReceiveHook interface. It's probably just a matter of doing something similar to https://review.source.android.com/13138 in order to automagically run a "pre-receive" / "update" scripts if they exist. Is anyone here opposed to having Gerrit automatically execute a "pre-receive" and/or "update" script, if such scripts exists in the "hooks" directory (like CGit would do)? The rational for this request is that we're now forcing all our users to push through Gerrit, but they no longer get some of the automated checks that used to reject commits early on. I'm not aware of any workaround for this issue. If this sounds reasonable, I'm willing to take a stab at implementing this feature request.
May 2, 2011
Project Member
#1
edwin.ke...@gmail.com
May 3, 2011
I'm aware of the patchset-created hook but it doesn't solve my problem. First of all, some people have the permission to push directly without a review. I still want to enforce my hook when they do this. Also, I want to *reject* commits that aren't accepted by my hook, so this hook needs to run before Gerrit does anything else.
May 3, 2011
I'm not adverse to hooks running, I just dislike the idea of forking a 20+ GiB Java heap to run a 15 line shell script. Its horribly expensive to do on the host, and depends on having a massive amount of swap, as well as basically running with memory constantly overcommitted. Which you can do on Linux, but not on Solaris. I had been hoping someone was going to open source a replacement for ProcessBuilder that basically used a smaller external JVM to perform the fork+exec, but I don't think that has materialized. If you provide support for the pre-receive and update hooks, please do it to the JGit project directly as implementations of the PreReceiveHook interface, then we can drag it into Gerrit and you can modify Gerrit to use this implementation in addition to its own logic in ReceiveCommits.
May 3, 2011
Why is "forking the Java heap" expensive? On Linux, ProcessBuilder will quickly end up in UNIXProcess. forkAndExec, which I assume will do what it pretends (fork() followed quickly by an execve()). The Solaris code path seems fairly similar. I guess the JVM will only touch a handful of COW pages in the child process, so the whole process should be fairly cheap. Am I missing something? I'll look at implementing support for the pre-receive and update hooks directly in JGit by providing two implementations of the PreReceiveHook interface.
Jun 24, 2011
On Solaris fork() means you need sufficient virtual memory reserved for the child process in case the child winds up touching every single page. Linux assumes the child might not do that and allows itself to overcommit its virtual memory, but then runs the risk of needing to kill a process via the OOM killer if it really is overcommitted. spawn() came about as a way to avoid this, but I don't know if the JVM is even using that yet. From comment 4, it sounds like no, its still using plain old fork.
Jun 24, 2011
Damn, you're right, this is somewhat covered here: http://developers.sun.com/solaris/articles/subprocess/subprocess.html#overcom I hope this isn't considered a blocker for this improvement though. The lack of integration with cgit's hooks is really problematic for people who used to rely on them and are switching over to Gerrit.
Sep 29, 2011
i find the whole discussion about fork() to be totally irrelevant to the subject: - the memory consumption is only momentary (until the exec()) and none of that memory is ever touched (give or take a few pages). consequently, the problem can be conveniently worked around by adding some excess swap space - it is to be expected that the jvm(s) will at some point use posix_spawn(), so the problem will go away entirely - the hooks would be no essential feature of gerrit. someone who needs them would choose a suitable platform - and, after all, there are other hooks already, so the resource consumption as such cannot be such a big deal
Jan 30, 2012
I have pushed the following change that provides the ability to run an update hook: https://gerrit-review.googlesource.com/#/c/31350/ While this change doesn't address the resource issues mention about it does provide the functionality described if you are prepared to take the resource hit.
Jan 30, 2012
Cool, thanks!
Dec 10, 2012
This change has now been merged into master so I think this issue can be closed?
Feb 26, 2013
I'm using gerrit 2.5.2 and I set up a ref-updated hook in review_site/hooks/ which always exit with non-zero status. But in the end, the commit is still pushed to the git repository. Could you let me what to do to make it work? My script: ==================== #!/bin/bash echo "$(date): rejected" >> /tmp/ref-updated-reject.log exit 1 ==================== And I can see the log file (/tmp/ref-updated-reject.log) exist with date listed in the file.
Feb 28, 2013
You want to use the "ref-update" hook. Not the "ref-updated" one. I also find it confusing, but they are two different hooks and just the first one is synchronous and can be used to reject a push, as you can learn from https://gerrit.googlesource.com/gerrit/+/master/Documentation/config-hooks.txt.
Mar 3, 2013
ref-update is not worked in gerrit version 2.5.2 and it now works in my owned build for version 2.6. Thanks for the help!
Sep 15, 2014
Another use case for rejecting review updates on push is that we'd like to write a hook that rejects reviews that are not following our defined workflow. In the case where we have reviews A->B->C->D that depend on eachother and there is one developer working on each of A, B, C, D respectively, then we need to ensure that developer D doesn't accidentally replace all the work that A->B->C are doing. We have a script that can do so for our particular case and we'd like to install this script on the server in a Gerrit hook, rather than rely on developers using the correct procedure locally before pushing. Doing it locally also introduces race conditions that we'd like to avoid. |
|
| ► Sign in to add a comment |