I would like to look into fixing this but not sure how to start. Andrii, will you be able to point me in the right directions?

Also, I thought that CQ was working for a few PG projects, why is this fix required for the Skia project specifically?

Frankly, i'd completely get rid of this function, and effectively require the use of bot_update, though that requires gclient config.

And that's exactly why it doesn't work for skia - why doesn't skia use bot_update? :)

Looks like maybe_apply_issue is used by nacl and skia: https://cs.chromium.org/search/?q=maybe_apply_issue&sq=package:chromium&type=cs

Eric should be able to help explain why we use it.

Skia doesn't use bot_update because we've always used git+gclient.  What does bot_update do for us?

Frankly, I'd always assumed that bot_update was for chromium only.

so, bot_update isn't chromium specific. Original reason for bot_update: support svn->git transition. But it has more goodies there:

 * properly[1] apply patches that change DEPS
 * retries
 * auto setup of gclient caching
 * dealing with old aborted gclient syncs that left .git/lock files
and I guess more.



[1] Really proper way would do this DEPS application recursively all the way, but so far I think it stops at top level DEPS.

Okay, we can move to using bot_update, but I want to wait until the dust settles after moving recipes into the Skia repo.

Filed  bug skia:5588  to track this work on Skia side.
Andrii, if you can convince Nacl to move to bot_update as well then you will be able to remove maybe_apply_issue and mark this bug as WontFix.

Spoke to Eric and we will not be able to get to this at least till next quarter due to other work in the pipeline. We definitely do not want to wait that long to get CQ support in PG.


Coming back to my question in https://bugs.chromium.org/p/chromium/issues/detail?id=631528#c1 :

I would like to look into fixing this but not sure how to start. Andrii, will you be able to point me in the right directions?

We can chat about it if that will be easier.

NaCl is actually already using bot_update, followed by a call to maybe_apply_patch. So, i'm not too worried about them.

Now speaking of your comment #1:
This is example run of Gerrit CQ job:
 https://luci-milo.appspot.com/swarming/task/30414eb707f1be10

among properties there are:

event.change.number	338801	recipe bootstrap
event.patchSet.ref	"refs/changes/01/338801/1"	recipe bootstrap
patch_project	"playground/gerrit-cq/normal"	recipe bootstrap
patch_storage	"gerrit"	recipe bootstrap

So, your recipe should:
  1. check patch_storage, if gerrit, then
  2. check/ensure patch_project matches what recipe expects. Note that patch_project will be the full repo name on your Gerrit host (e.g., chromium/src for chromium).
  3. git fetch origin `event.patchset.ref` (but if you use git cache, you'd need to use actual URL instead of origin)
  4.a And now if you want to keep backwards compatible with Rietveld patches, you have to rebase FETCH_HEAD onto your branch (likely, refs/heads/master).
  4.b Otherwise, you can just "git checkout FETCH_HEAD" and be done with it. Note that in either case, dependent patches will be applied properly (nice, isn't it?)

The job example above actually uses pure git commands and effectively ends up with 4.b. Bot_update now does the same as 4.b, but Andy is working on making it do 4.a so that it's like Rietveld. Otherwise, analyze gets confused by HUGE git diff.

Update:
Instead of supporting Gerrit patches in tryserver.maybe_apply_issue, a new endpoint was added to bot_update:
https://cs.chromium.org/chromium/tools/depot_tools/recipe_modules/bot_update/api.py?l=59

Skia recipes have been updated to use that new endpoint. Usage of the endpoint will be removed once Skia uses ensure_checkout.

Marking this as WontFix. Please reopen if I missed something.