Fix for Rietveld: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/668446

For Gerrit, I think we'll need to modify the recipe in bot_update to tell gerrit to patch at third_party/webrtc/ instead of src/

Aaron: Do you have any ideas?

OK, the reason it doesn't work with Gerrit is that it specifies 'src' as the patch_project [1], while Rietveld used to specify 'webrtc' [2]

[1] https://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_rel_ng/builds/546812
[2] https://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_rel_ng/builds/546827

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

commit 410907fe59ff83459e00f897bb6fef5a3274786a
Author: Edward Lemur <ehmaldonado@chromium.org>
Date: Fri Sep 15 18:39:34 2017

WebRTC: Remove hack in bot_update.py.

Bug:  765633 
Change-Id: I8f2a9ba807fbe176695a7d44467f3948eea07fda
Reviewed-on: https://chromium-review.googlesource.com/668446
Reviewed-by: Paweł Hajdan Jr. <phajdan.jr@chromium.org>
Reviewed-by: Henrik Kjellander <kjellander@chromium.org>
Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>

[modify] https://crrev.com/410907fe59ff83459e00f897bb6fef5a3274786a/recipes/recipe_modules/bot_update/resources/bot_update.py

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/infra/infra_internal/+/0fd5bf7a8419b64a96135a734e705b997be40de3

commit 0fd5bf7a8419b64a96135a734e705b997be40de3
Author: Andrii Shyshkalov <tandrii@chromium.org>
Date: Fri Sep 15 19:06:14 2017

So the full explanation is:
When you run "git cl try", or start the CQ, the CQ posts a blob of information to buildbucket
most of that info is just buildbucket params
Some of it is a build_properties blob
This build_properties blob then gets expanded by the bot, and becomes input to the recipe itself
One of the fields in that blob is "patch_project"
For Rietveld changes, this is the same as the "project" field. So "chromium", or "webrtc", or "infra"; in other words vague project names that are populated by codereview.settings
For Gerrit changes, this is the same as the path to the repository itself, so "chromium/src" or "v8/v8", or "src"
But in bot_update, there's a piece of logic which says "hey wait, if you're checking out chromium, but the patch project is X, then you should actually apply the patch in path Y instead of in the checkout root"
So the problem is that webrtc is the first repo to simultaneously satisfy two conditions:
1) Regularly run tryjobs on chromium trybots, without being chromium itself; and
2) Have a gerrit repo path which is not the same as its codereview.settings project name
tandrii's workaround above is to have the gerrit CQ pretend to be the rietveld CQ and forge a backwards-compatible patch_project field for webrtc changes
the long-term fix is to overhaul how bot_update decided which repo to patch (make it come from the change url, not the patch_project field) and then the hack can go away.

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/infra/infra_internal/+/3bcc905f25fef1cc13e9ea5c4465edc4037cc220

commit 3bcc905f25fef1cc13e9ea5c4465edc4037cc220
Author: Andrii Shyshkalov <tandrii@google.com>
Date: Fri Sep 15 23:15:47 2017

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/infra/infra_internal/+/07a7b4e52c38cd31ffa908a8fa1264a034e9bd89

commit 07a7b4e52c38cd31ffa908a8fa1264a034e9bd89
Author: Andrii Shyshkalov <tandrii@chromium.org>
Date: Sat Sep 16 00:23:18 2017

The hack in CQ has landed, so this should be working. However, we need a bug to remove this hack as a follow up, I'm here only in CQ maintainer capacity.
Back to agable@

Hmm, patching still fails, with an error like this.

First, rewinding head to replay your work on top of it...
Applying: Public launch
.git/rebase-apply/patch:95: new blank line at EOF.
+
warning: 1 line adds whitespace errors.
error: Failed to merge in the changes.
Using index info to reconstruct a base tree...
Falling back to patching base and 3-way merge...
Auto-merging DEPS
CONFLICT (add/add): Merge conflict in DEPS
Patch failed at 0001 Public launch
The copy of the patch that failed is found in: .git/rebase-apply/patch
When you have resolved this problem, run "git rebase --continue".
If you prefer to skip this patch, run "git rebase --skip" instead.
To check out the original branch and stop rebasing, run "git rebase --abort".

From https://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_rel_ng/builds/548350

I don't know why the DEPS file is involved here, the change is only in whitespace.txt (CL: https://webrtc-review.googlesource.com/c/src/+/1576)

This is getting urgent now. We need to have a way to try trybot patches on our CLs once we've migrated on Wednesday.

tl;dr I fixed CQ, not git cl try OR manual trigger from Gerrit UI.


For posterity, and also faster loading, here is logdog log[1] of bot_update step. Note that bot_update.py was invoked with 

... --gerrit_repo https://webrtc.googlesource.com/src --gerrit_ref refs/changes/76/1576/1 --revision src@HEAD

which shows that the patch is to be aplied against src, not anything inside it.
Relevant build properties:
  category	git_cl_try
  patch_project	src

