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

Issue 809634 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: May 2018
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 3
Type: Bug



Sign in to add a comment

refactor chroot management out of cros_build_lib

Project Member Reported by bmgordon@chromium.org, Feb 6 2018

Issue description

The chroot management stuff could be cleaned up if we were able to import other modules and generally be extracted from cros_build_lib.  It gets used from cros_sdk and the builders.
 
Also check callers and make the error return more consistent while refactoring this.  There is currently some confusion about whether returning False or raising an exception is the right way to indicate an error.
Components: Infra>Client>Chrome
Components: -Infra>Client>Chrome Infra>Client>ChromeOS
I don't think this is the correct component for this bug. Our team doesn't really know anything about cros_build_lib as far as I know. Maybe a typo?
Status: Started (was: Untriaged)
Project Member

Comment 5 by bugdroid1@chromium.org, May 16 2018

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

commit 121a2aa8872a2e6ca4edf1f64a2b407051917f06
Author: Benjamin Gordon <bmgordon@chromium.org>
Date: Wed May 16 19:41:31 2018

cros_test_lib: Refactor cros_build_lib mocks

A large number of unit tests import cros_build_lib_unittest to get
access to RunCommandMock and PopenMock.  Let's refactor these helper
classes out to start breaking up all the weird dependencies.

BUG= chromium:809634 
TEST=unit tests pass
CQ-DEPEND=CL:1060243

Change-Id: I99492246992f27a2fc792651dce0cbf965dbd0ff
Reviewed-on: https://chromium-review.googlesource.com/1045617
Commit-Ready: Benjamin Gordon <bmgordon@chromium.org>
Tested-by: Benjamin Gordon <bmgordon@chromium.org>
Reviewed-by: Benjamin Gordon <bmgordon@chromium.org>

[modify] https://crrev.com/121a2aa8872a2e6ca4edf1f64a2b407051917f06/mobmonitor/system/systeminfo_unittest.py
[modify] https://crrev.com/121a2aa8872a2e6ca4edf1f64a2b407051917f06/scripts/cros_mark_as_stable_unittest.py
[modify] https://crrev.com/121a2aa8872a2e6ca4edf1f64a2b407051917f06/scripts/autotest_quickmerge_unittest.py
[modify] https://crrev.com/121a2aa8872a2e6ca4edf1f64a2b407051917f06/cbuildbot/stages/branch_stages_unittest.py
[modify] https://crrev.com/121a2aa8872a2e6ca4edf1f64a2b407051917f06/cli/flash_unittest.py
[modify] https://crrev.com/121a2aa8872a2e6ca4edf1f64a2b407051917f06/cbuildbot/stages/sync_stages_unittest.py
[modify] https://crrev.com/121a2aa8872a2e6ca4edf1f64a2b407051917f06/lib/cros_test_lib.py
[modify] https://crrev.com/121a2aa8872a2e6ca4edf1f64a2b407051917f06/cbuildbot/prebuilts_unittest.py
[modify] https://crrev.com/121a2aa8872a2e6ca4edf1f64a2b407051917f06/scripts/cros_generate_breakpad_symbols_unittest.py
[modify] https://crrev.com/121a2aa8872a2e6ca4edf1f64a2b407051917f06/cbuildbot/stages/generic_stages_unittest.py
[modify] https://crrev.com/121a2aa8872a2e6ca4edf1f64a2b407051917f06/cbuildbot/repository_unittest.py
[modify] https://crrev.com/121a2aa8872a2e6ca4edf1f64a2b407051917f06/cbuildbot/swarming_lib_unittest.py
[modify] https://crrev.com/121a2aa8872a2e6ca4edf1f64a2b407051917f06/scripts/chrome_chromeos_lkgm_unittest.py
[modify] https://crrev.com/121a2aa8872a2e6ca4edf1f64a2b407051917f06/cbuildbot/stages/chrome_stages_unittest.py
[modify] https://crrev.com/121a2aa8872a2e6ca4edf1f64a2b407051917f06/lib/git_unittest.py
[modify] https://crrev.com/121a2aa8872a2e6ca4edf1f64a2b407051917f06/lib/gs_unittest.py
[modify] https://crrev.com/121a2aa8872a2e6ca4edf1f64a2b407051917f06/lib/path_util_unittest.py
[modify] https://crrev.com/121a2aa8872a2e6ca4edf1f64a2b407051917f06/lib/chrome_committer_unittest.py
[modify] https://crrev.com/121a2aa8872a2e6ca4edf1f64a2b407051917f06/cbuildbot/stages/test_stages_unittest.py
[modify] https://crrev.com/121a2aa8872a2e6ca4edf1f64a2b407051917f06/lib/remote_access_unittest.py
[modify] https://crrev.com/121a2aa8872a2e6ca4edf1f64a2b407051917f06/lib/gclient_unittest.py
[modify] https://crrev.com/121a2aa8872a2e6ca4edf1f64a2b407051917f06/lib/cros_build_lib_unittest.py
[modify] https://crrev.com/121a2aa8872a2e6ca4edf1f64a2b407051917f06/cli/cros/cros_chrome_sdk_unittest.py
[modify] https://crrev.com/121a2aa8872a2e6ca4edf1f64a2b407051917f06/lib/osutils_unittest.py
[modify] https://crrev.com/121a2aa8872a2e6ca4edf1f64a2b407051917f06/cbuildbot/stages/build_stages_unittest.py
[modify] https://crrev.com/121a2aa8872a2e6ca4edf1f64a2b407051917f06/cbuildbot/stages/artifact_stages_unittest.py
[modify] https://crrev.com/121a2aa8872a2e6ca4edf1f64a2b407051917f06/cros_bisect/autotest_evaluator_unittest.py
[modify] https://crrev.com/121a2aa8872a2e6ca4edf1f64a2b407051917f06/cbuildbot/stages/vm_test_stages_unittest.py
[modify] https://crrev.com/121a2aa8872a2e6ca4edf1f64a2b407051917f06/lib/cros_test_lib_unittest.py
[modify] https://crrev.com/121a2aa8872a2e6ca4edf1f64a2b407051917f06/lib/commandline_unittest.py
[modify] https://crrev.com/121a2aa8872a2e6ca4edf1f64a2b407051917f06/cli/command_unittest.py
[modify] https://crrev.com/121a2aa8872a2e6ca4edf1f64a2b407051917f06/scripts/cbuildbot_launch_unittest.py
[modify] https://crrev.com/121a2aa8872a2e6ca4edf1f64a2b407051917f06/lib/image_lib_unittest.py
[modify] https://crrev.com/121a2aa8872a2e6ca4edf1f64a2b407051917f06/lib/loas_unittest.py
[modify] https://crrev.com/121a2aa8872a2e6ca4edf1f64a2b407051917f06/cbuildbot/manifest_version_unittest.py
[modify] https://crrev.com/121a2aa8872a2e6ca4edf1f64a2b407051917f06/cros_bisect/simple_chrome_builder_unittest.py
[modify] https://crrev.com/121a2aa8872a2e6ca4edf1f64a2b407051917f06/lib/toolchain_unittest.py
[modify] https://crrev.com/121a2aa8872a2e6ca4edf1f64a2b407051917f06/lib/chroot_util_unittest.py
[modify] https://crrev.com/121a2aa8872a2e6ca4edf1f64a2b407051917f06/cbuildbot/cbuildbot_unittest.py
[modify] https://crrev.com/121a2aa8872a2e6ca4edf1f64a2b407051917f06/lib/patch_unittest.py
[modify] https://crrev.com/121a2aa8872a2e6ca4edf1f64a2b407051917f06/cbuildbot/commands_unittest.py

