New issue
Advanced search Search tips

Issue 886944 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jan 9
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocked on:
issue 855291

Blocking:
issue 887130



Sign in to add a comment

Support cherry-picking CLs into workspace builds.

Project Member Reported by dgarr...@chromium.org, Sep 19

Issue description

The new build configs for firmware branch builds have two different ChromeOS checkouts. This causes confusion about where and how cherry-picked changes should be applied.

Specifically, we want chromite and build related CLs to be applied to the "build checkout", and we want firmware related CLs to be applied to the "firmware" checkout.

After some discussion, the plan is to implement two "smart" sync stages that know how to select the relevant CLs based on project, and/or by branch.

This is not considered a blocker for putting firmwarebranch builds into production, because the old tryjob mechanisms are in place, and because it's believed that most firmware developers build locally, not through tryjobs, most of the time.
 
Blockedon: 855291
Blocking: 887130
Summary: Support cherry-picking CLs into workspace builds. (was: Support cherry-picking CLs into firmwarebranch builds.)
I have a design coming together that solves this and a variety of other related problems.

Create a new InfraWorkspace sync stage that is a normal sync stage known to the builder.

It syncs both the infra, and the workspace.

Sync related command line options, and build config values are directly handled by this stage, ie:
   --branch, --version, --gerrit-patches, etc.

It will (mostly) pass those along to the workspace sync, not the infra sync, with a special filter for chromite CLs. Infra will be hard coded to the 'master' branch.

The sync stage is what determines and reports the version number sync'd, so this stage will be able to correctly report the version sync'd (though, NOT the version after the uprevs).

The new config option "workspace_branch" can be deleted, and the standard "branch" option can be used instead.

The upside is that our standard artifact handling, and reporting mechanisms will be much more correct.

One downside to that is Legoland. The current "firmware" section is filtered by the branch "master". If we can get it to use the display_label "firmware" but ignore the branch, everything should work the way it does now.
Project Member

Comment 6 by bugdroid1@chromium.org, Nov 16

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/chromite/+/6521871a0890bbcbbb4d9eb52d9aecccabf311e9

commit 6521871a0890bbcbbb4d9eb52d9aecccabf311e9
Author: Don Garrett <dgarrett@google.com>
Date: Fri Nov 16 13:01:36 2018

CleanUpStage: Cleanup Workspaces.

Move workspace specific logic to the generic CleanUpStage. This
replaces the WorkspaceCleanupStage. This ensures that the workspace
has been cleaned up before the general Sync stage runs.

This is prep work for combining the general sync and workspace sync's
into a single Sync stage so that we can intelligently decide which CLs
to cherry-pick where.

BUG= chromium:886944 
TEST=run_tests &&
     cros tryjob --cbuildbot --pass-through='--workspace=<dir>' \
       firmware-nami-10775.B-firmwarebranch-tryjob

Change-Id: I8dbe675b6b1e335bcfe38b80378166c81ba4f5ee
Reviewed-on: https://chromium-review.googlesource.com/1271368
Commit-Ready: Don Garrett <dgarrett@chromium.org>
Tested-by: Don Garrett <dgarrett@chromium.org>
Reviewed-by: Alec Thilenius <athilenius@google.com>

[modify] https://crrev.com/6521871a0890bbcbbb4d9eb52d9aecccabf311e9/cbuildbot/stages/workspace_stages.py
[modify] https://crrev.com/6521871a0890bbcbbb4d9eb52d9aecccabf311e9/cbuildbot/stages/build_stages.py
[modify] https://crrev.com/6521871a0890bbcbbb4d9eb52d9aecccabf311e9/cbuildbot/builders/workspace_builders.py

Project Member

Comment 7 by bugdroid1@chromium.org, Nov 16

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/chromite/+/354e410b4ef6110657645442733d3a9de4fba715

commit 354e410b4ef6110657645442733d3a9de4fba715
Author: Don Garrett <dgarrett@google.com>
Date: Fri Nov 16 13:01:37 2018

generic_stage.GetRepoRepository: Make arguments flexible.

GetRepoRepository work correctly for workspace checkouts, and make the
arguments to it more flexible, so that it can be used for the infra
checkout of workspace builds.

BUG= chromium:886944 
TEST=run_tests

Change-Id: I62693edcc12fd807cf49d9396bf9737cb1177aa6
Reviewed-on: https://chromium-review.googlesource.com/1334709
Commit-Ready: Don Garrett <dgarrett@chromium.org>
Tested-by: Don Garrett <dgarrett@chromium.org>
Reviewed-by: Alex Klein <saklein@chromium.org>

[modify] https://crrev.com/354e410b4ef6110657645442733d3a9de4fba715/cbuildbot/stages/workspace_stages.py
[modify] https://crrev.com/354e410b4ef6110657645442733d3a9de4fba715/cbuildbot/stages/generic_stages.py

