Recommended way to reject pushes based on syntax checks?

[Sebastian Schuberth]

Hi,

I'd like to reject pushes to Gerrit based on a few source code and / or commit message syntax checks. Examples for the checks I'm thinking about would be:

- Check the source code for proper UTF-8 encoding.
- Verify that class member variables are prefixed with "m_".
- Validate commit message (wrapping, typos).

It seems to me there basically are two different ways to implement such checks:

1) Using a re-update hook [1].
2) Using a Gerrit plugin like the commit-validator-sample [2].

Given the requirement to only do these checks for *some* projects hosted on our Gerrit server, what would be the recommended way to implement this? Also, is there more ways to implement this than the two mentioned above?

Thanks for any help.

[1] https://gerrit.it.here.com/Documentation/config-hooks.html#_ref_update
[2] https://gerrit-review.googlesource.com/#/admin/projects/plugins/commit-validator-sample

Regards,
Sebastian

[David Ostrovsky]

Am Mittwoch, 23. April 2014 11:33:49 UTC+2 schrieb Sebastian Schuberth:

Hi,

I'd like to reject pushes to Gerrit based on a few source code and / or commit message syntax checks. Examples for the checks I'm thinking about would be:

To reject pushes for source code checks i the wrong way.

Use CI for that. Check OpenStack's validations [1].

Commit message validation can be done on the client side by abusing Change-Id commit-msg hook.

Check this implementation for an example [2].

[1] https://review.openstack.org/89757 

[2] http://cgit.freedesktop.org/libreoffice/core/tree/.git-hooks/commit-msg

[Sebastian Schuberth]

On Wed, Apr 23, 2014 at 11:59 AM, David Ostrovsky
<david.o...@gmail.com> wrote:

>> I'd like to reject pushes to Gerrit based on a few source code and / or
>> commit message syntax checks. Examples for the checks I'm thinking about
>> would be:
>>
>
> To reject pushes for source code checks i the wrong way.
> Use CI for that. Check OpenStack's validations [1].

I don't like the idea of having changes that break the basic rules
even showing up in Gerrit, thus my wish to reject the push in the
first place. We're already using a CI bot for running unit /
functional tests and giving verify +1/-1 accordingly, but that's a
different thing.

> Commit message validation can be done on the client side by abusing
> Change-Id commit-msg hook.

Local commit hooks have to be installed for each user first, and they
can be easily deleted / skipped (either deliberately or not). I need a
more fool-proof (server-side) solution.

--
Sebastian Schuberth

[David Ostrovsky]

Am Mittwoch, 23. April 2014 12:21:20 UTC+2 schrieb Sebastian Schuberth:

On Wed, Apr 23, 2014 at 11:59 AM, David Ostrovsky
<david.o...@gmail.com> wrote:

>> I'd like to reject pushes to Gerrit based on a few source code and / or
>> commit message syntax checks. Examples for the checks I'm thinking about
>> would be:
>>
>
> To reject pushes for source code checks i the wrong way.
> Use CI for that. Check OpenStack's validations [1].

I don't like the idea of having changes that break the basic rules
even showing up in Gerrit, thus my wish to reject the push in the
first place.

I see your point. This is conceptually wrong:

New patch set can break sane change.

Responsibility of Gerrit (core and plugins) is not to perform verifications,

but provide means to do it. Your verifications tool chain needs to be updated

regularly, by updating CI Jobs definition. Last but not least: take away the load

from Gerrit master and slaves.

OpenStack project performs ca. 10.000 code check verifications per day.

Imagine what would happen to their Gerrit if these builds would be done on Gerrit machine.

> Commit message validation can be done on the client side by abusing
> Change-Id commit-msg hook.

Local commit hooks have to be installed for each user first

You are using Gerrit, right? So you have done this already, in

<your-git-repo>.git/hooks/commit-msg

[David Pursehouse]

