New issue
Advanced search Search tips

Issue 693792 link

Starred by 1 user
Status: Fixed
Owner:
qyears...@chromium.org
Closed: Mar 2017
Cc:
tandrii@chromium.org
jeffcarp@chromium.org
vadimsh@chromium.org
friedman@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug

Blocked on:
issue 671684
Blocking:
issue 656171
issue 676491



Sign in to add a comment

Migrate web-platform-tests auto-import from Rietveld to Gerrit.

Project Member Reported by qyears...@chromium.org, Feb 18 2017 
There will be several advantages once we do this:

 - Gerrit will likely not have an issue with encodings ( bug 656171 )
 - Gerrit will likely be able to handle more/bigger files.

Things to do before this can be done:

 - Change rebaseline-cl to not depend on Rietveld ( bug 671684 )
 - Change wpt_expectations_updater.py to get latest try jobs from git cl try-results
 - Change test_importer to pass --gerrit instead of --rietveld

And, importantly:

 - Make sure authentication works for blink-w3c-test-autoroller@chromium.org to upload CLs to Gerrit.

vadimsh@ or tandrii@, do you know if passing credentials with --auth-refresh-token-json should work the same way for Gerrit as it does for Rietveld?

Comment 1 by vadimsh@chromium.org, Feb 18 2017 

No, afaik Gerrit upload uses current git credentials in ~/.gitcookies and ignores --auth-refresht-token-json. tandrii@ should know for sure.

Comment 2 by tandrii@chromium.org, Feb 20 2017 

Vadim is correct. What you should do is ensure that Puppet deploys proper credentials to your builder's netrc file. That means doing something similar to https://chrome-internal-review.googlesource.com/c/313261/

Comment 3 by tandrii@chromium.org, Feb 20 2017 

And, FTR, --auth-refresh-token-json is totally ignored in Gerrit mode of `git cl`.

Comment 4 by qyears...@chromium.org, Feb 21 2017

Labels: Proj-Gerrit-Migration
Owner: qyears...@chromium.org
Status: Assigned (was: Available)
Thanks Andrii! I'd like to try to do something like that this week :-)

Comment 5 by tandrii@chromium.org, Feb 22 2017 

Also, beware of making correct puppet config. I've just fixed one for recipe-roller (in fact, you can totally copy it's setup for your bot account): https://chrome-internal-review.googlesource.com/#/c/330984/

Comment 6 by qyears...@chromium.org, Feb 23 2017

Components: Blink>Infra>Predictability

Comment 7 by qyears...@chromium.org, Feb 23 2017

Blocking: 656171

Comment 8 by qyears...@chromium.org, Feb 23 2017

Blocking: 676491

Comment 9 by qyears...@chromium.org, Feb 23 2017 