Project Member

Comment 8 by bugdroid1@chromium.org, Nov 16

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/chromite/+/df19e52bd2a4480107160f674049ef9a3f05d516

commit df19e52bd2a4480107160f674049ef9a3f05d516
Author: Don Garrett <dgarrett@chromium.org>
Date: Fri Nov 16 22:44:47 2018

Revert "generic_stage.GetRepoRepository: Make arguments flexible."

This reverts commit 354e410b4ef6110657645442733d3a9de4fba715.

Reason for revert:  https://crbug.com/906241 

Original change's description:
> generic_stage.GetRepoRepository: Make arguments flexible.
> 
> GetRepoRepository work correctly for workspace checkouts, and make the
> arguments to it more flexible, so that it can be used for the infra
> checkout of workspace builds.
> 
> BUG= chromium:886944 
> TEST=run_tests
> 
> Change-Id: I62693edcc12fd807cf49d9396bf9737cb1177aa6
> Reviewed-on: https://chromium-review.googlesource.com/1334709
> Commit-Ready: Don Garrett <dgarrett@chromium.org>
> Tested-by: Don Garrett <dgarrett@chromium.org>
> Reviewed-by: Alex Klein <saklein@chromium.org>

Bug:  chromium:886944 
Change-Id: I3612c75c997310e3d39c6fa9bb73670e4e5039ff
Reviewed-on: https://chromium-review.googlesource.com/c/1340652
Reviewed-by: Don Garrett <dgarrett@chromium.org>
Commit-Queue: Don Garrett <dgarrett@chromium.org>
Tested-by: Don Garrett <dgarrett@chromium.org>

[modify] https://crrev.com/df19e52bd2a4480107160f674049ef9a3f05d516/cbuildbot/stages/workspace_stages.py
[modify] https://crrev.com/df19e52bd2a4480107160f674049ef9a3f05d516/cbuildbot/stages/generic_stages.py

Project Member

Comment 9 by bugdroid1@chromium.org, Nov 16

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/chromite/+/b14d723e35d472169ca9227cfd4dd1fcf23fc0e8

commit b14d723e35d472169ca9227cfd4dd1fcf23fc0e8
Author: Don Garrett <dgarrett@chromium.org>
Date: Fri Nov 16 22:45:51 2018

Revert "CleanUpStage: Cleanup Workspaces."

This reverts commit 6521871a0890bbcbbb4d9eb52d9aecccabf311e9.

Reason for revert:  https://crbug.com/906241 

Original change's description:
> CleanUpStage: Cleanup Workspaces.
> 
> Move workspace specific logic to the generic CleanUpStage. This
> replaces the WorkspaceCleanupStage. This ensures that the workspace
> has been cleaned up before the general Sync stage runs.
> 
> This is prep work for combining the general sync and workspace sync's
> into a single Sync stage so that we can intelligently decide which CLs
> to cherry-pick where.
> 
> BUG= chromium:886944 
> TEST=run_tests &&
>      cros tryjob --cbuildbot --pass-through='--workspace=<dir>' \
>        firmware-nami-10775.B-firmwarebranch-tryjob
> 
> Change-Id: I8dbe675b6b1e335bcfe38b80378166c81ba4f5ee
> Reviewed-on: https://chromium-review.googlesource.com/1271368
> Commit-Ready: Don Garrett <dgarrett@chromium.org>
> Tested-by: Don Garrett <dgarrett@chromium.org>
> Reviewed-by: Alec Thilenius <athilenius@google.com>

Bug:  chromium:886944 
Change-Id: Ia145759cdf4bc9f871a05dd00564603b7d8484b8
Reviewed-on: https://chromium-review.googlesource.com/c/1340729
Reviewed-by: Don Garrett <dgarrett@chromium.org>
Commit-Queue: Don Garrett <dgarrett@chromium.org>
Tested-by: Don Garrett <dgarrett@chromium.org>

[modify] https://crrev.com/b14d723e35d472169ca9227cfd4dd1fcf23fc0e8/cbuildbot/stages/workspace_stages.py
[modify] https://crrev.com/b14d723e35d472169ca9227cfd4dd1fcf23fc0e8/cbuildbot/stages/build_stages.py
[modify] https://crrev.com/b14d723e35d472169ca9227cfd4dd1fcf23fc0e8/cbuildbot/builders/workspace_builders.py

Project Member

Comment 10 by bugdroid1@chromium.org, Nov 17

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/chromite/+/8b0273016734c8ce2ba74177fe8dd3ab832de1d6

commit 8b0273016734c8ce2ba74177fe8dd3ab832de1d6
Author: Don Garrett <dgarrett@google.com>
Date: Sat Nov 17 00:17:25 2018

