Rietveld CQ should complain and abort if target branch doesn't start from ref. |
||||||||
Issue description
What steps will reproduce the problem?
(1) Upload a CL with wrong --target_branch, for example: git cl upload --target_branch=branch_heads/3029 (Correct path would have been --target_branch=refs/branch-heads/3029)
(2) After review, check "Commit"
What is the expected result?
Error notification, preferably on initial upload, or if that's not feasible on commit.
What happens instead?
I got the message: "CQ experienced an internal error when committing your CL and the maintainers were notified. Sorry for the inconvenience." with no further details.
Trooper ddoman@ was helping me, and forwarded a Python stack backtrace that reported:
File "/b/infra_internal/commit_queue/checkouts.py", line 441, in get_git_numberer_state
assert target_ref.startswith('refs/')
While that made it clear what was going on, it would be helpful to report this via the crrev GUI or "git cl" command line to avoid having to involve the trooper in investigating it.
For reference, the CL where this happened was https://codereview.chromium.org/2775513002/ .
,
Mar 24 2017
,
Mar 24 2017
I don't usually directly assign bugs like this, but I suspect that tandrii@ would know how to fix it quickly. If not then feel free to re-triage appropriately :)
,
Mar 24 2017
+Robbie who is of course correct :)
,
Mar 24 2017
Actually, CQ send me alert about this as well, and I started
,
Mar 24 2017
I suspect this a combination of 2 things: 1. We deployed recently Rietveld version that stopped making everything refs/pending/xxx/yyy for Chromium and V8 2. CQ has a bug with git numberer state checking against branch-heads, which somehow wasn't triggered on WebRTC which never had this Rietveld support. But to be clear, the bug is in not in git cl upload. It is in CQ.
,
Mar 24 2017
,
Mar 24 2017
Hm, I take it back. The problem is that target_branch didn't start from ref. So, perhaps CQ should complain loudly about it instead.
,
Mar 24 2017
Fix: https://chrome-internal-review.googlesource.com/339644 which will make CQ complain loudly about this.
,
Mar 24 2017
The following revision refers to this bug: https://chrome-internal.googlesource.com/infra/infra_internal/+/37cb8cfce5e3be391fec9aec10840b81cf4e7439 commit 37cb8cfce5e3be391fec9aec10840b81cf4e7439 Author: Andrii Shyshkalov <tandrii@chromium.org> Date: Fri Mar 24 13:58:30 2017
,
Mar 24 2017
The following revision refers to this bug: https://chrome-internal.googlesource.com/infra/infra_internal/+/37cb8cfce5e3be391fec9aec10840b81cf4e7439 commit 37cb8cfce5e3be391fec9aec10840b81cf4e7439 Author: Andrii Shyshkalov <tandrii@chromium.org> Date: Fri Mar 24 13:58:30 2017
,
Mar 24 2017
Testing with whitespace: $ git clbp --target_branch=branch_heads/3029 Using 50% similarity for rename/copy detection. Override with --similarity. build/whitespace_file.txt | 2 ++ 1 file changed, 2 insertions(+) Upload server: https://codereview.chromium.org (change with -s/--server) Issue created. URL: https://codereview.chromium.org/2773953002 (patchset: 1) Uploading base file for build/whitespace_file.txt $ git cl set-commit -d $ git cl comments 2017-03-24 14:24:41 UTC tandrii@chromium.org The CQ bit was checked by tandrii@chromium.org to run a CQ dry run 2017-03-24 14:25:09 UTC commit-bot@chromium.org Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2773953002/1 2017-03-24 14:25:12 UTC commit-bot@chromium.org The CQ bit was unchecked by commit-bot@chromium.org 2017-03-24 14:25:12 UTC commit-bot@chromium.org Dry run: Your CL can not be processed by CQ because of: * target_ref must start with "refs/". Hint: re-upload your CL with proper --target_branch or omit --target_branch argument entirely |
||||||||
►
Sign in to add a comment |
||||||||
Comment 1 by klausw@chromium.org
, Mar 24 2017