bad case example: https://chromium-review.googlesource.com/#/c/350116/
because of just 1 hour old checkout, CQ build failed for no reason other than git diff from gerrit's ref to origin/master reporting huge number of files, for which OWNER lgtm was missing.

This would be nice to have; my change didn't want to land, because I hadn't rebased, and on rietveld it would have either complained about a bad patch or just rebased for me.

martiniss@ any chance you have the cycles to take this on?

Doh, actually cc'ing him this time.

I actually think this will be a frequent problem because it makes presubmit green only for the most recently rebased patches.

I just ran into this and I agree this will be a frequent problem.

martiniss@, do you have cycles for this?

Actually, I’ll take this.

 Issue 605157  has been merged into this issue.

I think what should happen here is that CQ should issue an API call to gerrit to have _gerrit_ rebase the patchset before sending it out to 70 bots.  This way if a rebase fails, then it would happen immediately, rather than tie up 70 resources and happening later.

The upside of a gerrit patchset over a rietveld patch is that if you check out a gerrit patchset, it will always works.  I don't think it makes much sense for bot_update to perform a rebase after checking out a gerrit patchset

if CQ does that, that won't work for git-cl-try or "Choose trybots" functionality in gerrit

Yes, but

* git-cl-try: The tool could ask gerrit to rebase first (as a flag or by default), and then run the try
* Choose Trybot: This is within gerrit so gerrit could just rebase by default (or not, by config) before trying

My point is, I'm wary of designs which could cause a gerrit patch to "fail to apply" in bot_update.

Also what about the case of dependent patchsets, ie

A -- B -- C.
 \-- D -- HEAD

Say C is based on B.  So a "git rebase" of C will take B-C and rebase it on HEAD, which will fail because maybe A-B contains conflicting changes.

The current behavior is that patchset C will be try-able because it's a valid commit.  If we're not careful, this would break.

Making an API call to rebase in Gerrit would create another patchset, causing the developer’s local client to diverge. They would then have to download the latest. From a UX perspective that’s a non-starter for this approach.

tl;dr Gerrit has a API call to check rebase-ability [1] and I have bug for CQ to make use of it and abort immediately ( issue 622148 ). Of course, by the time trybot runs, patch rebase may still fail, but this is less likely to happen even for chromium commit frequency.


long version

Suppose we ask Gerrit to do so. It has problems of its own:
1. Rebasing in Gerrit creates a new patchset, so then tryjobs are sent with that new patchset. Then, suppose 1/30 jobs fails, and developer wants to retry. Do we rebase again and then re-schedule all 30 tryjobs for new patchset? I guess not, but then CQ has to be taught how to re-use results from previous patchsets that were "mere rebases"
2. UI for this would be crappy: there would a be patchset for almost every CQ run.
3. Local dev's checkout won't be magically rebased, confusing developers.
4. Gerrit stores all those patchsets as git objects. Why do we need all that garbage?
5. Rebase operation in general is quite expensive in Gerrit, particularly in Chromium repo. For this reason, ChromeOS infra advised me against chromium CQ calling rebase when 1 year ago. 


I hope I persuaded you that doing rebases in Gerrit isn't easier than doing this bug in bot_update.

Now a few other points raised here:

> This way if a rebase fails, then it would happen immediately,
> rather than tie up 70 resources and happening later.

But this would also cause more ever stored objects in Gerrit repo, swelling already huge chromium repo, while trybot's git repos can be treated as ephemeral storage.

> The current behavior is that patchset C will be try-able because
> it's a valid commit.  If we're not careful, this would break.

Yes, it would, but nobody is seriously relying on this, AFAIK.
Worst case 2 internal projects (this issue is public) that use bot_update && Gerrit will have red tryjobs on CLs that can't land anyway.


[1] https://gerrit-review.googlesource.com/Documentation/rest-api-changes.html#get-mergeable

>> This way if a rebase fails, then it would happen immediately,
>> rather than tie up 70 resources and happening later.

> But this would also cause more ever stored objects in Gerrit repo, swelling
> already huge chromium repo, while trybot's git repos can be treated as ephemeral
> storage.

I'll note that you didn't address the issue.  It'll still tie up machine resources.  And git objects are cheap, users don't check out CL branches, so I don't think stored objects are really an issue.


