New issue
Advanced search Search tips

Issue 729967 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Jun 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: ----
Type: ----



Sign in to add a comment

git cl upload with TBR=xxx fails against repo which doesn't grant CR+1 access

Project Member Reported by tandrii@chromium.org, Jun 6 2017

Issue description

As of today [1], git cl upload with TBR uploads against 
  refs/for/refs/heads/master%l=Code-Review+1,m....
which is fine if I can CR+1 my own change,
but if I can't CR+1, upload fails (difference vs Rietveld) with a lot of output[2] which makes it hard to find the relevant bit for users[3]

So, perhaps git cl should detect this and retry without CR+1?
I do realize this isn't trivial in current git cl codebase :(



[1] https://chromium.googlesource.com/chromium/tools/depot_tools/+/0ffdf2de157ed4252983254913f8c0b5e231d99c

[2] $ git cl upload
...
Adding self-LGTM (Code-Review +1) because of TBRs.
remote: Processing changes: refs: 1, done            
To https://xxx.googlesource.com/repo
! [remote rejected] 901242fee49c19eee974ce30f04c05b58280b74f -> refs/for/refs/heads/master%l=Code-Review+1,m=Initial_upload,notify=NONE (internal server error: applying label "Code-Review": 1 is restricted)
error: failed to push some refs to 'https://xxx.googlesource.com/repo'

Error after CL description prompt -- saving description to <$HOME>/.git_cl_description_backup

Failed to create a change. Please examine output above for the reason of the failure.
Hint: run command below to diagnose common Git/Gerrit credential problems:
 git cl creds-check

[3] reported to me by 2 people, yet I personally know where to look and saw the problem immediately.
 
Labels: -Restrict-View-Google Milestone-Launch
Owner: aga...@chromium.org
Status: Assigned (was: Untriaged)
1) How in the world is that git-cl-comments CL responsible for anything to do with TBR? This failure mode is much older than that.

2) The right way to do this is to set CR+1 with an API call instead of doing it in the refspec. That way its failure can be isolated.
[1] was meant as current depot_tools revision at which I see this problem; TBH, i didn't notice it touched git cl, and I didn't mean to imply it caused the problem :) The problem is cause the by the same fingers typing this very message :D 
Status: Started (was: Assigned)
https://chromium-review.googlesource.com/527325
Project Member

Comment 4 by bugdroid1@chromium.org, Jun 8 2017

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

commit fd238086f1c3caa5bc4c11d3457f81d9a30f3ed7
Author: Aaron Gable <agable@chromium.org>
Date: Thu Jun 08 17:37:42 2017

Use API to set TBR code-review

This prevents TBR (self code-review) permissions from blocking
the CL from being uploaded at all. Instead, it will fully
upload, and then show a better error message after upload is
complete.

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

[modify] https://crrev.com/fd238086f1c3caa5bc4c11d3457f81d9a30f3ed7/tests/git_cl_test.py
[modify] https://crrev.com/fd238086f1c3caa5bc4c11d3457f81d9a30f3ed7/git_cl.py

Status: Fixed (was: Started)
Project Member

Comment 6 by bugdroid1@chromium.org, Jun 9 2017

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

commit 2c376aa21bef061e6529c1459eb161ce31140d7c
Author: Andrii Shyshkalov <tandrii@chromium.org>
Date: Fri Jun 09 13:47:25 2017

git cl: Fix incorrect notify parameter when setting TBR.

Follow up on https://chromium-review.googlesource.com/c/527325/

R=agable@chromium.org,rmistry@chromium.org

Bug:  chromium:729967 
Bug:  skia:6744 
Change-Id: I7443298797a7c95c8ca01624e3cbf08c95e04855
Reviewed-on: https://chromium-review.googlesource.com/529246
Reviewed-by: Ravi Mistry <rmistry@chromium.org>
Commit-Queue: Andrii Shyshkalov <tandrii@chromium.org>

[modify] https://crrev.com/2c376aa21bef061e6529c1459eb161ce31140d7c/tests/git_cl_test.py
[modify] https://crrev.com/2c376aa21bef061e6529c1459eb161ce31140d7c/git_cl.py

Sign in to add a comment