Reland "CleanUpStage: Cleanup Workspaces."

This is a reland of 6521871a0890bbcbbb4d9eb52d9aecccabf311e9

Original change's description:
> CleanUpStage: Cleanup Workspaces.
>
> Move workspace specific logic to the generic CleanUpStage. This
> replaces the WorkspaceCleanupStage. This ensures that the workspace
> has been cleaned up before the general Sync stage runs.
>
> This is prep work for combining the general sync and workspace sync's
> into a single Sync stage so that we can intelligently decide which CLs
> to cherry-pick where.
>
> BUG= chromium:886944 
> TEST=run_tests &&
>      cros tryjob --cbuildbot --pass-through='--workspace=<dir>' \
>        firmware-nami-10775.B-firmwarebranch-tryjob
>
> Change-Id: I8dbe675b6b1e335bcfe38b80378166c81ba4f5ee
> Reviewed-on: https://chromium-review.googlesource.com/1271368
> Commit-Ready: Don Garrett <dgarrett@chromium.org>
> Tested-by: Don Garrett <dgarrett@chromium.org>
> Reviewed-by: Alec Thilenius <athilenius@google.com>

BUG= chromium:886944 
TEST=cros tryjob --cbuildbot prototype-factorybranch-tryjob
       --pass-through="--workspace=<dir>"

Change-Id: Ide7ebd5c01e1657a94c12af34d5ddd6ce314e5be
Reviewed-on: https://chromium-review.googlesource.com/c/1340730
Tested-by: Don Garrett <dgarrett@chromium.org>
Trybot-Ready: Don Garrett <dgarrett@chromium.org>
Reviewed-by: Don Garrett <dgarrett@chromium.org>
Commit-Queue: Don Garrett <dgarrett@chromium.org>

[modify] https://crrev.com/8b0273016734c8ce2ba74177fe8dd3ab832de1d6/cbuildbot/stages/workspace_stages.py
[modify] https://crrev.com/8b0273016734c8ce2ba74177fe8dd3ab832de1d6/cbuildbot/stages/build_stages.py
[modify] https://crrev.com/8b0273016734c8ce2ba74177fe8dd3ab832de1d6/cbuildbot/builders/workspace_builders.py

Project Member

Comment 11 by bugdroid1@chromium.org, Nov 28

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/chromite/+/23537c6f8297964489dd0a8ba316246bc6fd22ae

commit 23537c6f8297964489dd0a8ba316246bc6fd22ae
Author: Don Garrett <dgarrett@google.com>
Date: Wed Nov 28 03:14:03 2018

Reland generic_stage.GetRepoRepository: Make arguments flexible.

GetRepoRepository work correctly for workspace checkouts, and make the
arguments to it more flexible, so that it can be used for the infra
checkout of workspace builds.

BUG= chromium:886944 
TEST=run_tests

Change-Id: I89fa77f5ce229305164a6acd6383f747b9ccba13
Reviewed-on: https://chromium-review.googlesource.com/1340737
Commit-Ready: Don Garrett <dgarrett@chromium.org>
Tested-by: Don Garrett <dgarrett@chromium.org>
Reviewed-by: Mike Frysinger <vapier@chromium.org>

[modify] https://crrev.com/23537c6f8297964489dd0a8ba316246bc6fd22ae/cbuildbot/stages/workspace_stages.py
[modify] https://crrev.com/23537c6f8297964489dd0a8ba316246bc6fd22ae/cbuildbot/stages/generic_stages.py

Project Member

Comment 12 by bugdroid1@chromium.org, Nov 28

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/chromite/+/0eed05c79fc24959076b4416ae7be8d2cbb19ba2

commit 0eed05c79fc24959076b4416ae7be8d2cbb19ba2
Author: Don Garrett <dgarrett@chromium.org>
Date: Wed Nov 28 21:23:38 2018

Revert "Reland generic_stage.GetRepoRepository: Make arguments flexible."

This reverts commit 23537c6f8297964489dd0a8ba316246bc6fd22ae.

Reason for revert: Causing firmware versions to update to TOT version numbers.

Original change's description:
> Reland generic_stage.GetRepoRepository: Make arguments flexible.
> 
> GetRepoRepository work correctly for workspace checkouts, and make the
> arguments to it more flexible, so that it can be used for the infra
> checkout of workspace builds.
> 
> BUG= chromium:886944 
> TEST=run_tests
> 
> Change-Id: I89fa77f5ce229305164a6acd6383f747b9ccba13
> Reviewed-on: https://chromium-review.googlesource.com/1340737
> Commit-Ready: Don Garrett <dgarrett@chromium.org>
> Tested-by: Don Garrett <dgarrett@chromium.org>
> Reviewed-by: Mike Frysinger <vapier@chromium.org>

