New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 767284 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Sep 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

gerrit: deps walking fails when it hits a commit pushed by bots

Project Member Reported by ayatane@chromium.org, Sep 21 2017

Issue description

$ gerrit --raw deps 577067
gerrit: Unhandled exception:
Traceback (most recent call last):
  File "/usr/local/google/home/ayatane/src/chromiumos/chromite/bin/gerrit", line 168, in <module>
    DoMain()
  File "/usr/local/google/home/ayatane/src/chromiumos/chromite/bin/gerrit", line 164, in DoMain
    commandline.ScriptWrapperMain(FindTarget)
  File "/usr/local/google/home/ayatane/src/chromiumos/chromite/lib/commandline.py", line 910, in ScriptWrapperMain
    ret = target(argv[1:])
  File "/usr/local/google/home/ayatane/src/chromiumos/chromite/scripts/gerrit.py", line 562, in main
    functor(opts, *args)
  File "/usr/local/google/home/ayatane/src/chromiumos/chromite/scripts/gerrit.py", line 316, in UserActDeps
    visited_key=lambda cl: cl.gerrit_number)
  File "/usr/local/google/home/ayatane/src/chromiumos/chromite/scripts/gerrit.py", line 282, in _BreadthFirstSearch
    for child in children(node):
  File "/usr/local/google/home/ayatane/src/chromiumos/chromite/scripts/gerrit.py", line 310, in _Children
    change = _QueryChange(dep.ToGerritQueryText(), helper=helper)[-1]
IndexError: list index out of range
 

Comment 1 by vapier@chromium.org, Sep 21 2017

Summary: gerrit: deps walking fails when it hits a commit pushed by bots (was: IndexError in gerrit.py)
looks like what i triaged before

CL:577067 -> CL:676526 -> CL:676525 -> 8fa769e2bbeb194d5c02fd772e2c8616b4a2aaf7 -> bot commit -> crash

GerritDependencies for 676525 returns:
'dependsOn': [{'revision': '8fa769e2bbeb194d5c02fd772e2c8616b4a2aaf7'}]

so i guess the answer is that, for GerritDependencies that return no results, we have to assume it was a direct push that didn't go through Gerrit and ignore it.

Comment 2 by xixuan@chromium.org, Sep 21 2017

Labels: Hotlist-Fixit
Does this appear on CQ? or I will put it in fix-it. Feel free to raise the priority or move to other bug list if it's more serious than I thought. 

Comment 3 by vapier@chromium.org, Sep 21 2017

afaik it only shows up on local developer systems.  fix is heading to the CQ now.
Project Member

Comment 4 by bugdroid1@chromium.org, Sep 22 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/chromite/+/5726da903fc3014a86cbbb5c3c54efbd75ee1429

commit 5726da903fc3014a86cbbb5c3c54efbd75ee1429
Author: Mike Frysinger <vapier@chromium.org>
Date: Fri Sep 22 02:38:27 2017

gerrit: deps: handle edge cases better

When looking up the gerrit dependencies, we might run into a CL that is
based on a commit that did not go through Gerrit (e.g. a bot push).  In
that case, Gerrit won't know about the change, so searches for it will
return no results, and our indexing subsequently fails.  Ignore missing
results for Gerrit deps, but display an error for user specified ones.

Similarly, do not assume that the last result of the query is exactly
the one we want.  If the CQ-DEPEND is a Gerrit Change-Id, that could be
the same across different repos/branches.  We simply have to match them
all since there's no way to disambiguate.

BUG= chromium:767284 
TEST=`gerrit deps 676525` no longer fails

Change-Id: Id1212e402137fdbd076c83ef26a8e5a551d92bdd
Reviewed-on: https://chromium-review.googlesource.com/676177
Commit-Ready: Mike Frysinger <vapier@chromium.org>
Tested-by: Mike Frysinger <vapier@chromium.org>
Reviewed-by: Paul Hobbs <phobbs@google.com>

[modify] https://crrev.com/5726da903fc3014a86cbbb5c3c54efbd75ee1429/scripts/gerrit.py

Comment 5 by vapier@chromium.org, Sep 22 2017

Owner: vapier@chromium.org
Status: Fixed (was: Untriaged)

Comment 6 by vapier@chromium.org, Sep 26 2017

Cc: pho...@chromium.org
 Issue 746498  has been merged into this issue.

Sign in to add a comment