ref-update hook not triggered on inline edit

[Charly]

Hi all,

It seems that Gerrit hooks are not triggered when submitting a new patch set using the inline edit feature.

In our context, we rely on these hooks to do some quick source code validation (mostly code style and commit message checks) and rejecting commits that do not comply. Using inline edits allows a user to bypass this mechanism.

I believe there's currently no support to trigger the hooks for inline edits (and I understand that implementing the necessary bits to provide the hooks feedback/output from the WebUI is not trivial). Is there a way to disable inline edits altogether?

Best regards,

Charly

[David Pursehouse]

On Tue, Aug 16, 2016 at 11:38 PM Charly <jcd....@gmail.com> wrote:

Hi all,

It seems that Gerrit hooks are not triggered when submitting a new patch set using the inline edit feature.

As far as I know, this should work.  Which version of Gerrit are you using?

 

In our context, we rely on these hooks to do some quick source code validation (mostly code style and commit message checks) and rejecting commits that do not comply. Using inline edits allows a user to bypass this mechanism.

I believe there's currently no support to trigger the hooks for inline edits (and I understand that implementing the necessary bits to provide the hooks feedback/output from the WebUI is not trivial). Is there a way to disable inline edits altogether?

Best regards,

Charly

--
--
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.

[Charly]

It seems that Gerrit hooks are not triggered when submitting a new patch set using the inline edit feature.

As far as I know, this should work.  Which version of Gerrit are you using?

I should have started with that! I'm using 2.12.2 .

[Zaro]

We are using gerrit 2.11 and it works for us. Have you verified that
it works correctly? like did it work in a pervious version for you
and no longer works correctly on ver 2.12? Do you see anything in the
gerrit logs?

[Charly]

We are using gerrit 2.11 and it works for us.  Have you verified that
it works correctly?  like did it work in a pervious version for you
and no longer works correctly on ver 2.12?  Do you see anything in the
gerrit logs?

Hmm, I just tried again to double-check (although I did the very same thing before creating this post):

1. Created a review with an initial dummy patch set in a Python file

2. Attempted to submit a second patch set (that contains invalid code) from the command line using git-review triggered the hooks:

[2016-08-16 20:09:00,568] [SyncHook-1972] INFO  com.google.gerrit.common.ChangeHookRunner : hook[ref-update] exitValue:1

[2016-08-16 20:09:00,568] [SyncHook-1972] INFO  com.google.gerrit.common.ChangeHookRunner : hook[ref-update] output: *** pre-commit check failed for file `__init__.py' at commit: 41ab15e8c1e67b410b7d1247d4e4168da0eb6c93

[2016-08-16 20:09:00,568] [SyncHook-1972] INFO  com.google.gerrit.common.ChangeHookRunner : hook[ref-update] output: *** __init__.py:4:2: E201 whitespace after '{'

[2016-08-16 20:09:00,568] [SyncHook-1972] INFO  com.google.gerrit.common.ChangeHookRunner : hook[ref-update] output: *** __init__.py:4:17: E202 whitespace before '}'

[2016-08-16 20:09:00,568] [SyncHook-1972] INFO  com.google.gerrit.common.ChangeHookRunner : hook[ref-update] output: *** __init__.py:4:16: invalid syntax

[2016-08-16 20:09:00,568] [SyncHook-1972] INFO  com.google.gerrit.common.ChangeHookRunner : hook[ref-update] output: *** { invalid python }

[2016-08-16 20:09:00,568] [SyncHook-1972] INFO  com.google.gerrit.common.ChangeHookRunner : hook[ref-update] output: ***

3. Submitted the exact same change using the inline edit of the WebUI completed successfully and created the second patch set, although the code is invalid and is rejected by the ref-update hook; nothing in the logs

I don't recall trying this scenario on a previous version of Gerrit so I can't tell if it ever worked or not.

[Khai Do]

Sorry, I was mistaken.  I'm seeing the same thing on 2.11.   Hooks do not fire when updating or creating patchsets with inline edit.  I do not believe that it's possible to disable inline edit.  In fact i think there was an attempted to do this in the past but was rejected.  Anyways I suggest writing up a bug for this.  

[Khai Do]

This might help you: https://groups.google.com/d/msg/repo-discuss/N-i9Iplyr9o/5OKuEL5BFwAJ

[Charly]

Sorry, I was mistaken.  I'm seeing the same thing on 2.11.   Hooks do not fire when updating or creating patchsets with inline edit.  I do not believe that it's possible to disable inline edit.  In fact i think there was an attempted to do this in the past but was rejected.  Anyways I suggest writing up a bug for this.  

Thanks for confirming! Indeed there was a previous attempt to allow disabling inline edits (see https://gerrit-review.googlesource.com/#/c/71408/) which was abandoned on the basis that "it shouldn't matter how [one] created a patch set." to which I agree 100% as long as it doesn't allow users to bypass enforced checks.

I'll open a bug for this now that you confirmed it isn't a configuration issue on my side.

Thanks!

[Charly]

This might help you: https://groups.google.com/d/msg/repo-discuss/N-i9Iplyr9o/5OKuEL5BFwAJ

Thanks for the reference! However in the current situation we'd want to disable creating *and* updating patch sets with inline edits.

[Charly]

Sorry, I was mistaken.  I'm seeing the same thing on 2.11.   Hooks do not fire when updating or creating patchsets with inline edit.  I do not believe that it's possible to disable inline edit.  In fact i think there was an attempted to do this in the past but was rejected.  Anyways I suggest writing up a bug for this.  

Thanks for confirming! Indeed there was a previous attempt to allow disabling inline edits (see https://gerrit-review.googlesource.com/#/c/71408/) which was abandoned on the basis that "it shouldn't matter how [one] created a patch set." to which I agree 100% as long as it doesn't allow users to bypass enforced checks.

I'll open a bug for this now that you confirmed it isn't a configuration issue on my side.

Done: https://bugs.chromium.org/p/gerrit/issues/detail?id=4412 ;-)

[David Pursehouse]

I was unable to reproduce this issue.  Please see my comments in the ticket.

[Charly]

Sorry, I was mistaken.  I'm seeing the same thing on 2.11.   Hooks do not fire when updating or creating patchsets with inline edit.  I do not believe that it's possible to disable inline edit.  In fact i think there was an attempted to do this in the past but was rejected.  Anyways I suggest writing up a bug for this.  

Actually I was wrong: the ref-update hook is correctly fired, just not with the exact same refname:

* refs/publish/master when submitted with git-review

* refs/for/refs/heads/master when submitted via the WebUI after inline edits

This is why our check mechanism wasn't picking the changes, hence accepting them unconditionally. 

[Khai Do]

My apologies as well.   I checked again on ver 2.11 on a clean site install and found that the hooks do indeed fire when using inline edit.  My hooks file wasn't created properly on the last test i ran.  Thanks for the correction David.