Bug:  chromium:886944 
Change-Id: Ica6a8dc9f9d15838d68d1355b04279dbae8c1173
Reviewed-on: https://chromium-review.googlesource.com/c/1354380
Reviewed-by: Don Garrett <dgarrett@chromium.org>
Commit-Queue: Don Garrett <dgarrett@chromium.org>
Tested-by: Don Garrett <dgarrett@chromium.org>

[modify] https://crrev.com/0eed05c79fc24959076b4416ae7be8d2cbb19ba2/cbuildbot/stages/workspace_stages.py
[modify] https://crrev.com/0eed05c79fc24959076b4416ae7be8d2cbb19ba2/cbuildbot/stages/generic_stages.py

Project Member

Comment 13 by bugdroid1@chromium.org, Dec 4

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/chromite/+/6f87fb214b3c6da1198206a75c7ac9411249a59b

commit 6f87fb214b3c6da1198206a75c7ac9411249a59b
Author: Don Garrett <dgarrett@google.com>
Date: Tue Dec 04 22:06:10 2018

generic_stage.GetRepoRepository: Make arguments flexible.

Allow all arguments to be passed in explicitly, with defaults that
match the previously hard coded values.

BUG= chromium:886944 
TEST=run_tests

Change-Id: Icd74857901961ba6fd32591ff2e793b11d7b6900
Reviewed-on: https://chromium-review.googlesource.com/c/1359018
Commit-Queue: Don Garrett <dgarrett@chromium.org>
Tested-by: Don Garrett <dgarrett@chromium.org>
Reviewed-by: Alec Thilenius <athilenius@google.com>

[modify] https://crrev.com/6f87fb214b3c6da1198206a75c7ac9411249a59b/cbuildbot/stages/workspace_stages.py
[modify] https://crrev.com/6f87fb214b3c6da1198206a75c7ac9411249a59b/cbuildbot/stages/generic_stages.py

Project Member

Comment 14 by bugdroid1@chromium.org, Jan 8

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/chromite/+/d4fa134d2ed2605ea08bf4f2175c56e032247731

commit d4fa134d2ed2605ea08bf4f2175c56e032247731
Author: Don Garrett <dgarrett@google.com>
Date: Tue Jan 08 03:40:33 2019

workspace_sync: Support cherry-picking CLs.

Add cherry-pick support to the workspace sync stage. This sync is
smart enough to apply chromite changes to the infra checkout, and
other changes to the workspace checkout. It is NOT smart enough to do
the right thing with manifest CLs.

BUG= chromium:886944 
TEST=run_tests
     cros tryjob --cbuildbot --pass-through="--workspace=<dir>"
       prototype-buildspec-tryjob -g 1380851

Change-Id: Ic2b466f6b1ba2e333015c1929e332a36164e1816
Reviewed-on: https://chromium-review.googlesource.com/1387174
Commit-Ready: Don Garrett <dgarrett@chromium.org>
Tested-by: Don Garrett <dgarrett@chromium.org>
Reviewed-by: Lann Martin <lannm@chromium.org>
Reviewed-by: Don Garrett <dgarrett@chromium.org>

[modify] https://crrev.com/d4fa134d2ed2605ea08bf4f2175c56e032247731/config/chromeos_config.py
[modify] https://crrev.com/d4fa134d2ed2605ea08bf4f2175c56e032247731/cbuildbot/stages/workspace_stages.py
[modify] https://crrev.com/d4fa134d2ed2605ea08bf4f2175c56e032247731/config/config_dump.json

Project Member

Comment 15 by bugdroid1@chromium.org, Jan 8

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/chromite/+/f3976f98e03affd2aa6cfbc86b48004760d2c6a6

commit f3976f98e03affd2aa6cfbc86b48004760d2c6a6
Author: Don Garrett <dgarrett@google.com>
Date: Tue Jan 08 03:40:33 2019

repo_sync_manifest: Raise patching errors.

If PatchSeries.Apply failed with some patches, we were ignoring the
errors. Raise the first one hit, and ignore the rest.

BUG= chromium:886944 
TEST=cros tryjob --cbuildbot into workspace with invalid CL.

Change-Id: I7d9e89ed4d34da66923695e0c015b3c9d21c20f2
Reviewed-on: https://chromium-review.googlesource.com/1387866
Commit-Ready: Don Garrett <dgarrett@chromium.org>
Tested-by: Don Garrett <dgarrett@chromium.org>
Reviewed-by: Lann Martin <lannm@chromium.org>

[modify] https://crrev.com/f3976f98e03affd2aa6cfbc86b48004760d2c6a6/scripts/repo_sync_manifest.py

Status: Fixed (was: Started)

Sign in to add a comment