New issue
Advanced search Search tips

Issue 714720 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Apr 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug



Sign in to add a comment

git drover can't merge gerrit CL

Project Member Reported by sunn...@chromium.org, Apr 24 2017

Issue description

I set git config --local gerrit.host true to use gerrit by default. With that git cl drover prompts about bypassing CQ (rietveld does not) and then fails saying that git cl land --bypass-hooks failed. See output below:

$ git drover --branch 3071 --cherry-pick 40f1892ad93b2dc497141e351fddf32475b8cd62
Going to cherry-pick
"""
commit 40f1892ad93b2dc497141e351fddf32475b8cd62
Author: Sunny Sachanandani <sunnyps@chromium.org>
Date:   Fri Apr 21 13:02:57 2017 -0700

    cc: Remove slow BeginMainFrame dump.
    
    This is for M59 branch only. We'd like to keep the dump for canary but
    we need to land it on trunk, ship in one canary, merge to branch, and
    then revert on trunk.
    
    R=vmpstr
    BUG= 668892 , 622080, 711064
    
    Change-Id: I78031639da8ac09925e080b6af342e075909bafb
    
    CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
    
    Change-Id: I78031639da8ac09925e080b6af342e075909bafb
    Reviewed-on: https://chromium-review.googlesource.com/484703
    Commit-Queue: Sunny Sachanandani <sunnyps@chromium.org>
    Reviewed-by: Vladimir Levin <vmpstr@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#466444}
"""
to 3071. Continue (y/n)? y
Using 50% similarity for rename/copy detection. Override with --similarity.
Running presubmit upload checks ...

Presubmit checks passed.
 cc/trees/proxy_main.cc | 12 +-----------
 1 file changed, 1 insertion(+), 11 deletions(-)
serving on http://127.0.0.1:45424
remote: Processing changes: updated: 1, done            
remote: (W) No changes between prior commit a3f0f42 and new commit e4e2ed8        
remote:                                       
remote: Updated Changes:                      
remote:   https://chromium-review.googlesource.com/486159 cc: Remove slow BeginMainFrame dump.        
remote:                                       
To https://chromium.googlesource.com/chromium/src.git
 * [new branch]                e4e2ed8ce4bf52ff4736a7c239aa58bd9c5a75b1 -> refs/for/refs/branch-heads/3071%m=Initial_upload,notify=NONE,r=vmpstr                                            
                                              
About to land on 3071. Continue (y/n)? y      
Using 50% similarity for rename/copy detection. Override with --similarity.
                                              
It seems this repository has a Commit Queue, which can test and land changes for you. Are you sure you wish to bypass it?
Press Enter to bypass CQ, or Ctrl+C to abort  
Your Gerrit credentials might be misconfigured. Try: 
  git cl creds-check                          
Error: Command 'cl land --bypass-hooks' failed: Command '['git', 'cl', 'land', '--bypass-hooks']' returned non-zero exit status 1

$ git cl creds-check                 
git is already configured to use your .gitcookies from /usr/local/google/home/sunnyps/.gitcookies
Your .netrc and .gitcookies have credentials for these hosts:
                            Host                            User         Which file
================================        ========================        ===========
chromium-review.googlesource.com        git-sunnyps.chromium.org        .gitcookies
       chromium.googlesource.com        git-sunnyps.chromium.org        .gitcookies
 
Labels: Milestone-Launch
Owner: aga...@chromium.org
Status: Assigned (was: Untriaged)

Comment 2 by aga...@chromium.org, Apr 25 2017

Cc: tandrii@chromium.org
Status: Started (was: Assigned)
git-drover doesn't do anything with reviewers. It doesn't TBR them, it doesn't set Code-Review+1. This is because Rietveld was a terrible pile of junk that didn't actually enforce if you had LGTM. Like, at all.

Here's a CL to make git-drover properly set TBR so that CLs it uploads to gerrit will be self-approved and have the right folks on the R= line. Then the submission should succeed.

https://chromium-review.googlesource.com/486961

And here's a config change to chromium to let committers approve and submit changes to release branches.

https://chromium.googlesource.com/chromium/src/+/1fb19cfa078eaa4d3ce6e76841a09e90a8269fb6

Would be nice if the output of git cl land --bypass-hooks was shown if it failed.

Comment 4 by aga...@chromium.org, Apr 25 2017

It is -- "git cl land --bypass-hooks" is run in interactive mode, so all of the output is printed. The problem is that gerrit (and therefore git-cl) is often not very verbose about what exactly prevented you from doing the thing you wanted to do.
Project Member

Comment 5 by bugdroid1@chromium.org, Apr 27 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/tools/depot_tools/+/c7e84d041306082117c5760c10bf19891a35e88e

commit c7e84d041306082117c5760c10bf19891a35e88e
Author: Aaron Gable <agable@chromium.org>
Date: Thu Apr 27 21:51:36 2017

git-drover: TBR appropriate reviewers

When uploading to either Rietveld or Gerrit, this will
cause git-cl to add the appropriate reviewers to the
cherry-pick review, so they're aware the change is being
copied to the release branch.

When uploading to Gerrit, this will also cause git-cl to
set the Code-Review+1 bit, allowing the CL to be landed

R=tandrii@chromium.org

Bug:  714720 
Change-Id: Ie04e2657a91e4345796ac2200c0115fb18e460a1
Reviewed-on: https://chromium-review.googlesource.com/486961
Commit-Queue: Aaron Gable <agable@chromium.org>
Reviewed-by: Andrii Shyshkalov <tandrii@chromium.org>

[modify] https://crrev.com/c7e84d041306082117c5760c10bf19891a35e88e/git_drover.py
[modify] https://crrev.com/c7e84d041306082117c5760c10bf19891a35e88e/tests/git_drover_test.py

Project Member

Comment 6 by bugdroid1@chromium.org, Apr 27 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/tools/depot_tools/+/c7e84d041306082117c5760c10bf19891a35e88e

commit c7e84d041306082117c5760c10bf19891a35e88e
Author: Aaron Gable <agable@chromium.org>
Date: Thu Apr 27 21:51:36 2017

git-drover: TBR appropriate reviewers

When uploading to either Rietveld or Gerrit, this will
cause git-cl to add the appropriate reviewers to the
cherry-pick review, so they're aware the change is being
copied to the release branch.

When uploading to Gerrit, this will also cause git-cl to
set the Code-Review+1 bit, allowing the CL to be landed

R=tandrii@chromium.org

Bug:  714720 
Change-Id: Ie04e2657a91e4345796ac2200c0115fb18e460a1
Reviewed-on: https://chromium-review.googlesource.com/486961
Commit-Queue: Aaron Gable <agable@chromium.org>
Reviewed-by: Andrii Shyshkalov <tandrii@chromium.org>

[modify] https://crrev.com/c7e84d041306082117c5760c10bf19891a35e88e/git_drover.py
[modify] https://crrev.com/c7e84d041306082117c5760c10bf19891a35e88e/tests/git_drover_test.py

Comment 7 by aga...@chromium.org, Apr 27 2017

Status: Fixed (was: Started)
I've successfully used git-drover on gerrit to merge a CL: https://chromium-review.googlesource.com/c/489490/

Sign in to add a comment