Project Member

Comment 6 by bugdroid1@chromium.org, May 16 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/firmware/+/a00780d36db4d4270a59d0f134d096c9305174dd

commit a00780d36db4d4270a59d0f134d096c9305174dd
Author: Benjamin Gordon <bmgordon@chromium.org>
Date: Wed May 16 19:41:31 2018

pack_firmware_unittest: Refactor cros_build_lib mocks

RunCommandMock is moving from cros_build_lib_unittest into
cros_test_lib.

BUG= chromium:809634 
TEST=cros_run_unit_tests --board=coral --packages chromeos-base/chromeos-firmware-coral
CQ-DEPEND=CL:1045617

Change-Id: Ic48e0c0872ef69ba5e6bd0cc8e2cbbe5ebfdff72
Reviewed-on: https://chromium-review.googlesource.com/1060243
Commit-Ready: Benjamin Gordon <bmgordon@chromium.org>
Tested-by: Benjamin Gordon <bmgordon@chromium.org>
Reviewed-by: Jason Clinton <jclinton@chromium.org>
Reviewed-by: Don Garrett <dgarrett@chromium.org>

[modify] https://crrev.com/a00780d36db4d4270a59d0f134d096c9305174dd/pack_firmware_unittest.py

Project Member

Comment 7 by bugdroid1@chromium.org, May 16 2018

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

commit 4f522e01e2856828612aec72e303d3b870a78515
Author: chromite-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com <chromite-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Date: Wed May 16 22:42:12 2018

Roll src/third_party/chromite/ 5f638a109..4c1562dfd (3 commits)

https://chromium.googlesource.com/chromiumos/chromite.git/+log/5f638a1091fb..4c1562dfdeef

$ git log 5f638a109..4c1562dfd --date=short --no-merges --format='%ad %ae %s'
2018-05-14 shapiroc cbuildbot: Fix file check outside chroot
2018-05-04 bmgordon cros_test_lib: Refactor cros_build_lib mocks
2018-05-12 yunlian cros_setup_toolchain: build toolchains in reversed order.

Created with:
  roll-dep src/third_party/chromite
BUG= chromium:842834 , chromium:809634 ,chromium:711369


