Issue 3022: Pushing change with labels and submit option fails
Status:  New
Owner: ----
Cc:
Project Member Reported by david.pu...@sonymobile.com, Nov 19, 2014
************************************************************
***** NOTE: THIS BUG TRACKER IS FOR GERRIT CODE REVIEW *****
***** DO NOT SUBMIT BUGS FOR CHROME, ANDROID, INTERNAL *****
***** ISSUES WITH YOUR COMPANY'S GERRIT SETUP, ETC.    *****
***** THOSE ISSUE BELONG IN DIFFERENT ISSUE TRACKERS!  *****
************************************************************

Affected Version:

What steps will reproduce the problem?

git push origin HEAD:refs/for/master%l=Code-Review+1,l=Verified+1,submit

What is the expected output? What do you see instead?

Expected: the labels are applied and the change is then submitted.  If the change could not be submitted, it should at least be created and the labels applied.

Actual:

! [remote rejected] HEAD -> refs/for/master%l=Code-Review+2,l=Verified+1,submit (submit not allowed)

Please provide any additional information below.

I'm assuming it's because the project requires the labels to be set, but the submit is attempted before the labels are added.

Documentation says:

"On auto-merge of a change neither labels nor submit rules are checked"

So does that mean the behaviour is as expected?  If so it seems a bit strange.  I think a better implementation would be to apply any labels first and then attempt the submit.

Nov 19, 2014
Project Member #1 edwin.ke...@gmail.com
Do you have the submit permission assigned on refs/for/refs/heads/master?

[1] https://gerrit-review.googlesource.com/Documentation/user-upload.html#auto_merge
Nov 19, 2014
Project Member #2 david.pu...@sonymobile.com
Yes, submit permission is there.  I can submit changes through the UI.

Nov 19, 2014
Project Member #3 edwin.ke...@gmail.com
Being able to submit from the UI doesn't mean that you also can submit by push using the submit option. Both require the Submit permission on different refs. To submit from the UI you need Submit on refs/heads/* and for being able to submit by push using the submit option you need Submit on refs/for/refs/heads/* (not sure if refs/for/* would work too). This is intended. Push using the submit option ignores labels and the submit rules and hence you don't want to grant it with the "normal" submit permission. And yes, this behaviour is intended too.

Maybe the purpose of submit on push is not clear enough.
When you work with Gerrit you basically have 2 options.
1. Work with code-review and submit changes through the UI.
2. Don't work with code-review and push changes directly into the repository.
The later has the disadvantage that you can only do fast-forward pushes and you need to rebase whenever somebody else has pushed something.
The submit on push option provides a workflow in between and addressed the disadvantage of bypassing code-review: Don't work with code-review, push changes directly into the repository, but make use of the Gerrit submit strategy to automatically merge/rebase on server side. Only in case of conflicts manual local rebase is necessary.
So submit on push is really bypassing code-review and this is why its permission must be different from the normal submit in the UI.

Can you try again after assigning Submit on (litarally) "refs/for/refs/heads/master"?
Nov 19, 2014
Project Member #4 david.pu...@sonymobile.com
OK, now I can see that it is by design.

I think the documentation needs to be updated to clarify this.

Nov 19, 2014
Project Member #5 edwin.ke...@gmail.com
Yes, it's probably worth to explain it in the documentation in more details.
Nov 20, 2014
Project Member #6 David.Os...@gmail.com
> When you work with Gerrit you basically have 2 options.
> 1. Work with code-review and submit changes through the UI.
> 2. Don't work with code-review and push changes directly into the repository.

Actually you should have another option:

3. Work with code-review and submit changes during push of a change or new patch set.

There is a big different between workflow 2. und 3.

2. Given that the required ACL are set the following operation should succeed:

  $ git push origin HEAD:refs/for/master%submit

3. No additional ACL are required beside usual submit right through the UI:

  $ git push origin HEAD:refs/for/master%submit

should fail: VRFY+1 & CRVW+2 labels weren't set, whereas:

  $ git push origin HEAD:refs/for/master%l=Code-Review+1,l=Verified+1,submit

should just work.
Nov 20, 2014
Project Member #7 David.Os...@gmail.com
Typo in the last command, it must be Code-Review+2, obviously.