On Wed, Apr 23, 2014 at 7:21 PM, Sebastian Schuberth <sschu...@gmail.com> wrote:

On Wed, Apr 23, 2014 at 11:59 AM, David Ostrovsky
<david.o...@gmail.com> wrote:

>> I'd like to reject pushes to Gerrit based on a few source code and / or
>> commit message syntax checks. Examples for the checks I'm thinking about
>> would be:
>>
>
> To reject pushes for source code checks i the wrong way.
> Use CI for that. Check OpenStack's validations [1].

I don't like the idea of having changes that break the basic rules
even showing up in Gerrit, thus my wish to reject the push in the
first place. We're already using a CI bot for running unit /
functional tests and giving verify +1/-1 accordingly, but that's a
different thing.

If you already have a CI bot running, you've already done most of the setup.  If you don't want to set "verified" for code style checks, just add a new label 'Code-Style' or something like that, and have the CI bot report that after running the checks.

[Luca Milanesio]

Hi Sebastian,
without suggesting or mandating any policy on best-practices, the server side scripting plugins would do the job for you :-)

See an overview of Server Side scripting plugins at:
http://www.slideshare.net/lucamilanesio/gerrit-code-review-how-to-script-a-plugin-with-scala-and-groovy

For downloading Gerrit master with Server Side scripting plugin support:
http://ci.gerritforge.com/job/Gerrit-master-scripting/

What you would need to implement is a commit validation hook using either a Groovy or Scala script expression.

HTH.

Luca.

> --
> --
> To unsubscribe, email repo-discuss...@googlegroups.com
> More info at http://groups.google.com/group/repo-discuss?hl=en
>
> ---
> You received this message because you are subscribed to the Google Groups "Repo and Gerrit Discussion" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to repo-discuss...@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.

[Sebastian Schuberth]

On Wed, Apr 23, 2014 at 2:49 PM, Luca Milanesio
<luca.mi...@gmail.com> wrote:

> See an overview of Server Side scripting plugins at:
> http://www.slideshare.net/lucamilanesio/gerrit-code-review-how-to-script-a-plugin-with-scala-and-groovy

Interesting, thanks!

--
Sebastian Schuberth

[Sebastian Schuberth]

On Wed, Apr 23, 2014 at 1:46 PM, David Ostrovsky
<david.o...@gmail.com> wrote:

> OpenStack project performs ca. 10.000 code check verifications per day.
> Imagine what would happen to their Gerrit if these builds would be done on
> Gerrit machine.

I do not intend to build anything on the Gerrit server as part of the
checks but just to perform some very simple parsing.

> You are using Gerrit, right? So you have done this already, in
> <your-git-repo>.git/hooks/commit-msg

True, so deploying additional checks in commit-msg would probably not
be a problem after all. But still users would be able to easily skip
the checks if done locally.

--
Sebastian Schuberth

[Bertram Karch]

Hi Sebastian,

what about the gerrit ref-update hook?

This is the only hook which can reject a pushed commit.

We want to try to use this hook to check uploaded files if they have the right code formating before they will be accepted for review.

Do you have taken a look at this hook?

Bertram Karch  

[Sebastian Schuberth]

On Thu, May 1, 2014 at 12:43 PM, Bertram Karch <bek...@googlemail.com> wrote:

> what about the gerrit ref-update hook?
> This is the only hook which can reject a pushed commit.
> We want to try to use this hook to check uploaded files if they have the
> right code formating before they will be accepted for review.
> Do you have taken a look at this hook?

In my initial post in this thread I was considering to use a
ref-update hook myself, but given how uncomfortable it is to install
it (see the other thread of mine at [1]) I did not yet try it out.
Using a plugin just seems more appealing to me, at least these can be
installed via Gerrit SSH commands.

[1] https://groups.google.com/d/msg/repo-discuss/XQygUqC1GoE/nUEfG5DluokJ

--
Sebastian Schuberth