Next step: get the right netrc token for the bot account blink-w3c-test-autoroller@chromium.org (https://chrome-internal-review.googlesource.com/c/330552/).

The bug where we discussed adding this account originally was bug 632435, and friedman@ set up refresh token credentials for Rietveld in https://chromereviews.googleplex.com/482827013.

Comment 10 by qyears...@chromium.org, Feb 23 2017

Cc: friedman@chromium.org
Project Member

Comment 11 by bugdroid1@chromium.org, Feb 23 2017 

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/infra/puppet/+/ea54e14fa1aa281051f8ba0003fbea5be6f0a000

commit ea54e14fa1aa281051f8ba0003fbea5be6f0a000
Author: Quinten Yearsley <qyearsley@google.com>
Date: Thu Feb 23 22:52:33 2017
Project Member

Comment 12 by bugdroid1@chromium.org, Feb 24 2017 

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

commit 271d08b32e78ccf8117c6fc9b6e146e5d5ab6ed2
Author: qyearsley <qyearsley@chromium.org>
Date: Fri Feb 24 16:27:44 2017

Use git cl try-results intead of Rietveld in wpt_expectations_updater.py.

The purpose of this is to allow WPT import to use Gerrit.

BUG= 693792 

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

[modify] https://crrev.com/271d08b32e78ccf8117c6fc9b6e146e5d5ab6ed2/third_party/WebKit/Tools/Scripts/webkitpy/w3c/wpt_expectations_updater.py
[modify] https://crrev.com/271d08b32e78ccf8117c6fc9b6e146e5d5ab6ed2/third_party/WebKit/Tools/Scripts/webkitpy/w3c/wpt_expectations_updater_unittest.py

Comment 14 by qyears...@chromium.org, Feb 25 2017 

Alright, first auto-import using the bot account, uploading to Gerrit:

Got this interesting result:


Uploading change list.
Gathering directory owners emails to CC.
Issue: Issue number: 446406 (https://chromium-review.googlesource.com/446406)
Triggering try jobs.
Traceback (most recent call last):
  ...
webkitpy.common.system.executive.ScriptError: Failed to run "['git', 'cl', 'try', '-b', 'android_blink_rel']" exit_code: 1

output: You are not logged in. Please login first by running:
  depot-tools-auth login chromium-review.googlesource.com

https://build.chromium.org/p/chromium.infra.cron/builders/w3c-test-autoroller/builds/7973/steps/update%20wpt/logs/stdio


So, it looks like the bot account (blink-w3c-test-autoroller@chromium.org) is sucessfully uploading CLs, but not successfully triggering try jobs.

Comment 15 by vadimsh@chromium.org, Feb 25 2017 

Ugh.. Tryjobs are triggered through Builddbucket. It still needs '--auth-refresh-token-json' (because gerrit token is not usable from Buildbucket). Basically, stuff removed here is still needed for git cl try: https://codereview.chromium.org/2718503004/diff/20001/third_party/WebKit/Tools/Scripts/webkitpy/common/net/git_cl.py

Comment 16 by vadimsh@chromium.org, Feb 25 2017 

If you want it to be less awkward, it is possible, in theory, to make --auth-refresh-token-json work for Gerrit too (by generating a new token with both 'gerrit' and 'userinfo.email' OAuth scopes and somehow feeding it to 'git').

The other way around (using 'git' token for Buildbucket) is not going to work, because Gerrit tokens don't have 'userinfo.email' scope required when talking to GAE apps (like Buildbucket).

The fastest solution is too use two tokens (i.e. just add --auth-refresh-token-json stuff back). It's what secretly happens on developer machines too (git command use ~/.gitcookies, "git cl try" uses the token setup by 'depot-tools-auth login ...').

Comment 23 by qyears...@chromium.org, Feb 27 2017

Status: Fixed (was: Assigned)
Auto-importer is now using Gerrit by default, and successfully committed its first CL via Gerrit today: https://chromium-review.googlesource.com/c/446562/

Comment 24 by vadimsh@chromium.org, Feb 27 2017 

Awesome!

Comment 25 by qyears...@chromium.org, Mar 1 2017

Status: Started (was: Fixed)
The first CL which worked didn't involve uploading a second patchset, and it looks like there's probably one more little change that needs to be made before import works more generally.

Example failed build: https://build.chromium.org/p/chromium.infra.cron/builders/w3c-test-autoroller/builds/7983

I think that when uploading a second patchset, I need to change the command from "git cl upload -m <message>" to "git cl upload -t <message>" or "git cl upload -f -m <message>". I'll verify which one works and then make another CL for this.

Comment 26 by tandrii@chromium.org, Mar 1 2017 

the -m message isn't actually supposed (that's bug, btw, can you please file?) on not first upload.

Comment 27 by qyears...@chromium.org, Mar 1 2017 

So, is the desired behavior that git cl shouldn't prompt for patchset title when -m is given, and that the argument to -m should be used as the patchset title? Or that -m shouldn't apply to Gerrit and -t should be used instead for patchset titles?

Comment 28 by tandrii@chromium.org, Mar 1 2017 

Former. That is that -m is taken into account by git cl for Gerrit on not-first uploads, and hence inferring -t automatically.
Project Member

Comment 29 by bugdroid1@chromium.org, Mar 2 2017 

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

commit 6e035cf67feec1c6547bf3a919bc3882636ac1af
Author: qyearsley <qyearsley@chromium.org>
Date: Thu Mar 02 17:12:17 2017

When uploading updates to baselines/expectations, pass patchset title with "-t".

BUG= 693792 

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

[modify] https://crrev.com/6e035cf67feec1c6547bf3a919bc3882636ac1af/third_party/WebKit/Tools/Scripts/webkitpy/w3c/test_importer.py

Comment 30 by qyears...@chromium.org, Mar 2 2017

Status: Fixed (was: Started)

Comment 31 by foolip@chromium.org, Jul 3 2017

Components: Blink>Infra>Ecosystem

Comment 32 by foolip@chromium.org, Jul 3 2017

Components: -Blink>Infra>Predictability
►

Sign in to add a comment