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

Issue 709287 link

Starred by 1 user

Issue metadata

Status: Archived
Owner:
Closed: Apr 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug

Blocking:
issue 657224



Sign in to add a comment

Setting chrome_rev in chromite recipe is overly complicated.

Project Member Reported by dgarr...@chromium.org, Apr 6 2017

Issue description

Setting --chrome_rev in the chromite recipe is the last behavior that requires the recipe to parse the ChromeOS configuration file.

Since there is a configuration setting for this value, it seems easiest to just move it into the config, though that config change will require merging back to all active branches.
 
Blocking: 657224
Cc: d...@chromium.org bhthompson@chromium.org
Labels: Merge-Approved-57 Merge-Request-58 Merge-Request-56
After discussion with Bernie, the plan is to land my fix on TOT (where it has no effect), merge to R58 watch a few builds, then merge everywhere.

After it's fully rolled out, I'll retry to land my recipe change that depends on this.
Project Member

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

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

commit ec71d62b63764cd5b483dfd9641cf38d0e27567e
Author: Don Garrett <dgarrett@google.com>
Date: Fri Apr 07 05:27:03 2017

chromeos_config: Specify chrome_rev in pre-flight-branch.

We have been specifying the chrome_rev option on the command line via
a recipe configuration. Doing it in the build config like everything
else is much easier to mainatin.

After this is landed, and merged out to active branches, I'll remove
the recipe configuration.

BUG= chromium:709287 
TEST=Unittests.

Change-Id: Ia2b0b6f735eadae988f6edb9aad866f8c4d340d5
Reviewed-on: https://chromium-review.googlesource.com/470706
Commit-Ready: Don Garrett <dgarrett@chromium.org>
Tested-by: Don Garrett <dgarrett@chromium.org>
Reviewed-by: Bernie Thompson <bhthompson@chromium.org>

[modify] https://crrev.com/ec71d62b63764cd5b483dfd9641cf38d0e27567e/cbuildbot/config_dump.json
[modify] https://crrev.com/ec71d62b63764cd5b483dfd9641cf38d0e27567e/cbuildbot/chromeos_config.py

Project Member

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

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

commit ec71d62b63764cd5b483dfd9641cf38d0e27567e
Author: Don Garrett <dgarrett@google.com>
Date: Fri Apr 07 05:27:03 2017

chromeos_config: Specify chrome_rev in pre-flight-branch.

We have been specifying the chrome_rev option on the command line via
a recipe configuration. Doing it in the build config like everything
else is much easier to mainatin.

After this is landed, and merged out to active branches, I'll remove
the recipe configuration.

BUG= chromium:709287 
TEST=Unittests.

Change-Id: Ia2b0b6f735eadae988f6edb9aad866f8c4d340d5
Reviewed-on: https://chromium-review.googlesource.com/470706
Commit-Ready: Don Garrett <dgarrett@chromium.org>
Tested-by: Don Garrett <dgarrett@chromium.org>
Reviewed-by: Bernie Thompson <bhthompson@chromium.org>

[modify] https://crrev.com/ec71d62b63764cd5b483dfd9641cf38d0e27567e/cbuildbot/config_dump.json
[modify] https://crrev.com/ec71d62b63764cd5b483dfd9641cf38d0e27567e/cbuildbot/chromeos_config.py

Labels: -Merge-Approved-57 Merge-Request-57
Project Member

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

Labels: merge-merged-release-R58-9334.B
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/chromite/+/f58e59955d033b937a0b70165858ba0df0adf81a

commit f58e59955d033b937a0b70165858ba0df0adf81a
Author: Don Garrett <dgarrett@google.com>
Date: Fri Apr 07 19:05:47 2017

chromeos_config: Specify chrome_rev in pre-flight-branch.

We have been specifying the chrome_rev option on the command line via
a recipe configuration. Doing it in the build config like everything
else is much easier to mainatin.

After this is landed, and merged out to active branches, I'll remove
the recipe configuration.

BUG= chromium:709287 
TEST=Unittests.

Change-Id: Ia2b0b6f735eadae988f6edb9aad866f8c4d340d5
Reviewed-on: https://chromium-review.googlesource.com/470706
Commit-Ready: Don Garrett <dgarrett@chromium.org>
Tested-by: Don Garrett <dgarrett@chromium.org>
Reviewed-by: Bernie Thompson <bhthompson@chromium.org>
(cherry picked from commit ec71d62b63764cd5b483dfd9641cf38d0e27567e)
Reviewed-on: https://chromium-review.googlesource.com/471807
Reviewed-by: Don Garrett <dgarrett@chromium.org>
Commit-Queue: Don Garrett <dgarrett@chromium.org>

[modify] https://crrev.com/f58e59955d033b937a0b70165858ba0df0adf81a/cbuildbot/config_dump.json
[modify] https://crrev.com/f58e59955d033b937a0b70165858ba0df0adf81a/cbuildbot/chromeos_config.py