And as for dependent patchsets, there are many people (inc me, dan..) who rely on dependent patchsets trying correctly, and breaking this flow will be a blocker for any other projects to migrate to gerrit.  This is particularly popular in infra because you can continue working and trying new patchsets while the dependent ones are still under review.


And I guess I didn't actually explicitly mention another one of my concerns - When bot_update fails, I get bugs filed against me.  I kid you not I've gotten a handful of bugs in the past from people whose patch failed to apply and then filed a bug against me because the patch they uploaded was out of date (Despite the build containing large red letters saying "Patch failed to apply")

I found it refreshing that Gerrit patchsets had a way of guaranteeing that What-You-Upload-Is-What-You-Get.  The reason we didn't do this previously with rietveld was because rietveld obeyed SVN semantics.  Using the power of Git, we now have the power to allow users to specify _exactly_ how they want the checkout to look now.  I think it's silly to take that away.

OK, fine, forget about stored objects, that was the weakest of concerns. What about others I raised?


On your concern about tying machine resources: we are doing this already, and while I agree it's not the most efficient way, it's clearly not a blocker for us right now.


On dependent patches:
And I don't see a problem with dependent patchsets at all. My view:
If we have to CLs in gerrit, say A and B on top of A, then rebasing B is really simple:

C1 -> C2 -> C3 -> C4 == master
  \
    A
      \
        B == CL2

rebasing refs/CL2 should find C1 as common parent between master and CL2 and just make 
C1 -> ... _> C4 -> A -> B.
What did I miss?


bot_update failures leading to bugs -> CQ team gets them too, sadly. And now I get cc-ed or assigned to any git cl bug. So, I understand you, but IMO, that shouldn't block us from doing the right thing, whichever it is (rebasing in Gerrit or on bots or smth else). That said, I think that with Gerrit CQ refusing to try patches with merge conflicts, you'd get a lot fewer of these bugs.


I also like that gerrit patch is actually a proper tree object. And I think there must be eventually two modes in bot_update - one rebasing, as this bug asks, and another What-You-Upload-Is-What-You-Get.
I still think that we need rebasing one so that users to don't have to constantly rebase in fast-moving chromium repository.

I agree that rebasing in bot_update is probably the best way of maintaining the current workflow. However, I feel that the current workflow was defined in part due to technical limitations imposed by Rietveld. Gerrit opens new doors here, and I think it's premature to rule out taking advantage of these opportunities.

Propose:
1) "git cl try" does no rebasing, runs patch as uploaded.
2) "Dry Run" and CQ run have CQ submit an implicit "rebase" request to Gerrit prior to proceeding.
2a) If this creates a new patch set, the CQ job is run against that patch set.
3) Can "auto-rebase" option to "git cl try" if people want this.

The benefit here is that the user can now work with *exactly* the patch that was tried, we can avoid more mysterious patching/rebasing logic nested deep within our infrastructure, there is Gerrit lineage about the progression of the patch (rather than testing some magic derived revision), less bot-update logic, more visible failures, and the ability to avoid spawning tryjobs for a patch that is doomed to fail.

Let's at least consider this before writing a shim in bot_update to make the cool new thing work like the old thing.

(PS: The user- or commit-initiated explicit rebase is similar to how Google does CLs via snapshots and syncing internally, too).

Reasonable, but doesn't address my concern above: 

Suppose, we do as you propose in 2). Because rebasing in Gerrit creates a new patchset in chromium repo (due to frequency of commits), we are at 2a) => new patchset.
Then, suppose ~2 out of 30 jobs fails due to flake, as do ~10% of all CQ attempts today. Now, developer triggers CQ again. What happens?
If CQ calls rebase to Gerrit again, there is yet another patchset, the critical question is - which builders are triggered by CQ now? All? or just the ones that failed? We certainly don't have capacity atm to schedule all 30, but then it means implementing complicated CQ logic to re-use tryjob results across several patchsets, which is no doubt feasible, but complex.

And note that we still have to adjust UI to avoid confusing reviewers with all the intermediate revisions from each CQ Dry run[1]. Critique does that, yet it seems fair amount of work -> WDYT andybons@