So, I'd say WAI, since the hack I landed only applies to CQ-triggered builds.
On the other hand, if you do fix bot_update to work based on info fetched from Gerrit REST API, all triggerd tryjobs would work (because patch_project can be 100% ignored)

[1] https://luci-logdog.appspot.com/v/?s=chromium%2Fbb%2Ftryserver.chromium.linux%2Flinux_chromium_rel_ng%2F548350%2F%2B%2Frecipes%2Fsteps%2Fbot_update%2F0%2Fstdout

> On the other hand, if you do fix bot_update

From our conversation in chat on the 15th, I was under the impression that you would be doing the long-term fixes to bot_update as part of your CQ work. Is that now on us?

(I'm a strong believer that the solution here is for bot_update to be invoked with `--revision src@HEAD --revision src/third_party/webrtc@refs/changes/76/1576/1`. The in-recipe processing of patch-project (which is configured in recipe_modules/chromium/gclient_config.py) should be replaced to map the gerrit repo to a subpath instead. Basically a temporary hard-coding of DEPS paths for certain dependencies that we know want to run chromium tryjobs.)

> From our conversation in chat on the 15th, I was under the impression that you would be doing the long-term fixes to bot_update as part of your CQ work. Is that now on us?

Sorry for apparent confusion. I didn't mean to volunteer to anything beyond short term fix in CQ, hence my comment #10 above. I'm now the only maintainer of CQ backend and CQ isn't even related to my main projects (LUCI-scheduler & LUCI-migration), so I won't be tackling this issue.

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

commit a16ccaf11a5a0b145b166161b80d99eaef294f4b
Author: Aaron Gable <agable@chromium.org>
Date: Thu Sep 28 19:07:29 2017

bot_update: use patch repo instead of project if it is mapped

This is a step away from patch_project, which was a holdover
from Rietveld. Instead of mapping "projects" (which are
poorly-defined and not guaranteed to be unique) to subpaths,
instead map repository urls (which are at least unique).

Eventually we may be able to compute this directly from DEPS
instead of hardcoding the mapping here, but at least this is
a step in the right direction.

Bug:  765633 
Change-Id: Idd65984fc6edefcbedb0438d38c2338b10b7e8e5
Reviewed-on: https://chromium-review.googlesource.com/690776
Commit-Queue: Aaron Gable <agable@chromium.org>
Reviewed-by: Robbie Iannucci <iannucci@chromium.org>

[modify] https://crrev.com/a16ccaf11a5a0b145b166161b80d99eaef294f4b/recipes/README.recipes.md
[modify] https://crrev.com/a16ccaf11a5a0b145b166161b80d99eaef294f4b/recipes/recipe_modules/gclient/api.py
[modify] https://crrev.com/a16ccaf11a5a0b145b166161b80d99eaef294f4b/recipes/recipe_modules/gclient/tests/patch_project.py
[modify] https://crrev.com/a16ccaf11a5a0b145b166161b80d99eaef294f4b/recipes/recipe_modules/bot_update/examples/full.py
[modify] https://crrev.com/a16ccaf11a5a0b145b166161b80d99eaef294f4b/recipes/recipe_modules/gclient/config.py
[modify] https://crrev.com/a16ccaf11a5a0b145b166161b80d99eaef294f4b/recipes/recipe_modules/bot_update/api.py
[add] https://crrev.com/a16ccaf11a5a0b145b166161b80d99eaef294f4b/recipes/recipe_modules/bot_update/examples/full.expected/tryjob_gerrit_webrtc.json

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

commit dfb808eb3e80529802f51b2b270d1434faf3b95f
Author: Aaron Gable <agable@chromium.org>
Date: Thu Sep 28 19:44:25 2017

Map webrtc repo url to patch relative path

Depends on
https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/690776
(and the corresponding recipe deps roll) to be effective.

Bug:  765633 
Change-Id: Ibffa599f0267ade9a1a8af64e57dbb134da999ca
Reviewed-on: https://chromium-review.googlesource.com/690777
Reviewed-by: Robbie Iannucci <iannucci@chromium.org>
Commit-Queue: Aaron Gable <agable@chromium.org>

[modify] https://crrev.com/dfb808eb3e80529802f51b2b270d1434faf3b95f/scripts/slave/recipe_modules/chromium/gclient_config.py

We have a successful run!

https://luci-logdog.appspot.com/v/?s=chromium%2Fbb%2Ftryserver.chromium.linux%2Flinux_chromium_rel_ng%2F554583%2F%2B%2Frecipes%2Fsteps%2Fbot_update%2F0%2Fstdout

shows that the bot_update recipe module correctly passed --patch_root=src/third_party/webrtc to the bot_update script, which then correctly fetched the change ref in that subpath.

Splendid, mr. Gable. Well done! I will kick off a tryjob myself to check if it works for me.

The CQ hack is still in codebase, filed https://crbug.com/879358 to get rid of it.