Gerrit trivial rebase re-approval question

460 views
Skip to first unread message

timyao

unread,
Sep 29, 2010, 1:33:19 AM9/29/10
to Repo and Gerrit Discussion
Hi,
I have a question about the re-approval of gerrit commit. If the
project is set to "fast-forward-only" type of commit policy then a
rebase is very often if there are many developers working at same
time.

I found even with "Change-Id" kept, after the rebase the second time
submit still requires another approval. I'm not sure if Gerrit should
understand that this is a simple trivial rebase so it will merge the
re-submitted change into the mainline tree for you.

I googled a lot and only find on Gerrit 2.1.3 there seems to be a auto-
re-approve script available but I can not find where it is. I'm
wondering if this is being used or every one just take the re-submit/
re-approve path.

And what if the re-approve is delayed again? People need rebase/re-
sumbit/re-approve for the third time?

I guess a "cherry-pick" type of policy will make conflict much less if
the change is not made on same file. Is this correct?

-Tim

Rob Heittman

unread,
Sep 29, 2010, 10:57:07 AM9/29/10
to Repo and Gerrit Discussion
We use cherry-pick to get the extra information about testers and
reviewers into our commit logs. (Helps to spread the "git blame")
Due to changes in the same file, the need for trivial rebases comes up
pretty often here too, and can damage velocity more than I like.

I think the case that really annoys people here -- is when Gerrit
won't cherry-pick a change due to a path conflict, but git will
cherry-pick it onto HEAD just fine. It's easy enough to do just that
and git commit --amend, git push, but seems unneccessary and generates
extra spam for project followers. This is especially frequent on a C#
project we have under Gerrit, where the .csproj manifest is always
being touched to incorporate new files.

What I haven't been able to figure out is where/if Gerrit is detecting
and anticipating the path conflict before actually attempting the
cherry-pick operation, or if there is just a subtle difference between
JGit and C git in how cherry-picks are done.

timyao

unread,
Sep 29, 2010, 2:04:41 PM9/29/10
to Repo and Gerrit Discussion
Oh yes. I can imagine the problem caused by those mostly modified
manifest file like .csproj.

Back to my previous question. It turns out that the trivial_rebase.py
keeps the reviews from the previous approval before rebase. This is
very useful and I can see this works for me. But I'm still struggling
to find out if it's possible to avoid a re-approval for a trivial
rebase.

Mihai Rusu

unread,
Sep 29, 2010, 2:58:25 PM9/29/10
to timyao, Repo and Gerrit Discussion

This feature of Gerrit is one that annoys many of our users
(especially since they are coming from a system where once a patch has
been approved it can be updated and the approval carries on). They are
complaining about the additional time, about the additional steps
breaking their focus and workflow on other things, about the
additional mail spam which reduces the usefulness of the reviewers
mailing lists (where all change updates are being sent to). Because of
all that we even told them that if they just have a rebase update for
a patch that was already approved it is fine to approve it themselves
(since Gerrit allows that right now, which is a misfeature IMO and
there is a bug opened so when this will be fixed the workaround will
break).

--
Mihai Rusu

Nasser Grainawi

unread,
Sep 29, 2010, 5:19:25 PM9/29/10
to timyao, Repo and Gerrit Discussion
Let me know if you have questions about trivial_rebase.py. I know there are a couple bugs in it (namely it throws an error if you upload a merge commit because it can't compute a patch-id for it), but none that are serious.

As far as re-approval, re-applying all the reviews from the previous patchset is effectively a re-approval. What else did you need?

Nasser

> --
> To unsubscribe, email repo-discuss...@googlegroups.com
> More info at http://groups.google.com/group/repo-discuss?hl=en

Nasser Grainawi

unread,
Oct 5, 2010, 5:27:51 PM10/5/10
to Tim Yao, Repo and Gerrit Discussion
Use the --submit flag to the 'gerrit review' ssh command.

Nasser

On Oct 5, 2010, at 3:09 PM, Tim Yao wrote:

Now I'm very close to make it work. The approval flag is added by the script and I also changed a little bit to enable the "verified" flag. Now both flags are added back by the script. And the remaining thing is how to get the re-verified and re-approved change submitted into the mainline tree.

I'm wondering how to use gerrit approve command to submit a change. I understand now any user with "submit" rights can now do it from web site to submit the re-verified and re-approved change. But does the script can do the same thing also, or the user have to click the button explicitly?

-Tim


On Tue, Oct 5, 2010 at 5:40 AM, Nasser Grainawi <nas...@codeaurora.org> wrote:
Re-adding the list so these responses are archived.

Using any account other than Gerrit Code Review requires setting up an account in Gerrit for it. Your ssh command is failing because you aren't specifying the identity file to use. It should be ssh_host_dsa_key (or similar) from the gerrit/etc/ site directory.

Nasser

On Oct 4, 2010, at 4:40 PM, Tim Yao wrote:

Still not OK.
When I ran this command:
ssh -l 'Gerrit Code Review' -p 29418 localhost gerrit qsql

The error messages said "Permission denied (publickey)".

I'm wondering what is the magic of the 'Gerrit Code Review' account? Is this an internal user with private key associated with ssh_host_dsa_key?

How about use the account that the gerrit daemon is running as? I mean the [container.user] in gerrit config? I can create a normal gerrit account with this username then upload it's key in same way as normal users. Then I have to manually setup the access rule to allow this user to change review status, correct? Is the purpose of 'Gerrit Code Review' account is to avoid such trouble?

-Tim


On Fri, Oct 1, 2010 at 3:57 PM, Nasser Grainawi <nas...@codeaurora.org> wrote:
On 10/01/2010 04:05 PM, Tim Yao wrote:
I googled a little bit in order to find the right way to use the trivial
rebase script. And I found this link:
http://blob.inf.ed.ac.uk/sxw/2010/05/16/spotting-trivial-rebases-in-gerrit/

And it seems I got the same problem. The error log says "Host key
verification failed". I have not create the localhost user in gerrit so
I think that's the reason.

Actually I found your comment in that link. I'm wondering if there is
any update from you. Do I need create localhost user to make the script
work? Is it possible to specify a valid SSH user as opposed to fix that
as localhost in the script? Is this a common issue or I did something wrong?




You can change any reference to the localhost user to instead use the 'Gerrit Code Review' account (like it does when running the suexec command).

Nasser

--
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum


-- 

Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum



-- 
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum

Reply all
Reply to author
Forward
0 new messages