The AutoRoll server is located here: https://chromite-chromium-roll.skia.org

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

If the roll is causing failures, please contact the current sheriff, who should
be CC'd on the roll, and stop the roller if necessary.


TBR=chrome-os-gardeners@chromium.org

Change-Id: Icd8ebd153995e7b541c78dc08f8072239c16a140
Reviewed-on: https://chromium-review.googlesource.com/1062649
Reviewed-by: Chromite Chromium Autoroll <chromite-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Commit-Queue: Chromite Chromium Autoroll <chromite-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Cr-Commit-Position: refs/heads/master@{#559319}
[modify] https://crrev.com/4f522e01e2856828612aec72e303d3b870a78515/DEPS

Project Member

Comment 8 by bugdroid1@chromium.org, May 17 2018

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

commit 74645231834e82d664791c34f29c63d7e0933270
Author: Benjamin Gordon <bmgordon@chromium.org>
Date: Thu May 17 10:24:21 2018

cros_sdk_lib: Move chroot functions out of cros_build_lib

This lets us start breaking the circular dependencies between
cros_build_lib and osutils.  No functionality change other than
importing from the new module.

BUG= chromium:809634 
TEST=unit tests pass; tryjobs

Change-Id: Icb3468e4322784d16368c1039fb505dc71089831
Reviewed-on: https://chromium-review.googlesource.com/1045618
Commit-Ready: Benjamin Gordon <bmgordon@chromium.org>
Tested-by: Benjamin Gordon <bmgordon@chromium.org>
Reviewed-by: Don Garrett <dgarrett@chromium.org>

[add] https://crrev.com/74645231834e82d664791c34f29c63d7e0933270/lib/cros_sdk_lib.py
[modify] https://crrev.com/74645231834e82d664791c34f29c63d7e0933270/cbuildbot/stages/build_stages_unittest.py
[add] https://crrev.com/74645231834e82d664791c34f29c63d7e0933270/lib/cros_sdk_lib_unittest.py
[modify] https://crrev.com/74645231834e82d664791c34f29c63d7e0933270/cbuildbot/chroot_lib_unittest.py
[modify] https://crrev.com/74645231834e82d664791c34f29c63d7e0933270/scripts/cros_sdk_unittest.py
[modify] https://crrev.com/74645231834e82d664791c34f29c63d7e0933270/lib/cros_build_lib.py
[modify] https://crrev.com/74645231834e82d664791c34f29c63d7e0933270/scripts/cbuildbot_launch_unittest.py
[modify] https://crrev.com/74645231834e82d664791c34f29c63d7e0933270/cbuildbot/chroot_lib.py
[modify] https://crrev.com/74645231834e82d664791c34f29c63d7e0933270/scripts/cbuildbot_launch.py
[modify] https://crrev.com/74645231834e82d664791c34f29c63d7e0933270/scripts/cros_sdk.py
[modify] https://crrev.com/74645231834e82d664791c34f29c63d7e0933270/lib/cros_build_lib_unittest.py
[add] https://crrev.com/74645231834e82d664791c34f29c63d7e0933270/lib/cros_sdk_lib_unittest
[modify] https://crrev.com/74645231834e82d664791c34f29c63d7e0933270/cbuildbot/stages/build_stages.py

Project Member

Comment 9 by bugdroid1@chromium.org, May 17 2018

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

commit 967fde3df89a8c5dc9946ce0d6045dc26785940e
Author: chromite-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com <chromite-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Date: Thu May 17 12:17:52 2018

Roll src/third_party/chromite/ 84d0e6526..be1ac9516 (4 commits)

https://chromium.googlesource.com/chromiumos/chromite.git/+log/84d0e6526470..be1ac951607e

$ git log 84d0e6526..be1ac9516 --date=short --no-merges --format='%ad %ae %s'
2018-05-15 yunlian Test ThinLTO on peach_pit
2018-05-16 dgarrett chromeos_config: Remove trybot_list.
2018-05-14 achuith cros_run_vm_test: Introduce Run()
2018-05-04 bmgordon cros_sdk_lib: Move chroot functions out of cros_build_lib

Created with:
  roll-dep src/third_party/chromite
BUG=chromium:None,chromium:None,chromium:828749,chromium:809634


The AutoRoll server is located here: https://chromite-chromium-roll.skia.org

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

If the roll is causing failures, please contact the current sheriff, who should
be CC'd on the roll, and stop the roller if necessary.


TBR=chrome-os-gardeners@chromium.org

Change-Id: I8d7668d4891451a613c52552f203a10c0dbc17f6
Reviewed-on: https://chromium-review.googlesource.com/1063991
Reviewed-by: Chromite Chromium Autoroll <chromite-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Commit-Queue: Chromite Chromium Autoroll <chromite-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Cr-Commit-Position: refs/heads/master@{#559498}
[modify] https://crrev.com/967fde3df89a8c5dc9946ce0d6045dc26785940e/DEPS

Status: Fixed (was: Started)

Sign in to add a comment