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

Issue 685318 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Aug 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug


Sign in to add a comment

Migrate v8 to PolyGerrit

Project Member Reported by aga...@chromium.org, Jan 25 2017

Issue description

Repos:
https://chromium.googlesource.com/v8/v8/+/refs/meta/config/project.config

Point of contact:
machenbach@

Lists:
v8-team@
v8-dev@googlegroups.com

Expected date of PSA and enabling Gerrit:
Thursday, Jan 26 (Pacific)

Expected date of full migration:
Unknown, depends on feedback
 
As I discussed with Andrii, before switching on Gerrit by default, I'd like a smaller subset of people to opt in first. We were not there yet. Are we now?
E.g. this is strictly blocked on resolving the story for external contributors being able to submit to CQ. Is that resolved?

Comment 3 by aga...@chromium.org, Jan 26 2017

Yep, that's explicitly the plan. That's why the date given above (Thursday) is the date for *enabling* Gerrit review. Then we'll be able to let people opt in (by passing the --gerrit flag to git-cl, or setting a particular git-config value), test all the workflows, and make sure things seem to your satisfaction.

Specifically regarding CQ and external contributors: yes, we have the story for that. You can follow it in issue 641422.
If issue 641422 is a blocker for "Migrate v8 to PolyGerrit", should it be marked as such?
Blockedon: 641422
I'm marking the blocker. Thanks Aaron for the clarification! And please go ahead :)
I've tried tryjobs for whitespace issue https://chromium-review.googlesource.com/c/433541/ and while bot_update applies patch just fine, maybe_trigger relies on api['issue'] property:

@@@STEP_LOG_LINE@exception@  File "/b/build/scripts/slave/recipes/v8.py", line 103, in RunSteps@@@
@@@STEP_LOG_LINE@exception@    v8.maybe_trigger(**additional_trigger_properties)@@@
@@@STEP_LOG_LINE@exception@  File "/b/build/scripts/slave/.recipe_deps/recipe_engine/recipe_engine/recipe_api.py", line 505, in _inner@@@
@@@STEP_LOG_LINE@exception@    return func(*a, **kw)@@@
@@@STEP_LOG_LINE@exception@  File "/b/build/scripts/slave/recipe_modules/v8/api.py", line 1103, in maybe_trigger@@@
@@@STEP_LOG_LINE@exception@    issue=self.m.properties['issue'],@@@


which of course fails.
hmja - just some lame hardcoded assumption. But I won't be able to look into it before next week. Feel free to change it in a way that the rietveld version still works. Maybe just conditionally check in maybe_trigger which properties are there and pass them in a more robust way. I assume we need to pass something else in the gerrit case?
hmja - just some lame hardcoded assumption. But I won't be able to look into it before next week. Feel free to change it in a way that the rietveld version still works. Maybe just conditionally check in maybe_trigger which properties are there and pass them in a more robust way. I assume we need to pass something else in the gerrit case?

Comment 9 by aga...@chromium.org, Jan 27 2017

calculate_patch_base also relies on self.m.properties['issue']. maybe_trigger also relies on other properties like self.m.properties['rietveld'].

I don't know this recipe module well enough to
a) know exactly which recipes use it
b) know where those recipes run
c) know whether its fine to just use "self.m.properties.get('issue')" and have some Nonetypes floating around

I can file CLs to do those sorts of hacky work arounds, and then test some whitespace issues, but it'll all be trial-and-error for me. If someone else knows this recipe better and can fix it the right way, that'd be best.
Status: Started (was: Assigned)
CLs for setting the repo up have been filed but not landed:
https://chromium-review.googlesource.com/c/434162/ for ACLs
https://chromium-review.googlesource.com/c/434177/ for gerrit CQ
https://chromium-review.googlesource.com/c/434178/ for chumpdetector
https://chromium-review.googlesource.com/c/434221/ for some cleanup of ACLs

Once the first one lands, I'll send the PSA that I have prepped and ready to go. At this point, seems like that'll probably be Monday, unless either tandrii or machenbach is working unreasonably late.
well, since I work on weekends... I've reviewed the CLs.

I am against PSA until tryjobs pass as otherwise we are wasting V8 dev's time.
Cc: jochen@chromium.org danno@chromium.org
Before you send a PSA to the whole team, please let a small subset know first: jochen, danno and me.

I'd like to have this a bit more smoke-tested before the whole team has a bash with it.
Project Member

Comment 13 by bugdroid1@chromium.org, Jan 30 2017

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

commit 99f475af14c2bacaafaedee2748b422a09821770
Author: Michael Achenbach <machenbach@chromium.org>
Date: Mon Jan 30 15:44:27 2017

V8: Include gerrit properties on triggered trybots

BUG= 685318 

Change-Id: I2c380a20ac7d069eabc497dbcfd70b6a9cb358b5
Reviewed-on: https://chromium-review.googlesource.com/434657
Reviewed-by: Andrii Shyshkalov <tandrii@chromium.org>
Reviewed-by: Aaron Gable <agable@chromium.org>
Commit-Queue: Michael Achenbach <machenbach@chromium.org>