Project Member

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

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

commit f58e59955d033b937a0b70165858ba0df0adf81a
Author: Don Garrett <dgarrett@google.com>
Date: Fri Apr 07 19:05:47 2017

chromeos_config: Specify chrome_rev in pre-flight-branch.

We have been specifying the chrome_rev option on the command line via
a recipe configuration. Doing it in the build config like everything
else is much easier to mainatin.

After this is landed, and merged out to active branches, I'll remove
the recipe configuration.

BUG= chromium:709287 
TEST=Unittests.

Change-Id: Ia2b0b6f735eadae988f6edb9aad866f8c4d340d5
Reviewed-on: https://chromium-review.googlesource.com/470706
Commit-Ready: Don Garrett <dgarrett@chromium.org>
Tested-by: Don Garrett <dgarrett@chromium.org>
Reviewed-by: Bernie Thompson <bhthompson@chromium.org>
(cherry picked from commit ec71d62b63764cd5b483dfd9641cf38d0e27567e)
Reviewed-on: https://chromium-review.googlesource.com/471807
Reviewed-by: Don Garrett <dgarrett@chromium.org>
Commit-Queue: Don Garrett <dgarrett@chromium.org>

[modify] https://crrev.com/f58e59955d033b937a0b70165858ba0df0adf81a/cbuildbot/config_dump.json
[modify] https://crrev.com/f58e59955d033b937a0b70165858ba0df0adf81a/cbuildbot/chromeos_config.py

Project Member

Comment 8 by sheriffbot@chromium.org, Apr 8 2017

Labels: -Merge-Request-58 Hotlist-Merge-Approved Merge-Approved-58
Your change meets the bar and is auto-approved for M58. Please go ahead and merge the CL to branch 3029 manually. Please contact milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), bhthompson@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Owner: dgarr...@chromium.org
Status: Started (was: Untriaged)
Labels: -Merge-Request-56 -Merge-Request-57 Merge-Approved-57 Merge-Approved-56
56 is dead, and 57 has shipped its final stable, but as this is a build side change it should be ok.
Project Member

Comment 11 by bugdroid1@chromium.org, Apr 10 2017

Labels: merge-merged-release-R57-9202.B
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/chromite/+/74196f1b99b663c5b03ed61c3b8e6ab938f946a3

commit 74196f1b99b663c5b03ed61c3b8e6ab938f946a3
Author: Don Garrett <dgarrett@google.com>
Date: Mon Apr 10 23:26:34 2017

chromeos_config: Specify chrome_rev in pre-flight-branch.

We have been specifying the chrome_rev option on the command line via
a recipe configuration. Doing it in the build config like everything
else is much easier to mainatin.

After this is landed, and merged out to active branches, I'll remove
the recipe configuration.

BUG= chromium:709287 
TEST=Unittests.

Change-Id: Ia2b0b6f735eadae988f6edb9aad866f8c4d340d5
Reviewed-on: https://chromium-review.googlesource.com/470706
Commit-Ready: Don Garrett <dgarrett@chromium.org>
Tested-by: Don Garrett <dgarrett@chromium.org>
Reviewed-by: Bernie Thompson <bhthompson@chromium.org>
(cherry picked from commit ec71d62b63764cd5b483dfd9641cf38d0e27567e)
Reviewed-on: https://chromium-review.googlesource.com/474047
Reviewed-by: Don Garrett <dgarrett@chromium.org>
Commit-Queue: Don Garrett <dgarrett@chromium.org>

[modify] https://crrev.com/74196f1b99b663c5b03ed61c3b8e6ab938f946a3/cbuildbot/config_dump.json
[modify] https://crrev.com/74196f1b99b663c5b03ed61c3b8e6ab938f946a3/cbuildbot/chromeos_config.py

The R56 change does not cherry-pick cleanly. Since it's not important, I'm going to skip that merge. If it becomes important, I'll help resolve it.
Project Member

Comment 13 by bugdroid1@chromium.org, Apr 11 2017

Project Member

Comment 14 by sheriffbot@chromium.org, Apr 11 2017

This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible!

If all merges have been completed, please remove any remaining Merge-Approved labels from this issue.

Thanks for your time! To disable nags, add the Disable-Nags label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: -Merge-Approved-56 -Merge-Approved-57 -Merge-Approved-58
The recipe change to remove the --chrome_rev command line option is in place. Assuming that overnight builds worked correctly, this is fixed!
Status: Fixed (was: Started)

Comment 18 by dchan@google.com, May 30 2017

Labels: VerifyIn-60
Labels: VerifyIn-61

Comment 20 by dchan@chromium.org, Jan 22 2018

Status: Archived (was: Fixed)

Sign in to add a comment