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

Issue 672378 link

Starred by 3 users

Issue metadata

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

Blocked on:
issue 611808
issue 642484
issue 765545
issue 765633

Blocking:
issue 600469



Sign in to add a comment

Migrate WebRTC to PolyGerrit

Project Member Reported by kjellander@chromium.org, Dec 8 2016

Issue description

Repos:
https://chromium.googlesource.com/external/webrtc/+/refs/meta/config/project.config
https://chromium.googlesource.com/external/webrtc/+/master/codereview.settings

Point of contact:
kjellander@

Lists:
discuss-webrtc@googlegroups.com
webrtc-users@


I filed this bug since I'd like to start dogfooding this as soon I get a green light from tandrii@. It's still pending some gnumbd work since WebRTC is using that.
I mainly copied one of agable@ other migration bugs.
 
Blockedon: 611808
Thanks! We're definitely looking forward to getting WebRTC dogfooding as soon as gnumbd is ready.
I'd like to start this now that we're off gnumbd. Unfortunately we're blocked by  bug 611808  which is a significant amount of work (I confirmed it with https://chromium-review.googlesource.com/c/423071/ and a tryjob: https://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_rel_ng/builds/363630)

Would it be possible to get our CQ wired up with Gerrit support while waiting for that? I can't see any CQ features when I look at https://chromium-review.googlesource.com/c/423071/
Your project.config is not yet correct - either wait for agable@ OR do it yourself based on https://paste.googleplex.com/5845787124695040. And of course cq.cfg needs "gerrit {}" and "git_repo_url: xxx" lines.
Yep, the plan is to do this ASAP now that you no longer need gnumbd and the prod freeze is over. You're welcome to jump on this now if you feel like it, but I have a script that I follow to make sure it works right and all the right communication happens, and I plan to start that tomorrow when my trooper shift is over.
Please, don't enable Gerrit for codereview *yet* for wider webrtc audience because Gerrit CLs can't land with proper footers yet (validator rejects them before generator gets a chance to generate them because validator runs on old patchset, which I think is a bug in Gerrit that I'm fixing this week).
Ah, I wasn't aware of this issue. Where is it being tracked?

kjellander: In this case, please don't do any of the steps for enabling gerrit, or communicate that it is ready yet. Although this one blocker is mostly resolved, I'll make sure we're truly ready before communicating with you and the team that dogfood is ready.
Blockedon: 642484
This bug tracks generation of footers in Gerrit:  issue 642484 
and it's still open :(

I don't see a problem of enabling Gerrit though, and it's definitely easier to verify the fix of the  issue 611808  with CQ and Gerrit CL. Perhaps one can limit ability upload to Gerrit to just kjellander@ though.
Thanks for chiming in. Just be confirm, even if  issue 642484  is fixed, we'd still be blocked by  issue 611808  due to using a git subtree mirror in Chrome, right? We want to get rid of that but since it's a large restructuring I'm curious on if there's no other way (but I'm not saying we should put a dirty hack in for just our case).
Correct under assumption that you want chromium tryjobs on WebRTC CLs, which afaiu is very important for WebRTC.
Labels: Milestone-Fishfood
Labels: -Milestone-Fishfood Milestone-Afterglow
Cc: ehmaldonado@chromium.org
Can we setup Gerrit so we can start using it experimentally (i.e. git cl upload --gerrit)?

Right now it doesn't show Code-review or Commit-queue options for us (example: https://chromium-review.googlesource.com/c/506069/)
Project Member

Comment 14 by bugdroid1@chromium.org, May 17 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/external/webrtc.git/+/81a28f1174e3c175f4c60040d8754ecf7e4bbb28

commit 81a28f1174e3c175f4c60040d8754ecf7e4bbb28
Author: ehmaldonado <ehmaldonado@webrtc.org>
Date: Wed May 17 11:46:06 2017

Add gerrit to cq.cfg

BUG= chromium:672378 
NOTRY=True

Review-Url: https://codereview.webrtc.org/2888533004
Cr-Commit-Position: refs/heads/master@{#18177}

[modify] https://crrev.com/81a28f1174e3c175f4c60040d8754ecf7e4bbb28/infra/config/cq.cfg

Project Member

Comment 15 by bugdroid1@chromium.org, May 17 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/external/webrtc.git/+/b30843a2faae8975b2d9f6706f9013327b169948

commit b30843a2faae8975b2d9f6706f9013327b169948
Author: ehmaldonado <ehmaldonado@webrtc.org>
Date: Wed May 17 12:01:47 2017

Revert of Add gerrit to cq.cfg (patchset #2 id:20001 of https://codereview.webrtc.org/2888533004/ )

Reason for revert:
Might have broken CQ.

Original issue's description:
> Add gerrit to cq.cfg
>
> BUG= chromium:672378 
> NOTRY=True
>
> Review-Url: https://codereview.webrtc.org/2888533004
> Cr-Commit-Position: refs/heads/master@{#18177}
> Committed: https://chromium.googlesource.com/external/webrtc/+/81a28f1174e3c175f4c60040d8754ecf7e4bbb28

TBR=tandrii@chromium.org,kjellander@webrtc.org,tandrii@google.com
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= chromium:672378 

Review-Url: https://codereview.webrtc.org/2888113002
Cr-Commit-Position: refs/heads/master@{#18178}

[modify] https://crrev.com/b30843a2faae8975b2d9f6706f9013327b169948/infra/config/cq.cfg

Project Member

Comment 16 by bugdroid1@chromium.org, May 17 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/external/webrtc.git/+/acb8e4171876ebcee524b34b116b11fe6f7ddcf4

commit acb8e4171876ebcee524b34b116b11fe6f7ddcf4
Author: ehmaldonado <ehmaldonado@webrtc.org>
Date: Wed May 17 12:26:34 2017

Reland of Add gerrit to cq.cfg (patchset #1 id:1 of https://codereview.webrtc.org/2888113002/ )

Reason for revert:
False alarm

Original issue's description:
> Revert of Add gerrit to cq.cfg (patchset #2 id:20001 of https://codereview.webrtc.org/2888533004/ )
>
> Reason for revert:
> Might have broken CQ.
>
> Original issue's description:
> > Add gerrit to cq.cfg
> >
> > BUG= chromium:672378 
> > NOTRY=True
> >
> > Review-Url: https://codereview.webrtc.org/2888533004
> > Cr-Commit-Position: refs/heads/master@{#18177}
> > Committed: https://chromium.googlesource.com/external/webrtc/+/81a28f1174e3c175f4c60040d8754ecf7e4bbb28
>
> TBR=tandrii@chromium.org,kjellander@webrtc.org,tandrii@google.com
> # Skipping CQ checks because original CL landed less than 1 days ago.
> NOPRESUBMIT=true
> NOTREECHECKS=true
> NOTRY=true
> BUG= chromium:672378 
>
> Review-Url: https://codereview.webrtc.org/2888113002
> Cr-Commit-Position: refs/heads/master@{#18178}
> Committed: https://chromium.googlesource.com/external/webrtc/+/b30843a2faae8975b2d9f6706f9013327b169948

TBR=tandrii@chromium.org,kjellander@webrtc.org,tandrii@google.com
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= chromium:672378 

Review-Url: https://codereview.webrtc.org/2886143002
Cr-Commit-Position: refs/heads/master@{#18181}

[modify] https://crrev.com/acb8e4171876ebcee524b34b116b11fe6f7ddcf4/infra/config/cq.cfg

Project Member

Comment 17 by bugdroid1@chromium.org, May 17 2017

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

commit 591dc20cf87064d636e8921448ba8a5d00e97039
Author: Edward Lemur <ehmaldonado@chromium.org>
Date: Wed May 17 13:29:10 2017

WebRTC: Add gerrit tryjob expectation to the recipes.

Bug: chromium:672378 
Change-Id: Ib55954614f6e2b110fe06e580e4c389e05403f36
Reviewed-on: https://chromium-review.googlesource.com/505497
Reviewed-by: Andrii Shyshkalov <tandrii@chromium.org>
Reviewed-by: Henrik Kjellander <kjellander@chromium.org>
Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>

[modify] https://crrev.com/591dc20cf87064d636e8921448ba8a5d00e97039/scripts/slave/recipes/webrtc/standalone.py
[add] https://crrev.com/591dc20cf87064d636e8921448ba8a5d00e97039/scripts/slave/recipes/webrtc/standalone.expected/tryserver_webrtc_linux_dbg_gerrit.json

I've enabled all of the chromium gerrit review settings: https://chromium.googlesource.com/external/webrtc/+/f88ff12789efadd0cb3782f4761f3bb398d47df4

All yours to experiment with.
I can run trybots using the CQ Dry Run button (yay :D), but I don't see a way to select a subset of bots to run.
That's because we need to add a configuration for the buildbucket plugin like this: https://chromium.googlesource.com/chromium/src/+/refs/meta/config/buildbucket.config

I'd add it for you, but I don't know what buckets+bots you want to have enabled :)
If it's not too much trouble, could you please add these?
And teach us how to do it ;)?

master.tryserver.webrtc
 android_arm64_rel
 android_clang_dbg
 android_compile_arm64_dbg
 android_compile_arm64_rel
 android_compile_dbg
 android_compile_mips_dbg
 android_compile_rel
 android_compile_x64_dbg
 android_compile_x86_dbg
 android_compile_x86_rel
 android_dbg
 android_experimental
 android_more_configs
 android_rel
 ios32_sim_ios9_dbg
 ios64_sim_ios10_dbg
 ios64_sim_ios9_dbg
 ios_api_framework
 ios_arm64_dbg
 ios_arm64_rel
 ios_dbg
 ios_rel
 linux32_dbg
 linux32_rel
 linux_arm
 linux_asan
 linux_baremetal
 linux_compile_dbg
 linux_compile_rel
 linux_dbg
 linux_libfuzzer_rel
 linux_memcheck
 linux_more_configs
 linux_msan
 linux_rel
 linux_tsan2
 linux_ubsan
 linux_ubsan_vptr
 mac_asan
 mac_baremetal
 mac_compile_dbg
 mac_compile_rel
 mac_dbg
 mac_rel
 presubmit
 win_asan
 win_baremetal
 win_clang_dbg
 win_clang_rel
 win_compile_dbg
 win_compile_rel
 win_compile_x64_dbg
 win_compile_x64_rel
 win_dbg
 win_rel
 win_x64_clang_dbg
 win_x64_clang_rel
 win_x64_dbg
 win_x64_rel
 win_x64_win10
 win_x64_win8
Sure! It's super easy, if you're used to dealing with git refs other than "origin/master":

$ cd external/webrtc
$ git fetch origin refs/meta/config
$ git checkout FETCH_HEAD
$ # edit files
$ git commit
$ git push origin HEAD:refs/for/refs/meta/config
$ git checkout origin/master  # return to normal

The result is a CL which looks like this: https://chromium-review.googlesource.com/c/513044/
Blockedon: 726068
Blockedon: -726068
 Issue 738329  has been merged into this issue.
Labels: -Milestone-Afterglow Milestone-Turndown
I'd like to set a migration date of Tuesday, September 26, to flip the default and turn Rietveld read-only for webrtc. Does that work for you?
Cc: phoglund@chromium.org
Patrik: That should work, right? Who should own this from the WebRTC side?

We should send out a PSA soon then, so people get enough time to finish their ongoing CLs.
No need for a WebRTC owner; I've done this exact migration dozens of times now. I have the PSAs and CLs all drafted. All I need to know is the date, and what lists to send PSAs to.
Cc: oprypin@chromium.org
Right. Aaron, if you run into problems I think you should talk to Edward as usual.

You should send PSAs to webrtc-users@google.com and https://groups.google.com/forum/#!forum/discuss-webrtc (external mailing list for open source contributors).

For date I suggest Sep 25. We currently have a huge migration landing (LevelUp), and everybody is away on an offsite next week, so the week after that is probably good to aim for. WDYT Edward, Oleh, Henrik?
I'd prefer the 26th (Tuesday) over the 25th, so that I can use Monday to get all my changes set up and send the final "one day warning" PSA.
26th is good. Let me know when you send to the external list since it's moderated.
SGTM
Actually going to do this on the 27th due to promo committee meetings being on the 26th. will send PSA asap.
Blockedon: 765633 765545
Blocking: 600469
Project Member

Comment 37 by bugdroid1@chromium.org, Sep 29 2017

The following revision refers to this bug:
  https://webrtc.googlesource.com/src.git/+/3db476232779b8808d12075c81104c0ef703d7b3

commit 3db476232779b8808d12075c81104c0ef703d7b3
Author: Aaron Gable <agable@chromium.org>
Date: Fri Sep 29 01:38:07 2017

Make Gerrit the default for WebRTC changes

Bug:  chromium:672378 
Change-Id: Idc6035b28daa916a15cceb64a79da06b1765a8ce
Reviewed-on: https://webrtc-review.googlesource.com/4600
Commit-Queue: Niklas Enbom <niklas.enbom@webrtc.org>
Reviewed-by: Niklas Enbom <niklas.enbom@webrtc.org>
Reviewed-by: Justin Uberti <juberti@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#20031}
[modify] https://crrev.com/3db476232779b8808d12075c81104c0ef703d7b3/codereview.settings

Status: Fixed (was: Assigned)

Sign in to add a comment