Actually, RPC post upload would also work with Gerrit - basically doing the same as "git cl desc" does already. It's substantially slower and is outright inefficient anyway.

I got PostUploadHooks working for Gerrit issues in Skia's PRESUBMIT.py :
https://skia.googlesource.com/skia/+/master/PRESUBMIT.py#460

Getting and updating the description is pretty straightforward: You just use GetDescription and UpdateDescription on the Changelist object. It will then do the right thing for Rietveld and Gerrit. No need to RpcServer(). It is probably my fault that all PostUploadHooks use the RPC object, because they all used Skia's PRESUBMIT.py as reference after I added Post upload hooks.

Here is the tricky part: For Gerrit the description has to end in "Change-ID" so I had to strip it off the original description and then add it back at the end.
Should probably make a helper method that does this somewhere in depot_tools, feel free to use Skia's PRESUBMIT.py as reference.

rmistry@ I don't like postuploadhooks approach you outlined for Gerrit for reasons below, but yes I agree that it does work so this isn't Pri1. Still:

1. Changing description after upload creates yet another patchset in Gerrit.
2. Using ChangeList object in RPESUBMIT scripts should be discouraged just like the use of rietveld_obj and for the same reason: refactoring git_cl.py will break PRESUBMIT.py scripts.

And 3. Dealing with Change-Id and other Gerrit-style footers while inserting old "TAGS=xxx" must be done carefully. I've already written this in git_cl Description object (+ test coverage) using depot_tools/git_footers.py library.

So, my proposal is to add a new API method to presubmit_support.output_api to tell git_cl to add extra lines to description message.

All that sounds good to me. My goal was to continue to use post upload hooks with Gerrit similar to how it is used with Rietveld to unblock Skia dogfooders. There definitely exist much better solutions to do it the right way. Hopefully this will be picked up by somebody.

+agable@ +kbr@ FYI

this task is short term to enable smoother Gerrit migration, but it's not the only way to do it - one can just write extra if conditions to do RPC to Gerrit server instead of rietveld.

Long term CQ_INCLUDE_TRYBOTS should be killed, of course.

Agree with both making the current CQ_INCLUDE_TRYBOTS mechanism work with Gerrit, as well as ultimately getting rid of CQ_INCLUDE_TRYBOTS.

However, there's a major problem with the current presubmit scripts (collisions between attempts to add CQ_INCLUDE_TRYBOTS), and an incremental solution is needed. It can be evolved further. For this reason I'm blocking this on Issue 688765.

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

commit 3186301a4e97ed5df7964ae3337a61ca4b0e5fc5
Author: Andrii Shyshkalov <tandrii@chromium.org>
Date: Wed Feb 08 10:39:24 2017

git cl: allow to forcibly bypass cache and fetch CL description.

R=kbr@chromium.org
BUG=633572,688765

Change-Id: I2ce6530148bc2f00fe9f6a80aaccc520c69a2f83
Reviewed-on: https://chromium-review.googlesource.com/439186
Commit-Queue: Andrii Shyshkalov <tandrii@chromium.org>
Reviewed-by: Kenneth Russell <kbr@chromium.org>

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

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

commit 83051152f32458b829cd787dac37d03ce938d542
Author: Andrii Shyshkalov <tandrii@chromium.org>
Date: Wed Feb 08 11:20:17 2017

git cl: fix bugs in UpdateDescription.

R=kbr@chromium.org
BUG=633572,688765

Change-Id: I05130cfd05f89a4605c34b15cadf13a33e3565d9
Reviewed-on: https://chromium-review.googlesource.com/439445
Commit-Queue: Andrii Shyshkalov <tandrii@chromium.org>
Reviewed-by: Kenneth Russell <kbr@chromium.org>

[modify] https://crrev.com/83051152f32458b829cd787dac37d03ce938d542/git_cl.py

I think this was already fully implemented in Issue 688765 both for Rietveld and Gerrit. If you agree then please duplicate this into the other issue.

This is true that thanks to your work, it now works for Gerrit, which is great indeed.

However, updating description is ~5 second extra latency. This means the number of patchsets will be greatly increased on Gerrit (where description update implies new patchset).

Therefore, I think this issue still holds.

Sounds good. Looking forward to additional optimizations.

This is currently broken. See https://bugs.chromium.org/p/chromium/issues/detail?id=710547 and the bug duped into it. It isn't correctly preserving the Change-Id footer.

kbr, I'm going to assign that other issue to you and ask you to take another look. I'm going to leave this one open to track the goal of creating a real api so it doesn't have to be done after the fact.

Linked this to  Issue 642457  to make that change -- which affects the code already landed for this bug -- easier to find.

 Issue 759802  has been merged into this issue.

Removing Milestone-Afterglow, as it has ceased to have meaning. More refined milestones may be added back in the near future.

Although this functionality works, I'm going to keep this open since we'd much rather have the CQ_INCLUDE_TRYBOT lines included *before* the initial upload, rather than set via API calls after the initial upload.

 Issue gerrit:8541  has been merged into this issue.

We've brainstormed this on an (sorry, internal) design doc [1]. We agreed to that adding support for this directly in cq.cfg of a project is the best way forward. The issue 892433 (also internal) will deal with adding this feature to CQ.

This ticket will be to actually migrate chromium/src today's PRESUBMIT.py to cq.cfg.

[1] https://docs.google.com/document/d/1FoiFnVNYNQYWOsmJYsISh1jJd9an-yI2Wp0ulTG9f44/edit?ts=5bb674bd#heading=h.h1s42wy7px2e

Just to confirm - this will handle all of the per-directory PRESUBMIT.py files, and the trybots they add? Not just the top-level one?

Re #31: Yes

 Issue 843737  has been merged into this issue.

We should send a PSA out for this to chromium-dev as soon as we can, for the following two reasons:

1) We will have to re-train people to know how to change the configs (i.e., what to do instead of using presubmit hooks).

2) More importantly, it's possible that someone is doing a presubmit that is more complicated than `if you modify one of these files, add these configs`, and we should try to discover that sooner rather than later. I don't *think* anyone is doing that, but I've been wrong before :).