[modify] https://crrev.com/99f475af14c2bacaafaedee2748b422a09821770/scripts/slave/recipe_modules/v8/api.py
[modify] https://crrev.com/99f475af14c2bacaafaedee2748b422a09821770/scripts/slave/recipes/v8.expected/full_tryserver_v8_v8_linux64_asan_rel_ng.json
[modify] https://crrev.com/99f475af14c2bacaafaedee2748b422a09821770/scripts/slave/recipes/v8.expected/full_tryserver_v8_v8_linux64_avx2_rel_ng.json
[modify] https://crrev.com/99f475af14c2bacaafaedee2748b422a09821770/scripts/slave/recipes/v8.expected/full_tryserver_v8_v8_linux64_gyp_rel_ng.json
[modify] https://crrev.com/99f475af14c2bacaafaedee2748b422a09821770/scripts/slave/recipes/v8.expected/full_tryserver_v8_v8_linux64_rel_ng.json
[modify] https://crrev.com/99f475af14c2bacaafaedee2748b422a09821770/scripts/slave/recipes/v8.expected/full_tryserver_v8_v8_linux64_verify_csa_rel_ng.json
[modify] https://crrev.com/99f475af14c2bacaafaedee2748b422a09821770/scripts/slave/recipes/v8.expected/full_tryserver_v8_v8_linux_arm64_rel_ng.json
[modify] https://crrev.com/99f475af14c2bacaafaedee2748b422a09821770/scripts/slave/recipes/v8.expected/full_tryserver_v8_v8_linux_arm_rel_ng.json
[modify] https://crrev.com/99f475af14c2bacaafaedee2748b422a09821770/scripts/slave/recipes/v8.expected/full_tryserver_v8_v8_linux_dbg_ng.json
[modify] https://crrev.com/99f475af14c2bacaafaedee2748b422a09821770/scripts/slave/recipes/v8.expected/full_tryserver_v8_v8_linux_nodcheck_rel_ng.json
[modify] https://crrev.com/99f475af14c2bacaafaedee2748b422a09821770/scripts/slave/recipes/v8.expected/full_tryserver_v8_v8_linux_noi18n_rel_ng.json
[modify] https://crrev.com/99f475af14c2bacaafaedee2748b422a09821770/scripts/slave/recipes/v8.expected/full_tryserver_v8_v8_linux_rel_ng.json
[add] https://crrev.com/99f475af14c2bacaafaedee2748b422a09821770/scripts/slave/recipes/v8.expected/full_tryserver_v8_v8_linux_rel_ng_gerrit.json
[modify] https://crrev.com/99f475af14c2bacaafaedee2748b422a09821770/scripts/slave/recipes/v8.expected/full_tryserver_v8_v8_linux_verify_csa_rel_ng.json
[modify] https://crrev.com/99f475af14c2bacaafaedee2748b422a09821770/scripts/slave/recipes/v8.expected/full_tryserver_v8_v8_mac_rel_ng.json
[modify] https://crrev.com/99f475af14c2bacaafaedee2748b422a09821770/scripts/slave/recipes/v8.expected/full_tryserver_v8_v8_win64_rel_ng.json
[modify] https://crrev.com/99f475af14c2bacaafaedee2748b422a09821770/scripts/slave/recipes/v8.expected/full_tryserver_v8_v8_win64_rel_ng_test_filter_builder.json
[modify] https://crrev.com/99f475af14c2bacaafaedee2748b422a09821770/scripts/slave/recipes/v8.expected/full_tryserver_v8_v8_win_nosnap_shared_rel_ng.json
[modify] https://crrev.com/99f475af14c2bacaafaedee2748b422a09821770/scripts/slave/recipes/v8.expected/full_tryserver_v8_v8_win_rel_ng.json
[modify] https://crrev.com/99f475af14c2bacaafaedee2748b422a09821770/scripts/slave/recipes/v8.py

Project Member

Comment 14 by bugdroid1@chromium.org, Jan 30 2017

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

commit 6c84b93ad88d44e2f46306041baf5bbdb06a0a86
Author: machenbach <machenbach@chromium.org>
Date: Mon Jan 30 19:45:47 2017

Whitespace change to check CQ

BUG= chromium:685318 
TBR=tandrii@chromium.org

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

[modify] https://crrev.com/6c84b93ad88d44e2f46306041baf5bbdb06a0a86/tools/whitespace.txt

The whitespace change to test the Gerrit CQ fails on one builder:
https://chromium-review.googlesource.com/c/434127/
https://luci-milo.appspot.com/buildbot/tryserver.v8/v8_linux_chromium_gn_rel/32069

The error is that that builder gets a full chromium checkout and then applies the patch against the v8 subdirectory. But the Gerrit version of the bot is still trying to apply the patch against src/:

===Processing patch solutions===
Patch root is 'src'
Processing solution 'src'
  relative root is '', target is 'DEPS'