[1] and also answering 1) of your proposal: git cl try has been switched to CQ Dry Run (https://groups.google.com/a/chromium.org/d/msg/infra-dev/L_PuQmSXvtE/M3Ywme6lAAAJ)

I understand the concerns and I want to ensure that we’re using Gerrit’s features to help make our lives in infra easier.

However, from what I understand of the proposals above, rebasing in bot_update is the only one that matches the current developer experience with Rietveld. Other proposals add new steps/change workflows.

I am happy to revisit this after the migration occurs, but for now I’m trying to minimize the amount that needs to change (on the developer side) in order for users to start using Gerrit.

So for now I think we should rebase in bot_update and revisit this after the migration.

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

commit f83fa37b3451a862df10e0a5f8a1265a28e75b1f
Author: Andrew Bonventre <andybons@chromium.org>
Date: Tue Aug 09 18:47:40 2016

Add flag to rebase before gclient sync when applying a Gerrit patch

BUG= 612417 

Change-Id: I695bb93afd5ccde2c34b0dfc78989248e9b923e9

[modify] https://crrev.com/f83fa37b3451a862df10e0a5f8a1265a28e75b1f/recipe_modules/bot_update/api.py
[add] https://crrev.com/f83fa37b3451a862df10e0a5f8a1265a28e75b1f/recipe_modules/bot_update/example.expected/gerrit_rebase_patch_ref.json
[modify] https://crrev.com/f83fa37b3451a862df10e0a5f8a1265a28e75b1f/recipe_modules/bot_update/example.py
[modify] https://crrev.com/f83fa37b3451a862df10e0a5f8a1265a28e75b1f/recipe_modules/bot_update/resources/bot_update.py

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/tools/build.git/+/1ef1717a1d167fa243e0db9f89b1f715ca90b251

commit 1ef1717a1d167fa243e0db9f89b1f715ca90b251
Author: recipe-roller <recipe-roller@chromium.org>
Date: Tue Aug 09 19:17:00 2016

Roll recipe dependencies (trivial).

This is an automated CL created by the recipe roller. This CL rolls recipe
changes from upstream projects (e.g. depot_tools) into downstream projects
(e.g. tools/build).

More info is at https://goo.gl/zkKdpD. Use https://goo.gl/noib3a to file a bug
(or complain)

depot_tools:
  https://crrev.com/f83fa37b3451a862df10e0a5f8a1265a28e75b1f Add flag to rebase before gclient sync when applying a Gerrit patch (andybons@chromium.org)

R=andybons@chromium.org
BUG= 612417 

TBR=martiniss@chromium.org,phajdan.jr@chromium.org

Review-Url: https://codereview.chromium.org/2225143004

[modify] https://crrev.com/1ef1717a1d167fa243e0db9f89b1f715ca90b251/infra/config/recipes.cfg

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/chrome/tools/build_limited/scripts/slave/+/1daf3a0f5a96ebf059eaba714800db4c71ec1327

commit 1daf3a0f5a96ebf059eaba714800db4c71ec1327
Author: recipe-roller <recipe-roller@chromium.org>
Date: Tue Aug 09 19:22:46 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/tools/build.git/+/52b73ce928346e5ff2de78658eed76bf048b1d45

commit 52b73ce928346e5ff2de78658eed76bf048b1d45
Author: tandrii <tandrii@chromium.org>
Date: Tue Aug 09 19:57:32 2016

Presubmit recipe: rebase Gerrit patches.

Make use of gerrit_rebase_patch_ref property in bot_update.
Depends on https://chromium-review.googlesource.com/c/365990.

BUG= 612417 
R=andybons@chromium.org

Review-Url: https://codereview.chromium.org/2224413002

[modify] https://crrev.com/52b73ce928346e5ff2de78658eed76bf048b1d45/scripts/slave/recipes/run_presubmit.expected/depot_tools_and_gerrit.json
[modify] https://crrev.com/52b73ce928346e5ff2de78658eed76bf048b1d45/scripts/slave/recipes/run_presubmit.expected/infra_with_runhooks_and_gerrit.json
[modify] https://crrev.com/52b73ce928346e5ff2de78658eed76bf048b1d45/scripts/slave/recipes/run_presubmit.py

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/tools/build.git/+/52b73ce928346e5ff2de78658eed76bf048b1d45

commit 52b73ce928346e5ff2de78658eed76bf048b1d45
Author: tandrii <tandrii@chromium.org>
Date: Tue Aug 09 19:57:32 2016

Presubmit recipe: rebase Gerrit patches.

Make use of gerrit_rebase_patch_ref property in bot_update.
Depends on https://chromium-review.googlesource.com/c/365990.

BUG= 612417 
R=andybons@chromium.org

Review-Url: https://codereview.chromium.org/2224413002

[modify] https://crrev.com/52b73ce928346e5ff2de78658eed76bf048b1d45/scripts/slave/recipes/run_presubmit.expected/depot_tools_and_gerrit.json
[modify] https://crrev.com/52b73ce928346e5ff2de78658eed76bf048b1d45/scripts/slave/recipes/run_presubmit.expected/infra_with_runhooks_and_gerrit.json
[modify] https://crrev.com/52b73ce928346e5ff2de78658eed76bf048b1d45/scripts/slave/recipes/run_presubmit.py

And here is the try of this: https://luci-milo.appspot.com/swarming/task/3089b7de5610f810

the CL hasn't been rebased for >1 week. Thanks to new flag, presubmit step correctly determines that there is only 1 file in a change, and performs the check accordingly.

The following revision refers to this bug:
  https://chromium.googlesource.com/infra/infra.git/+/b5c3c41b8d0a75e4ead063ca0b849c4552533be3

commit b5c3c41b8d0a75e4ead063ca0b849c4552533be3
Author: recipe-roller <recipe-roller@chromium.org>
Date: Tue Aug 09 20:46:52 2016

Roll recipe dependencies (trivial).

This is an automated CL created by the recipe roller. This CL rolls recipe
changes from upstream projects (e.g. depot_tools) into downstream projects
(e.g. tools/build).

More info is at https://goo.gl/zkKdpD. Use https://goo.gl/noib3a to file a bug
(or complain)

build:
  https://crrev.com/1ef1717a1d167fa243e0db9f89b1f715ca90b251 Roll recipe dependencies (trivial). (recipe-roller@chromium.org)
  https://crrev.com/1f32353ff4a7011bbded5898da32981437de16eb [Android] Specify --adb-path when printing perf test results. (jbudorick@chromium.org)
depot_tools:
  https://crrev.com/f83fa37b3451a862df10e0a5f8a1265a28e75b1f Add flag to rebase before gclient sync when applying a Gerrit patch (andybons@chromium.org)

R=phajdan.jr@chromium.org,jbudorick@chromium.org,andybons@chromium.org,martiniss@chromium.org,recipe-roller@chromium.org
BUG=635975, 612417 

TBR=martiniss@chromium.org,phajdan.jr@chromium.org

Review-Url: https://codereview.chromium.org/2225893005

[modify] https://crrev.com/b5c3c41b8d0a75e4ead063ca0b849c4552533be3/infra/config/recipes.cfg

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/chrome/tools/build_limited/scripts/slave/+/6f64ed9b128b7d44933d9e1dfacbe63532b17f83

commit 6f64ed9b128b7d44933d9e1dfacbe63532b17f83
Author: recipe-roller <recipe-roller@chromium.org>
Date: Tue Aug 09 21:21:32 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/infra/infra.git/+/a50d06ce59b6d926b60ce25bb1f5c131fed6bde2

commit a50d06ce59b6d926b60ce25bb1f5c131fed6bde2
Author: recipe-roller <recipe-roller@chromium.org>
Date: Tue Aug 09 21:29:21 2016

Roll recipe dependencies (trivial).

This is an automated CL created by the recipe roller. This CL rolls recipe
changes from upstream projects (e.g. depot_tools) into downstream projects
(e.g. tools/build).

More info is at https://goo.gl/zkKdpD. Use https://goo.gl/noib3a to file a bug
(or complain)

build:
  https://crrev.com/52b73ce928346e5ff2de78658eed76bf048b1d45 Presubmit recipe: rebase Gerrit patches. (tandrii@chromium.org)
  https://crrev.com/ed36f101999bd972cea1b5be8e8e663e4c882a13 [perf] Remove Large Profile Generator. (dtu@chromium.org)
  https://crrev.com/ed494a5ee803bdbb1b0ac7df24cd6d819efc53f5 WebRTC: Disable Linux32 Debug+Release bots for now. (kjellander@chromium.org)

R=tandrii@chromium.org,kjellander@chromium.org,dtu@chromium.org,andybons@chromium.org,phajdan.jr@chromium.org,ehmaldonado@chromium.org
BUG= 636080 , 612417 

TBR=martiniss@chromium.org,phajdan.jr@chromium.org

Review-Url: https://codereview.chromium.org/2225413004

[modify] https://crrev.com/a50d06ce59b6d926b60ce25bb1f5c131fed6bde2/infra/config/recipes.cfg

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5ee096441f5fb3aacd8a79121ba44cf256f0dbc9

commit 5ee096441f5fb3aacd8a79121ba44cf256f0dbc9
Author: recipe-roller <recipe-roller@chromium.org>
Date: Wed Aug 10 00:49:31 2016

Roll recipe dependencies (trivial).

This is an automated CL created by the recipe roller. This CL rolls recipe
changes from upstream projects (e.g. depot_tools) into downstream projects
(e.g. tools/build).

More info is at https://goo.gl/zkKdpD. Use https://goo.gl/noib3a to file a bug
(or complain)

build:
  https://crrev.com/1ef1717a1d167fa243e0db9f89b1f715ca90b251 Roll recipe dependencies (trivial). (recipe-roller@chromium.org)
  https://crrev.com/1f32353ff4a7011bbded5898da32981437de16eb [Android] Specify --adb-path when printing perf test results. (jbudorick@chromium.org)
  https://crrev.com/52b73ce928346e5ff2de78658eed76bf048b1d45 Presubmit recipe: rebase Gerrit patches. (tandrii@chromium.org)
  https://crrev.com/ed36f101999bd972cea1b5be8e8e663e4c882a13 [perf] Remove Large Profile Generator. (dtu@chromium.org)
  https://crrev.com/ed494a5ee803bdbb1b0ac7df24cd6d819efc53f5 WebRTC: Disable Linux32 Debug+Release bots for now. (kjellander@chromium.org)
  https://crrev.com/abb024b2505d48e69dd8ba049f7b619677640fe9 Move Flutter engine to src/flutter (abarth@chromium.org)
  https://crrev.com/4e6de3a742791006b6ca41aab6c1bd8868461ce8 Update the path to the travis directory (abarth@chromium.org)
depot_tools:
  https://crrev.com/f83fa37b3451a862df10e0a5f8a1265a28e75b1f Add flag to rebase before gclient sync when applying a Gerrit patch (andybons@chromium.org)

R=dtu@chromium.org,tandrii@chromium.org,kjellander@chromium.org,jbudorick@chromium.org,andybons@chromium.org,recipe-roller@chromium.org,martiniss@chromium.org,phajdan.jr@chromium.org,ehmaldonado@chromium.org,ianh@google.com,abarth@chromium.org
BUG=635975, 612417 , 636080 

TBR=martiniss@chromium.org,phajdan.jr@chromium.org

Review-Url: https://codereview.chromium.org/2223213004
Cr-Commit-Position: refs/heads/master@{#410903}

[modify] https://crrev.com/5ee096441f5fb3aacd8a79121ba44cf256f0dbc9/infra/config/recipes.cfg

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/411f3a717cba9dbf92413f6dfa59cd1bb6a6bfc4

commit 411f3a717cba9dbf92413f6dfa59cd1bb6a6bfc4
Author: recipe-roller <recipe-roller@chromium.org>
Date: Wed Aug 10 18:29:51 2016

Roll recipe dependencies (trivial).

This is an automated CL created by the recipe roller. This CL rolls recipe
changes from upstream projects (e.g. depot_tools) into downstream projects
(e.g. tools/build).

More info is at https://goo.gl/zkKdpD. Use https://goo.gl/noib3a to file a bug
(or complain)

build:
  https://crrev.com/1ef1717a1d167fa243e0db9f89b1f715ca90b251 Roll recipe dependencies (trivial). (recipe-roller@chromium.org)
  https://crrev.com/1f32353ff4a7011bbded5898da32981437de16eb [Android] Specify --adb-path when printing perf test results. (jbudorick@chromium.org)
  https://crrev.com/52b73ce928346e5ff2de78658eed76bf048b1d45 Presubmit recipe: rebase Gerrit patches. (tandrii@chromium.org)
  https://crrev.com/ed36f101999bd972cea1b5be8e8e663e4c882a13 [perf] Remove Large Profile Generator. (dtu@chromium.org)
  https://crrev.com/ed494a5ee803bdbb1b0ac7df24cd6d819efc53f5 WebRTC: Disable Linux32 Debug+Release bots for now. (kjellander@chromium.org)
  https://crrev.com/abb024b2505d48e69dd8ba049f7b619677640fe9 Move Flutter engine to src/flutter (abarth@chromium.org)
  https://crrev.com/4e6de3a742791006b6ca41aab6c1bd8868461ce8 Update the path to the travis directory (abarth@chromium.org)
  https://crrev.com/7746bb86402b08a137921c1a2abe07352d95bd74 Roll recipe dependencies (trivial). (recipe-roller@chromium.org)
  https://crrev.com/e6e74c9db3e74f8603acf4d21389439aa20b232c Roll recipe dependencies (trivial). (recipe-roller@chromium.org)
  https://crrev.com/2c78661f30da29ac3d08a6e26b2392ada22dbba4 Check if device disconnects every time the last test of a revision fails (zxiong@google.com)
  https://crrev.com/12b302b6ed29f15838a9145262d7913d19ae4aba Use iossim/test-without-building for Earlgrey tests. (huangml@chromium.org)
  https://crrev.com/a661277f9f72d06f313dba599b4e6a4a7dd9887f Remove the android_isolate_path option from LocalGTestTest. (jbudorick@chromium.org)
depot_tools:
  https://crrev.com/f83fa37b3451a862df10e0a5f8a1265a28e75b1f Add flag to rebase before gclient sync when applying a Gerrit patch (andybons@chromium.org)
  https://crrev.com/1dae95a13f123a16bc1d03bd2c587e770ce2e29f Require internal recipe tryjob (martiniss@chromium.org)
  https://crrev.com/fff0635af2a79778a23be93802f2cabd6c9aedc4 Roll recipe dependencies (trivial). (recipe-roller@chromium.org)
recipe_engine:
  https://crrev.com/87823726a3c8ef0016780ac1b752a758f2191a55 Require internal recipe tryjob (martiniss@chromium.org)

R=dtu@chromium.org,zxiong@google.com,tandrii@chromium.org,huangml@chromium.org,kjellander@chromium.org,jbudorick@chromium.org,andybons@chromium.org,recipe-roller@chromium.org,martiniss@chromium.org,phajdan.jr@chromium.org,ehmaldonado@chromium.org,ianh@google.com,abarth@chromium.org
BUG= 628801 , 633333 ,635975, chromium:635183 , 636080 , 612417 

TBR=martiniss@chromium.org,phajdan.jr@chromium.org

Review-Url: https://codereview.chromium.org/2228243002
Cr-Commit-Position: refs/heads/master@{#411089}

[modify] https://crrev.com/411f3a717cba9dbf92413f6dfa59cd1bb6a6bfc4/infra/config/recipes.cfg

This isn't fixed. The new parameter is effective, but it should be made default. And that means find all recipes that call bot_update.ensure_checkout() AND which rely on old behavior to explicitly pass gerrit_rebase_patch_ref=False. Then, land a change to depot_tools to actually set gerrit_rebase_patch_ref=True by default, and then help recipe roller with non-trivial rolls for all repos downstream.

And also, this change should be tested with dependent Gerrit changes.

Andrii asked me to paste the following error from Skia bots here:


===Rebasing===
===Running git checkout -b tmp/faa8df714c5545e9ba6b90c04af64341 (attempt #1)===
In directory: /b/work/skia
Switched to a new branch 'tmp/faa8df714c5545e9ba6b90c04af64341'
===Succeeded in 0.0 mins===

===Running git rebase 9df5a4c354de0c8f54983f33c792327d782d16f3 (attempt #1)===
In directory: /b/work/skia
First, rewinding head to replay your work on top of it...

*** Please tell me who you are.

Run

  git config --global user.email "you@example.com"
  git config --global user.name "Your Name"

to set your account's default identity.
Omit --global to set the identity only in this repository.

fatal: empty ident name (for <chrome-bot@skia-vm-070.c.skia-buildbots.google.com.internal>) not allowed
You need to set your committer info first
===Failed in 0.0 mins===

===backing off, sleeping for 4 secs===
===Running git rebase 9df5a4c354de0c8f54983f33c792327d782d16f3 (attempt #2)===
In directory: /b/work/skia
Current branch HEAD is up to date.
===Succeeded in 0.0 mins===



Complete logs are here: https://luci-milo.appspot.com/swarming/task/30d65d8329d07710

Also in the log above it seems that there is a failure in attempt #1 but then it does not complain in attempt #2 and silently passes. This looks like a bug.

 Issue 602906  has been merged into this issue.

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

commit f0b0378318a0bca451dc05303ba3c0971b249103
Author: andybons <andybons@chromium.org>
Date: Wed Sep 07 19:00:58 2016

Remove gerrit_rebase_patch_ref flag from bot_update API

BUG= 612417 

Review-Url: https://codereview.chromium.org/2317213002

[modify] https://crrev.com/f0b0378318a0bca451dc05303ba3c0971b249103/recipe_modules/bot_update/api.py

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

commit f0b0378318a0bca451dc05303ba3c0971b249103
Author: andybons <andybons@chromium.org>
Date: Wed Sep 07 19:00:58 2016

Remove gerrit_rebase_patch_ref flag from bot_update API

BUG= 612417 

Review-Url: https://codereview.chromium.org/2317213002

[modify] https://crrev.com/f0b0378318a0bca451dc05303ba3c0971b249103/recipe_modules/bot_update/api.py

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

commit f0b0378318a0bca451dc05303ba3c0971b249103
Author: andybons <andybons@chromium.org>
Date: Wed Sep 07 19:00:58 2016

Remove gerrit_rebase_patch_ref flag from bot_update API

BUG= 612417 

Review-Url: https://codereview.chromium.org/2317213002

[modify] https://crrev.com/f0b0378318a0bca451dc05303ba3c0971b249103/recipe_modules/bot_update/api.py

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/tools/build.git/+/ece649094b8b366432846b994819eca89d790cf9

commit ece649094b8b366432846b994819eca89d790cf9
Author: recipe-roller <recipe-roller@chromium.org>
Date: Wed Sep 07 19:15:57 2016

Roll recipe dependencies (trivial).

This is an automated CL created by the recipe roller. This CL rolls recipe
changes from upstream projects (e.g. depot_tools) into downstream projects
(e.g. tools/build).

More info is at https://goo.gl/zkKdpD. Use https://goo.gl/noib3a to file a bug
(or complain)

depot_tools:
  https://crrev.com/f0b0378318a0bca451dc05303ba3c0971b249103 Remove gerrit_rebase_patch_ref flag from bot_update API (andybons@chromium.org)

TBR=martiniss@chromium.org,phajdan.jr@chromium.org
BUG= 612417 

Recipe-Tryjob-Bypass-Reason: Autoroller
Bugdroid-Send-Email: False
Review-Url: https://codereview.chromium.org/2318293002

[modify] https://crrev.com/ece649094b8b366432846b994819eca89d790cf9/infra/config/recipes.cfg

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2b02064f475f73e96e5dcbb623cb6757d7c908db

commit 2b02064f475f73e96e5dcbb623cb6757d7c908db
Author: recipe-roller <recipe-roller@chromium.org>
Date: Wed Sep 07 19:38:40 2016

Roll recipe dependencies (trivial).

This is an automated CL created by the recipe roller. This CL rolls recipe
changes from upstream projects (e.g. depot_tools) into downstream projects
(e.g. tools/build).

More info is at https://goo.gl/zkKdpD. Use https://goo.gl/noib3a to file a bug
(or complain)

build:
  https://crrev.com/ece649094b8b366432846b994819eca89d790cf9 Roll recipe dependencies (trivial). (recipe-roller@chromium.org)
depot_tools:
  https://crrev.com/f0b0378318a0bca451dc05303ba3c0971b249103 Remove gerrit_rebase_patch_ref flag from bot_update API (andybons@chromium.org)

TBR=martiniss@chromium.org,phajdan.jr@chromium.org
BUG= 612417 

Recipe-Tryjob-Bypass-Reason: Autoroller
Bugdroid-Send-Email: False
Review-Url: https://codereview.chromium.org/2316313002
Cr-Commit-Position: refs/heads/master@{#417031}

[modify] https://crrev.com/2b02064f475f73e96e5dcbb623cb6757d7c908db/infra/config/recipes.cfg

The following revision refers to this bug:
  https://chromium.googlesource.com/infra/infra.git/+/7761fc03d8cb1d604f9a58af630194e7633228c4

commit 7761fc03d8cb1d604f9a58af630194e7633228c4
Author: recipe-roller <recipe-roller@chromium.org>
Date: Wed Sep 07 22:34:24 2016

Roll recipe dependencies (trivial).

This is an automated CL created by the recipe roller. This CL rolls recipe
changes from upstream projects (e.g. depot_tools) into downstream projects
(e.g. tools/build).

More info is at https://goo.gl/zkKdpD. Use https://goo.gl/noib3a to file a bug
(or complain)

build:
  https://crrev.com/8373aa1227eccc13db3b1c14e3699d4282b9ff1f Convert commandline to str in recipe_modules/goma (tikuta@chromium.org)
  https://crrev.com/191bb2bd43d119b86f82d72a34dd65c86f0caa55 Fix GYP-based closure try bots while they continue getting ported to GN (dbeam@chromium.org)
  https://crrev.com/11785fb7f116f51bb6442bbbe5b4096055448e57 Add counting the number of CPU's in goma module (tikuta@chromium.org)
  https://crrev.com/a29ebd495ab583c49bc8b0d1954c536e9eb48b10 goma canary; should fail when goma start failed. (yyanagisawa@chromium.org)
  https://crrev.com/383c08f181a0bd59c5c34628be1042c05a88727a WebRTC: Change Google Storage bucket for logcat dumps (kjellander@chromium.org)
  https://crrev.com/13856309f4dff0281657c5f282c5c99e55096947 chromium_android: fix dangerous default value (phajdan.jr@chromium.org)
  https://crrev.com/b9d0b44a84c7dc24fe9f69367fe8a05cfbb70d01 Delete android/perf recipe (phajdan.jr@chromium.org)
  https://crrev.com/cf533954eb5866ebdd425f81806d30264e79021c V8: Switch on MB on older branches (machenbach@chromium.org)
  https://crrev.com/1b0324b4e3dfad7455edaf416134848245331167 Convert Site Isolation Win layout tests to run with --site-per-process. (alexmos@chromium.org)
  https://crrev.com/378036a294f1ead296e09284dc83387dcacedd1f Revert of Fix GYP-based closure try bots while they continue getting ported to GN (patchset #6 id:100001 of https://codereview.chromium.org/2315123002/ ) (dbeam@chromium.org)
  https://crrev.com/f9c6f0966908f1d125ea8f519cf31b8a84aff886 DevTools: Update devtools recipe with tryserver logic (chenwilliam@chromium.org)
  https://crrev.com/ece649094b8b366432846b994819eca89d790cf9 Roll recipe dependencies (trivial). (recipe-roller@chromium.org)
  https://crrev.com/b6837ba5832ef4b25103a9e9680e7cd0c275ef41 [Android] Enable USB reset on perf bots. (jbudorick@chromium.org)
  https://crrev.com/239067c0b2b9d9f905d8a0925cd13439edead8ae [Android] Set up continuous builder + trybot for cronet. (jbudorick@chromium.org)
  https://crrev.com/8e6fcd316b9f3551ec6618e39adb516accd0b0f9 Re-add perf-id and results-url. (dtu@chromium.org)
depot_tools:
  https://crrev.com/f0b0378318a0bca451dc05303ba3c0971b249103 Remove gerrit_rebase_patch_ref flag from bot_update API (andybons@chromium.org)

TBR=martiniss@chromium.org,phajdan.jr@chromium.org
BUG=none,634350,chromium:616035,642046,644563,644721,477150,544330,637806,584079,417518,644370,612417,642194

Recipe-Tryjob-Bypass-Reason: Autoroller
Bugdroid-Send-Email: False
Review-Url: https://codereview.chromium.org/2319333002

[modify] https://crrev.com/7761fc03d8cb1d604f9a58af630194e7633228c4/infra/config/recipes.cfg

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/chrome/tools/build_limited/scripts/slave/+/d7cfc709a57d0b9a63d17ac8cc1dc3b580f3444a

commit d7cfc709a57d0b9a63d17ac8cc1dc3b580f3444a
Author: recipe-roller <recipe-roller@chromium.org>
Date: Thu Sep 08 03:24:31 2016