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

Issue 717457 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Closed: May 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Feature

Blocking:
issue 713356



Sign in to add a comment

Move Chromium's gclient configs into build repo

Project Member Reported by machenb...@chromium.org, May 2 2017

Issue description

These configs are misplaced in depot_tools. Any changes to those configs require rolls with recipe expectation changes into build.

Those configs should move into the chromium recipe module in build, just like it's done for V8 and WebRTC, e.g.:
https://cs.chromium.org/chromium/build/scripts/slave/recipe_modules/v8/gclient_config.py

Just moving this isn't trivial as the engine disallows overwriting config items:
https://cs.chromium.org/chromium/infra/recipes-py/recipe_engine/config.py?q=recipe_engine/config.py+package:%5Echromium$&l=250

Options:
A) Make this move with a breaking roll, i.e.:
 - remove the configs in depot_tools in CL1
 - CL2 rolls CL1 into build (this will break)
 - copy the configs to recipe_modules/chromium/gclient_config.py in CL2 and roll it manually
B) 5-way dance
 - add new configs in build with different names, e.g. chromium_ng (copies of the configs in depot_tools)
 - change all uses of the configs to the new items (to find all these is not trivial)
 - remove in depot_tools
 - roll removal CL
 - add equal configs with the old names in build (side-by-side)
 - change all uses of the configs to the old items again
 - remove the new configs in build

Option A is usually discouraged, but in this case would save a tremendous amount of work. Besides the manual roll-CL into build, there should only be trivial rolls following.

Would be happy to do this myself, if there are no red flags against option A.
 
Project Member

Comment 1 by bugdroid1@chromium.org, May 4 2017

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

commit a469aeec53a5983550e4b6a61e569971cb583b36
Author: Michael Achenbach <machenbach@chromium.org>
Date: Thu May 04 12:52:50 2017

Remove Chromium's gclient configs from depot_tools

This directly depends on adding these configs to build:
https://chromium-review.googlesource.com/c/496187/

The CL adding the configs to build must also roll depot_tools to this CL.

In case of a revert, both CLs must be reverted.

Bug:  717457 
Change-Id: I458b52e184b668a3ace18ea0948363a2795f7161
Reviewed-on: https://chromium-review.googlesource.com/496107
Reviewed-by: Paweł Hajdan Jr. <phajdan.jr@chromium.org>
Commit-Queue: Michael Achenbach <machenbach@chromium.org>

[modify] https://crrev.com/a469aeec53a5983550e4b6a61e569971cb583b36/recipes/recipe_modules/gclient/example.py
[modify] https://crrev.com/a469aeec53a5983550e4b6a61e569971cb583b36/recipes/recipe_modules/gclient/config.py
[modify] https://crrev.com/a469aeec53a5983550e4b6a61e569971cb583b36/recipes/recipe_modules/gclient/tests/patch_project.py

Project Member

Comment 2 by bugdroid1@chromium.org, May 4 2017

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

commit b810b4628c808d13c4b6481db5693a1e6e0bd0d2
Author: Michael Achenbach <machenbach@chromium.org>
Date: Thu May 04 13:03:12 2017

Add Chromium's gclient configs

This directly depends on removing these configs from depot_tools:
https://chromium-review.googlesource.com/c/496107/

This also rolls depot_tools to the above CL.

In case of a revert, both CLs must be reverted.

Bug:  717457 
Change-Id: Ibb44bda63a96628f5b0e487c54118375e06391bb
Reviewed-on: https://chromium-review.googlesource.com/496187
Commit-Queue: Michael Achenbach <machenbach@chromium.org>
Reviewed-by: Paweł Hajdan Jr. <phajdan.jr@chromium.org>

[modify] https://crrev.com/b810b4628c808d13c4b6481db5693a1e6e0bd0d2/infra/config/recipes.cfg
[add] https://crrev.com/b810b4628c808d13c4b6481db5693a1e6e0bd0d2/scripts/slave/recipe_modules/chromium/tests/gclient.py
[add] https://crrev.com/b810b4628c808d13c4b6481db5693a1e6e0bd0d2/scripts/slave/recipe_modules/chromium/gclient_config.py

Project Member

Comment 3 by bugdroid1@chromium.org, May 4 2017

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

commit b810b4628c808d13c4b6481db5693a1e6e0bd0d2
Author: Michael Achenbach <machenbach@chromium.org>
Date: Thu May 04 13:03:12 2017

Add Chromium's gclient configs

This directly depends on removing these configs from depot_tools:
https://chromium-review.googlesource.com/c/496107/

This also rolls depot_tools to the above CL.

In case of a revert, both CLs must be reverted.

Bug:  717457 
Change-Id: Ibb44bda63a96628f5b0e487c54118375e06391bb
Reviewed-on: https://chromium-review.googlesource.com/496187
Commit-Queue: Michael Achenbach <machenbach@chromium.org>
Reviewed-by: Paweł Hajdan Jr. <phajdan.jr@chromium.org>

[modify] https://crrev.com/b810b4628c808d13c4b6481db5693a1e6e0bd0d2/infra/config/recipes.cfg
[add] https://crrev.com/b810b4628c808d13c4b6481db5693a1e6e0bd0d2/scripts/slave/recipe_modules/chromium/tests/gclient.py
[add] https://crrev.com/b810b4628c808d13c4b6481db5693a1e6e0bd0d2/scripts/slave/recipe_modules/chromium/gclient_config.py