===Running git rev-parse HEAD (attempt #1)===
In directory: src
d9ffd1b220d31b7fd2fab3b1c1cb9513cafb99be
===Succeeded in 0.0 mins===

===Applying gerrit ref===
Repo is 'https://chromium.googlesource.com/v8/v8' @ 'd9ffd1b220d31b7fd2fab3b1c1cb9513cafb99be', ref is 'refs/changes/27/434127/1', root is 'src'
===Running git retry fetch https://chromium.googlesource.com/v8/v8 refs/changes/27/434127/1 (attempt #1)===
In directory: src
warning: no common commits
From https://chromium.googlesource.com/v8/v8
 * branch                      refs/changes/27/434127/1 -> FETCH_HEAD
===Succeeded in 1.7 mins===

===Running git checkout FETCH_HEAD (attempt #1)===
In directory: src
Previous HEAD position was d9ffd1b220d3... Convert SaveCardBubbleControllerImpl to use the new navigation callbacks.
HEAD is now at 304b82dc2117... Another Whitespace change to check Gerrit CQ
===Succeeded in 0.1 mins===

===Rebasing===
===Running git checkout -b tmp/5c39081848134bb6b68f7437ca5e321d (attempt #1)===
In directory: src
Switched to a new branch 'tmp/5c39081848134bb6b68f7437ca5e321d'
===Succeeded in 0.0 mins===

<and then the rebases themselves start to fail, obviously, because they share no history with src.git>

I'm investigating what needs to be changed for patch root to be respected.
Here is more background info about using trybots for deps'ed projects:  http://crbug.com/491098 
Another blocker is that the triggered tryjobs don't show up. Compare:
https://codereview.chromium.org/2666563002
https://chromium-review.googlesource.com/c/434127/
Project Member

Comment 18 by bugdroid1@chromium.org, Jan 31 2017

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

commit 71b8e66f89391f1691632ef22bcd582599f865e1
Author: Andrii Shyshkalov <tandrii@chromium.org>
Date: Tue Jan 31 12:07:38 2017

Register v8/v8 in gclient for trying Gerrit v8 patches on chromium trybots.

R=machenbach@chromium.org
BUG= 685318 

Change-Id: I61192392eec98ad6b9a32ef933f2d22cf2aedd0e
Reviewed-on: https://chromium-review.googlesource.com/435318
Commit-Queue: Andrii Shyshkalov <tandrii@chromium.org>
Commit-Queue: Michael Achenbach <machenbach@chromium.org>
Reviewed-by: Michael Achenbach <machenbach@chromium.org>

[modify] https://crrev.com/71b8e66f89391f1691632ef22bcd582599f865e1/recipe_modules/gclient/config.py

The chromium tryjob succeeded after the change above. Though a whitespace CL is not the best test as analyze is smart and bails out. But the "git diff" call that precedes analyze is in the correct directory.

So, only the missing triggered tryjobs are left. CQ seems to correctly block on them. Also buildbucket has them scheduled. I.e. the query
https://cr-buildbucket.appspot.com/_ah/api/buildbucket/v1/search?max_builds=500&fields=builds(failure_reason%2Cid%2Cparameters_json%2Cresult%2Cstatus%2Ctags%2Curl)&tag=buildset%3Apatch%2Fgerrit%2Fchromium-review.googlesource.com%2F434127%2F1
shows also the triggered jobs. It looks as if gerrit UI filters them out somehow...
Project Member

Comment 20 by bugdroid1@chromium.org, Jan 31 2017

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

commit 82edaf874bf18973a228e9a59fab84bdba265ffd
Author: Michael Achenbach <machenbach@chromium.org>
Date: Tue Jan 31 14:51:15 2017

V8: Fix missing gerrit property on trigger

BUG= 685318 
TBR=tandrii@chromium.org,agable@chromium.org

Change-Id: Ibf2c8bd144cb6a332fef0d65b293d04bdc916b57
Reviewed-on: https://chromium-review.googlesource.com/435339
Reviewed-by: Michael Achenbach <machenbach@chromium.org>
Commit-Queue: Michael Achenbach <machenbach@chromium.org>

[modify] https://crrev.com/82edaf874bf18973a228e9a59fab84bdba265ffd/scripts/slave/recipe_modules/v8/api.py

Re 19: The above fix lets the triggered jobs appear in cq status now:
https://chromium-cq-status.appspot.com/v2/patch-status/chromium-review.googlesource.com/434127/1

On the gerrit UI they are still missing :(
Cc: -andyberkheimer@chromium.org
The Gerrit UI uses the buildbucket "builder:" tag to identify and group jobs that had the same builder. For some reason, that tag isn't set on triggered jobs. Since those triggered jobs don't have a "builder:" tag at all, they get passed over during the grouping stage and don't show up.

Two possible fixes:
1) Make triggered jobs have the "builder:" tag in buildbucket
2) Make the buildbucket gerrit plugin use the build.parameters_json.builder_name instead of the builder: tag

Not sure which is easier or faster or more desirable; will consult with nodir@ on (1) before starting implementation on (2).
Cc: no...@chromium.org
I have a pair of CLs which should make this problem go away:
https://chromium-review.googlesource.com/c/435008/ to make masters set the builder: tag when they trigger jobs
https://chromium-review.googlesource.com/c/435009/ to make buildbucket calculate it automatically even if the master doesn't set it
All of the triggered jobs now show up on this whitespace CL: https://chromium-review.googlesource.com/c/435461/
Draft PSA has been sent to danno, machenbach, and jochen.
Got feedback on draft PSA. Registered all accounts in codereview.settings and WATCHLISTS so they can be CC'd. Changed default UI to PolyGerrit. Updated PSA.

Sent PSA!
https://groups.google.com/a/google.com/d/msg/v8-team/oduxKq5CKOY/oM4B8JipBgAJ
https://groups.google.com/d/msg/v8-dev/gzfeEUSInVU/jB-4hmseAgAJ
Project Member

Comment 28 by bugdroid1@chromium.org, Feb 6 2017

Project Member

Comment 29 by bugdroid1@chromium.org, Feb 6 2017

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

commit 48b52a216d945c0b783b63642d4a4f7b87004eb1
Author: Michael Achenbach <machenbach@chromium.org>
Date: Mon Feb 06 14:02:05 2017

Revert "V8: Switch deps roller to gerrit"

This reverts commit f2f81bc7ea2050cf37e8362fec123116c6c2e8b4.

Reason for revert: The bot uses git cache and git cl upload uses the wrong origin in the cache for pushing to gerrit.

Original change's description:
> V8: Switch deps roller to gerrit
> 
> BUG= 685318 
> 
> Change-Id: Ib33883097557693bfe82c82880c66611ca9d2f37
> Reviewed-on: https://chromium-review.googlesource.com/438325
> Reviewed-by: Andrii Shyshkalov <tandrii@chromium.org>
> Commit-Queue: Michael Achenbach <machenbach@chromium.org>
> 

TBR=machenbach@chromium.org,hablich@chromium.org,tandrii@chromium.org,agable@chromium.org,chromium-reviews@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 685318 

Change-Id: Id48ee1663dad7b926d58959c9095818dc6ce241b
Reviewed-on: https://chromium-review.googlesource.com/438328
Reviewed-by: Michael Achenbach <machenbach@chromium.org>
Commit-Queue: Michael Achenbach <machenbach@chromium.org>

[modify] https://crrev.com/48b52a216d945c0b783b63642d4a4f7b87004eb1/scripts/slave/recipes/v8/auto_roll_v8_deps.expected/roll.json
[modify] https://crrev.com/48b52a216d945c0b783b63642d4a4f7b87004eb1/scripts/slave/recipes/v8/auto_roll_v8_deps.py

Blockedon: 689035
Project Member

Comment 31 by bugdroid1@chromium.org, Feb 6 2017

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

commit b775ec987d5d090ab720ec2c080d0de110efdeee
Author: Michael Achenbach <machenbach@chromium.org>
Date: Mon Feb 06 15:17:59 2017

Revert "Revert "V8: Switch deps roller to gerrit""

This reverts commit 48b52a216d945c0b783b63642d4a4f7b87004eb1.

Reason for revert:  Issue 689035  fixed.

Original change's description:
> Revert "V8: Switch deps roller to gerrit"
> 
> This reverts commit f2f81bc7ea2050cf37e8362fec123116c6c2e8b4.
> 
> Reason for revert: The bot uses git cache and git cl upload uses the wrong origin in the cache for pushing to gerrit.
> 
> Original change's description:
> > V8: Switch deps roller to gerrit
> > 
> > BUG= 685318 
> > 
> > Change-Id: Ib33883097557693bfe82c82880c66611ca9d2f37
> > Reviewed-on: https://chromium-review.googlesource.com/438325
> > Reviewed-by: Andrii Shyshkalov <tandrii@chromium.org>
> > Commit-Queue: Michael Achenbach <machenbach@chromium.org>
> > 
> 
> TBR=machenbach@chromium.org,hablich@chromium.org,tandrii@chromium.org,agable@chromium.org,chromium-reviews@chromium.org
> NOPRESUBMIT=true
> NOTREECHECKS=true
> NOTRY=true
> BUG= 685318 
> 
> Change-Id: Id48ee1663dad7b926d58959c9095818dc6ce241b
> Reviewed-on: https://chromium-review.googlesource.com/438328
> Reviewed-by: Michael Achenbach <machenbach@chromium.org>
> Commit-Queue: Michael Achenbach <machenbach@chromium.org>
> 

TBR=agable@chromium.org,machenbach@chromium.org,hablich@chromium.org,tandrii@chromium.org,chromium-reviews@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 685318 , 689035 

Change-Id: If26f9d66be6fb9c5a6fff9c13486d36a91126c56
Reviewed-on: https://chromium-review.googlesource.com/438404
Reviewed-by: Michael Achenbach <machenbach@chromium.org>
Commit-Queue: Michael Achenbach <machenbach@chromium.org>

[modify] https://crrev.com/b775ec987d5d090ab720ec2c080d0de110efdeee/scripts/slave/recipes/v8/auto_roll_v8_deps.expected/roll.json
[modify] https://crrev.com/b775ec987d5d090ab720ec2c080d0de110efdeee/scripts/slave/recipes/v8/auto_roll_v8_deps.py

Blockedon: gerrit:5475
Blockedon: gerrit:4489
Blockedon: 692067

Comment 35 by no...@chromium.org, Feb 14 2017

Cc: -no...@chromium.org
Blockedon: 692642
Blockedon: 618023
Project Member

Comment 38 by bugdroid1@chromium.org, Feb 27 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/67255619f48ecdad87ea354bf7b7053f4f3c603b

commit 67255619f48ecdad87ea354bf7b7053f4f3c603b
Author: Michael Achenbach <machenbach@chromium.org>
Date: Mon Feb 27 09:41:40 2017

[release] Let merge tools use gerrit by default

BUG= chromium:685318 
NOTRY=true
TBR=hablich@chromium.org

Change-Id: Ic5a6d721372ff93d1c2254bad0e8c1d9c0b0af94
Reviewed-on: https://chromium-review.googlesource.com/446344
Reviewed-by: Michael Achenbach <machenbach@chromium.org>
Reviewed-by: Aaron Gable <agable@chromium.org>
Commit-Queue: Michael Achenbach <machenbach@chromium.org>
Cr-Commit-Position: refs/heads/master@{#43440}
[modify] https://crrev.com/67255619f48ecdad87ea354bf7b7053f4f3c603b/tools/release/common_includes.py
[modify] https://crrev.com/67255619f48ecdad87ea354bf7b7053f4f3c603b/tools/release/git_recipes.py
[modify] https://crrev.com/67255619f48ecdad87ea354bf7b7053f4f3c603b/tools/release/test_scripts.py

Blockedon: v8:6030
Project Member

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

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

commit a49511cd49669ac93f98772d2c7d8fbd9d79588e
Author: Michael Achenbach <machenbach@chromium.org>
Date: Thu Mar 02 13:47:19 2017

V8: Work around missing property for triggered bots

BUG= 685318 
TBR=tandrii@chromium.org,agable@chromium.org

Change-Id: I139d073b7f59b90e69abbc3c430ae0cd7bb4c37f
Reviewed-on: https://chromium-review.googlesource.com/448223
Reviewed-by: Andrii Shyshkalov <tandrii@chromium.org>
Reviewed-by: Michael Achenbach <machenbach@chromium.org>
Commit-Queue: Andrii Shyshkalov <tandrii@chromium.org>

[modify] https://crrev.com/a49511cd49669ac93f98772d2c7d8fbd9d79588e/scripts/slave/recipe_modules/v8/api.py

Project Member

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

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

commit 546bb3a34dfa8c072f4fea60ac00268a3fba5df2
Author: Andrii Shyshkalov <tandrii@chromium.org>
Date: Thu Mar 02 14:25:40 2017

Project Member

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

The following revision refers to this bug:
  https://chromium.googlesource.com/infra/gerrit-plugins/buildbucket/+/be1b6aafae6e95e1cc58d5b33ae92bc037efd46b

commit be1b6aafae6e95e1cc58d5b33ae92bc037efd46b
Author: Andrii Shyshkalov <tandrii@chromium.org>
Date: Thu Mar 02 16:47:09 2017

Use patch_repository_url in Gerrit.

CQ used incorrect property before (fixed in
https://chrome-internal-review.googlesource.com/c/332743),
and that's why I mistakenly asked to use the wrong property here.

R=andybons@chromium.org
BUG= 645616 , 685318 

Change-Id: Ibe381f9937bc39bb0fba19c08f1a52724ac2ee54
Reviewed-on: https://chromium-review.googlesource.com/448459
Reviewed-by: Andrew Bonventre <andybons@chromium.org>

[modify] https://crrev.com/be1b6aafae6e95e1cc58d5b33ae92bc037efd46b/test/cr-tryjob-picker_test.html
[modify] https://crrev.com/be1b6aafae6e95e1cc58d5b33ae92bc037efd46b/src/main/resources/static/cr-tryjob-picker.js

Project Member

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

The following revision refers to this bug:
  https://chromium.googlesource.com/infra/gerrit-plugins/buildbucket/+/be1b6aafae6e95e1cc58d5b33ae92bc037efd46b

commit be1b6aafae6e95e1cc58d5b33ae92bc037efd46b
Author: Andrii Shyshkalov <tandrii@chromium.org>
Date: Thu Mar 02 16:47:09 2017

Use patch_repository_url in Gerrit.

CQ used incorrect property before (fixed in
https://chrome-internal-review.googlesource.com/c/332743),
and that's why I mistakenly asked to use the wrong property here.

R=andybons@chromium.org
BUG= 645616 , 685318 

Change-Id: Ibe381f9937bc39bb0fba19c08f1a52724ac2ee54
Reviewed-on: https://chromium-review.googlesource.com/448459
Reviewed-by: Andrew Bonventre <andybons@chromium.org>

[modify] https://crrev.com/be1b6aafae6e95e1cc58d5b33ae92bc037efd46b/test/cr-tryjob-picker_test.html
[modify] https://crrev.com/be1b6aafae6e95e1cc58d5b33ae92bc037efd46b/src/main/resources/static/cr-tryjob-picker.js

Project Member

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

The following revision refers to this bug:
  https://chromium.googlesource.com/infra/gerrit-plugins/buildbucket/+/be1b6aafae6e95e1cc58d5b33ae92bc037efd46b

commit be1b6aafae6e95e1cc58d5b33ae92bc037efd46b
Author: Andrii Shyshkalov <tandrii@chromium.org>
Date: Thu Mar 02 16:47:09 2017

Use patch_repository_url in Gerrit.

CQ used incorrect property before (fixed in
https://chrome-internal-review.googlesource.com/c/332743),
and that's why I mistakenly asked to use the wrong property here.

R=andybons@chromium.org
BUG= 645616 , 685318 

Change-Id: Ibe381f9937bc39bb0fba19c08f1a52724ac2ee54
Reviewed-on: https://chromium-review.googlesource.com/448459
Reviewed-by: Andrew Bonventre <andybons@chromium.org>

[modify] https://crrev.com/be1b6aafae6e95e1cc58d5b33ae92bc037efd46b/test/cr-tryjob-picker_test.html
[modify] https://crrev.com/be1b6aafae6e95e1cc58d5b33ae92bc037efd46b/src/main/resources/static/cr-tryjob-picker.js

Blockedon: v8:6109
Labels: -Milestone-Dogfood Milestone-Launch
Blockedon: 706135
Blockedon: 708152
Project Member

Comment 49 by bugdroid1@chromium.org, Apr 7 2017

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

commit ca02f065486627347da49805bd2638bedf5bea34
Author: Michael Achenbach <machenbach@chromium.org>
Date: Fri Apr 07 08:03:51 2017

V8: Make sanitizer-coverage bot gerrit aware

This calculates the correct patch base for gerrit patches, including dependent patches.

This also uploads the data to a separate Google Storage location.

Bug:  685318 , 643237
Change-Id: Id9c563698c50d67d7e5d780adef61c42a3079f6a
Reviewed-on: https://chromium-review.googlesource.com/469650
Commit-Queue: Michael Achenbach <machenbach@chromium.org>
Reviewed-by: Andrii Shyshkalov <tandrii@chromium.org>
Reviewed-by: Emma Soederberg <emso@google.com>

[modify] https://crrev.com/ca02f065486627347da49805bd2638bedf5bea34/scripts/slave/recipe_modules/v8/api.py
[modify] https://crrev.com/ca02f065486627347da49805bd2638bedf5bea34/scripts/slave/recipe_modules/v8/test_api.py
[modify] https://crrev.com/ca02f065486627347da49805bd2638bedf5bea34/scripts/slave/recipe_modules/v8/testing.py
[modify] https://crrev.com/ca02f065486627347da49805bd2638bedf5bea34/scripts/slave/recipes/v8.py
[add] https://crrev.com/ca02f065486627347da49805bd2638bedf5bea34/scripts/slave/recipes/v8.expected/full_tryserver_v8_v8_linux64_sanitizer_coverage_rel_gerrit.json
[modify] https://crrev.com/ca02f065486627347da49805bd2638bedf5bea34/scripts/slave/recipes/v8.expected/full_tryserver_v8_v8_linux_rel_ng_gerrit.json

Blockedon: v8:6482
Labels: -Milestone-Launch Milestone-Turndown
Since we don't have any major blockers, how about making gerrit the default now?
Blockedon: gerrit:6607 gerrit:6608
Marking two blockers as FYI. They don't really block switching, but would be nice to have.
Project Member

Comment 56 by bugdroid1@chromium.org, Jun 30 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/c746dcf9ecfe96b9bfeb3a87e80e76875b1679f8

commit c746dcf9ecfe96b9bfeb3a87e80e76875b1679f8
Author: Aaron Gable <agable@chromium.org>
Date: Fri Jun 30 17:37:37 2017

Make Gerrit the default code review for V8

Bug:  chromium:685318 
Change-Id: Ia603ad4a0a35bba5c5572cad32364ff3695b3a74
Reviewed-on: https://chromium-review.googlesource.com/558191
Commit-Queue: Aaron Gable <agable@chromium.org>
Reviewed-by: Michael Achenbach <machenbach@chromium.org>
Cr-Commit-Position: refs/heads/master@{#46368}
[modify] https://crrev.com/c746dcf9ecfe96b9bfeb3a87e80e76875b1679f8/codereview.settings

Comment 57 by agable@google.com, Jun 30 2017

Woo! I'm going to keep this bug open to track the following work:
* Marking V8 read-only in Rietveld
* Disabling V8's Rietveld CQ
* Removing the CQ's direct push permissions (since Gerrit uses Submit)
Then the migration will be truly complete.
Blockedon: 738679
Project Member

Comment 59 by bugdroid1@chromium.org, Jul 3 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/8840d622aa86b1d23c44e834e101e871e9b3b203

commit 8840d622aa86b1d23c44e834e101e871e9b3b203
Author: Michael Achenbach <machenbach@chromium.org>
Date: Mon Jul 03 13:41:18 2017

[release] Make gerrit the default for all release tools

This switches also the V8->Chromium rolls to Gerrit.

NOTRY=true
TBR=hablich@chromium.org

Bug:  chromium:685318 
Change-Id: Idc168f790541f09bd2f2d7c2f72806ac9e966843
Reviewed-on: https://chromium-review.googlesource.com/558913
Reviewed-by: Michael Achenbach <machenbach@chromium.org>
Commit-Queue: Michael Achenbach <machenbach@chromium.org>
Cr-Commit-Position: refs/heads/master@{#46388}
[modify] https://crrev.com/8840d622aa86b1d23c44e834e101e871e9b3b203/tools/release/common_includes.py
[modify] https://crrev.com/8840d622aa86b1d23c44e834e101e871e9b3b203/tools/release/git_recipes.py
[modify] https://crrev.com/8840d622aa86b1d23c44e834e101e871e9b3b203/tools/release/test_scripts.py

Project Member

Comment 60 by bugdroid1@chromium.org, Jul 3 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/ffbd92d7d2d9ef883e22e35fe046a89cec0080f4

commit ffbd92d7d2d9ef883e22e35fe046a89cec0080f4
Author: Michael Achenbach <machenbach@chromium.org>
Date: Mon Jul 03 13:46:18 2017

[release] Drop rietveld-reload feature from release tools

The release tools used to reload CL descriptions from rietveld to consider
late edits. This makes no sense anymore with gerrit, so we drop the feature.

NOTRY=true
TBR=hablich@chromium.org

Bug:  chromium:685318 
Change-Id: I08231795ba3b25c0939aa2b4428973086548484d
Reviewed-on: https://chromium-review.googlesource.com/558915
Reviewed-by: Michael Achenbach <machenbach@chromium.org>
Commit-Queue: Michael Achenbach <machenbach@chromium.org>
Cr-Commit-Position: refs/heads/master@{#46389}
[modify] https://crrev.com/ffbd92d7d2d9ef883e22e35fe046a89cec0080f4/tools/release/create_release.py
[modify] https://crrev.com/ffbd92d7d2d9ef883e22e35fe046a89cec0080f4/tools/release/push_to_candidates.py
[modify] https://crrev.com/ffbd92d7d2d9ef883e22e35fe046a89cec0080f4/tools/release/test_scripts.py

Project Member

Comment 61 by bugdroid1@chromium.org, Jul 3 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/72b5d5c00c67797e8513a3ce477de4158801d749

commit 72b5d5c00c67797e8513a3ce477de4158801d749
Author: Michael Achenbach <machenbach@chromium.org>
Date: Mon Jul 03 14:31:35 2017

Revert "[release] Make gerrit the default for all release tools"

This reverts commit 8840d622aa86b1d23c44e834e101e871e9b3b203.

Reason for revert: This requires the infra-side to be adapted to look
for open rolls. There's no such feature in the gerrit recipe_module
or git-cl tooling yet.

Original change's description:
> [release] Make gerrit the default for all release tools
> 
> This switches also the V8->Chromium rolls to Gerrit.
> 
> NOTRY=true
> TBR=hablich@chromium.org
> 
> Bug:  chromium:685318 
> Change-Id: Idc168f790541f09bd2f2d7c2f72806ac9e966843
> Reviewed-on: https://chromium-review.googlesource.com/558913
> Reviewed-by: Michael Achenbach <machenbach@chromium.org>
> Commit-Queue: Michael Achenbach <machenbach@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#46388}

TBR=machenbach@chromium.org,hablich@chromium.org,tandrii@chromium.org

Change-Id: I597538b6165b9952b5df9cde72466b95739cf56b
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  chromium:685318 
Reviewed-on: https://chromium-review.googlesource.com/558225
Reviewed-by: Michael Achenbach <machenbach@chromium.org>
Commit-Queue: Michael Achenbach <machenbach@chromium.org>
Cr-Commit-Position: refs/heads/master@{#46390}
[modify] https://crrev.com/72b5d5c00c67797e8513a3ce477de4158801d749/tools/release/common_includes.py
[modify] https://crrev.com/72b5d5c00c67797e8513a3ce477de4158801d749/tools/release/git_recipes.py
[modify] https://crrev.com/72b5d5c00c67797e8513a3ce477de4158801d749/tools/release/test_scripts.py

Project Member

Comment 64 by bugdroid1@chromium.org, Jul 10 2017

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

commit 9cb802ca48eb8f8ec22fe6f3f63fb29d33f4a7fd
Author: Michael Achenbach <machenbach@chromium.org>
Date: Mon Jul 10 08:42:27 2017

Improve gerrit recipe_module example

The /a suffix in the hots URL is not necessary as the gerrit_util adds it by
default to authenticate. Having it hard-coded in the example can be misleading.

Bug:  685318 
Change-Id: I333cd8b2aa9020aadfd186f2e18fbff0aa917681
Reviewed-on: https://chromium-review.googlesource.com/564611
Reviewed-by: Andrii Shyshkalov <tandrii@chromium.org>
Commit-Queue: Michael Achenbach <machenbach@chromium.org>

[modify] https://crrev.com/9cb802ca48eb8f8ec22fe6f3f63fb29d33f4a7fd/recipes/recipe_modules/gerrit/examples/full.expected/basic.json
[modify] https://crrev.com/9cb802ca48eb8f8ec22fe6f3f63fb29d33f4a7fd/recipes/recipe_modules/gerrit/examples/full.py

Project Member

Comment 67 by bugdroid1@chromium.org, Jul 10 2017

Project Member

Comment 68 by bugdroid1@chromium.org, Jul 10 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/22e808eb8d1ac460e2df3c31840294507532f47f

commit 22e808eb8d1ac460e2df3c31840294507532f47f
Author: Michael Achenbach <machenbach@chromium.org>
Date: Mon Jul 10 14:56:25 2017

[release] Explicitly use Gerrit in all release tools

Pass --gerrit explicitly to be resiliant to possible rollbacks of the Gerrit
switch.

This'll also enforce using Gerrit on older release branches when using
the release tools for cherry-picking.

NOTRY=true
TBR=hablich@chromium.org

Bug:  chromium:685318 
Change-Id: If60784b4c804f38ca36649ac4b2e62209d7cf729
Reviewed-on: https://chromium-review.googlesource.com/565415
Reviewed-by: Michael Achenbach <machenbach@chromium.org>
Commit-Queue: Michael Achenbach <machenbach@chromium.org>
Cr-Commit-Position: refs/heads/master@{#46523}
[modify] https://crrev.com/22e808eb8d1ac460e2df3c31840294507532f47f/tools/release/common_includes.py
[modify] https://crrev.com/22e808eb8d1ac460e2df3c31840294507532f47f/tools/release/git_recipes.py
[modify] https://crrev.com/22e808eb8d1ac460e2df3c31840294507532f47f/tools/release/test_scripts.py

Blockedon: 747014
Project Member

Comment 70 by bugdroid1@chromium.org, Jul 25 2017

Labels: merge-merged-6.0
The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/0622f419254b5fc301ee90030dd3ced1ee6a38b3

commit 0622f419254b5fc301ee90030dd3ced1ee6a38b3
Author: Aaron Gable <agable@chromium.org>
Date: Tue Jul 25 07:54:38 2017

Make Gerrit the default code review for V8

(cherry-picked from c746dcf9ecfe96b9bfeb3a87e80e76875b1679f8)

Bug:  chromium:685318 
Change-Id: Ia603ad4a0a35bba5c5572cad32364ff3695b3a74
Reviewed-on: https://chromium-review.googlesource.com/583789
Reviewed-by: Michael Achenbach <machenbach@chromium.org>
Cr-Commit-Position: refs/branch-heads/6.0@{#89}
Cr-Branched-From: 97dbf624a5eeffb3a8df36d24cdb2a883137385f-refs/heads/6.0.286@{#1}
Cr-Branched-From: 12e6f1cb5cd9616da7b9d4a7655c088778a6d415-refs/heads/master@{#45439}
[modify] https://crrev.com/0622f419254b5fc301ee90030dd3ced1ee6a38b3/codereview.settings

Blocking: 600469
Project Member

Comment 72 by bugdroid1@chromium.org, Jul 27 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/infra/infra/+/3674da8d7a2b89c2d36e5929d8031b8876dc558d

commit 3674da8d7a2b89c2d36e5929d8031b8876dc558d
Author: Aaron Gable <agable@chromium.org>
Date: Thu Jul 27 18:26:14 2017

Rietveld: refuse new changes to chromium and v8

Bug:  685318 , 685321 
Change-Id: I947abe065151e05debf3f624dc173ab224293e5f
Reviewed-on: https://chromium-review.googlesource.com/589813
Reviewed-by: Katie Thomas <katthomas@google.com>
Commit-Queue: Aaron Gable <agable@chromium.org>

[modify] https://crrev.com/3674da8d7a2b89c2d36e5929d8031b8876dc558d/appengine/chromium_rietveld/settings.py

Status: Fixed (was: Started)
Is there a clean up bug for doing general post-gerrit migration clean ups? E.g. removing the rietveld legacy from tons of recipe test cases...
The bugs for things like that are in Milestone-Turndown.

I don't think there is one for the V8 recipes in particular -- if you'd like to file one, that makes sense; if you'd like to continue using this one to track that work, that makes sense too.
Cc: -tandrii@chromium.org

Sign in to add a comment