Project Member

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

Owner: machenb...@chromium.org
Status: Started (was: Untriaged)
Project Member

Comment 6 by bugdroid1@chromium.org, May 5 2017

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

commit 6954e25b9a47ba250b912a867c98c3a75b3d4a40
Author: John Budorick <jbudorick@chromium.org>
Date: Fri May 05 17:06:04 2017

Add build/chromium dependency to rebaseline_o_matic recipe.

Change-Id: Ibf5b18e65b79c327fbf20524cc1fbf9afc4820dc
Bug:  717457 
Reviewed-on: https://chromium-review.googlesource.com/497689
Reviewed-by: Paweł Hajdan Jr. <phajdan.jr@chromium.org>
Commit-Queue: Paweł Hajdan Jr. <phajdan.jr@chromium.org>

[modify] https://crrev.com/6954e25b9a47ba250b912a867c98c3a75b3d4a40/recipes/recipes/rebaseline_o_matic.py

Project Member

Comment 7 by bugdroid1@chromium.org, May 5 2017

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

commit f1c30193b5c0ab5219d2438073a65cd3ec65ddfa
Author: Jeff Carpenter <jeffcarp@chromium.org>
Date: Fri May 05 17:17:24 2017

Add build/chromium dependency to wpt_import recipe.

Bug:  717457 ,  718927 
Change-Id: I4abad09acd544c6ac3a59af82eac2d381c6e4cde
Reviewed-on: https://chromium-review.googlesource.com/497312
Reviewed-by: Paweł Hajdan Jr. <phajdan.jr@chromium.org>
Commit-Queue: Paweł Hajdan Jr. <phajdan.jr@chromium.org>

[modify] https://crrev.com/f1c30193b5c0ab5219d2438073a65cd3ec65ddfa/recipes/recipes/wpt_import.py

Project Member

Comment 8 by bugdroid1@chromium.org, May 5 2017

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

commit 7628efe4da4fa0ec2288b00e5cb6de86887b66ba
Author: Jeff Carpenter <jeffcarp@chromium.org>
Date: Fri May 05 17:17:34 2017

Add build/chromium dependency to wpt_export recipe.

Bug:  717457 ,  718927 
Change-Id: I061132cba3e59efb0de2937b70b88406224290f4
Reviewed-on: https://chromium-review.googlesource.com/497488
Reviewed-by: Paweł Hajdan Jr. <phajdan.jr@chromium.org>
Commit-Queue: Paweł Hajdan Jr. <phajdan.jr@chromium.org>

[modify] https://crrev.com/7628efe4da4fa0ec2288b00e5cb6de86887b66ba/recipes/recipes/wpt_export.py

Project Member

Comment 9 by bugdroid1@chromium.org, May 8 2017

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

commit e39a9a46be39aac5fad2d66cf11ccdd6f8b9f081
Author: Michael Achenbach <machenbach@chromium.org>
Date: Mon May 08 09:38:25 2017

Migrate remaining chromium configs to build repo

This removes remaining configs that are tied to the
chromium recipe_module.

This directly depends on:
https://chromium-review.googlesource.com/c/497449/

In case of a revert, both CLs should be reverted.

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

[modify] https://crrev.com/e39a9a46be39aac5fad2d66cf11ccdd6f8b9f081/recipes/recipe_modules/gclient/example.py
[modify] https://crrev.com/e39a9a46be39aac5fad2d66cf11ccdd6f8b9f081/recipes/recipe_modules/gclient/config.py

Project Member

Comment 10 by bugdroid1@chromium.org, May 8 2017

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

commit b4a79690367881c427c9c5adf614823a01fc9990
Author: Michael Achenbach <machenbach@chromium.org>
Date: Mon May 08 10:00:03 2017

Add back helper method still used in build repo

Follow up fix for:
https://chromium-review.googlesource.com/c/497432

TBR=tandrii@chromium.org

Bug:  717457 
Change-Id: I66fb86f46cbdc6d9e34ce656541760af313beea8
Reviewed-on: https://chromium-review.googlesource.com/497410
Reviewed-by: Michael Achenbach <machenbach@chromium.org>
Commit-Queue: Michael Achenbach <machenbach@chromium.org>

[modify] https://crrev.com/b4a79690367881c427c9c5adf614823a01fc9990/recipes/recipe_modules/gclient/config.py

Project Member

Comment 11 by bugdroid1@chromium.org, May 8 2017

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

commit ff38188006831a2f281b68c091a0c02f3fd51ec1
Author: Michael Achenbach <machenbach@chromium.org>
Date: Mon May 08 10:07:58 2017

Migrate remaining chromium configs from depot_tools

This adds remaining configs from depot_tools that
are tied to the chromium recipe_module.

This directly depends on:
https://chromium-review.googlesource.com/c/497432/

In case of a revert, both CLs should be reverted.

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

[modify] https://crrev.com/ff38188006831a2f281b68c091a0c02f3fd51ec1/infra/config/recipes.cfg
[modify] https://crrev.com/ff38188006831a2f281b68c091a0c02f3fd51ec1/scripts/slave/recipe_modules/chromium/tests/gclient.py
[modify] https://crrev.com/ff38188006831a2f281b68c091a0c02f3fd51ec1/scripts/slave/recipe_modules/chromium/gclient_config.py

Status: Verified (was: Started)
This is done.

Sign in to add a comment