cbuildbot, et al: More path_util for translation
In cbuildbot, we're half-heartedly using path_util:
* We use the default chroot settings in path_util.ToChrootPath(), even
though we were passed a buildroot; the "buildroot" is equivalent to
the source_path= parameter in path_util
* We're still doing plenty of os.path.join() to construct chroot paths.
Use path_util better, to be more reslient to path structure updates.
A few notes on some non-trivial changes:
* When mocking IsInsideChroot=False (to reflect actual usage, and to
ensure path_util's chroot-path translation actually activates), we
discover the test expectations in testBuildWithEnv() and friends were
wrong. We want to execute inside-chroot paths via cros_sdk, not
outside-chroot paths.
* BuildImageStageMock() was unconditionally mock-patching os.readlink().
With heavier use of link resolution (path_util), it's easy to run into
issues with this (symlink loops). Force in a little better filesystem
structure, so we can get by _BuildImages() without this mock.
* Fixing cbuildbot also requires fixing a few bits in Artifacts
services, so pull those (and a few other *_unittest.py I noticed)
while at it.
BUG=b:281721389, b:281704677, b:265885353
TEST=./run_tests
TEST=./run_tests with a non-default chroot path and without
/mnt/host/source/chroot/tmp existing; e.g.,
`sudo mount --bind /mnt/empty /mnt/host/source/chroot`
TEST=chromiumos-sdk tryjob
Change-Id: I90ffcc64448254b6c0c4382bd8a49fbd8433970a
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/chromite/+/4522312
Commit-Queue: Brian Norris <briannorris@chromium.org>
Tested-by: Brian Norris <briannorris@chromium.org>
Reviewed-by: Sergey Frolov <sfrolov@google.com>
diff --git a/api/controller/artifacts_unittest.py b/api/controller/artifacts_unittest.py
index faf7ab0..aba247b 100644
--- a/api/controller/artifacts_unittest.py
+++ b/api/controller/artifacts_unittest.py
@@ -85,13 +85,14 @@
osutils.SafeMakedirs(self.output_dir)
self.sysroot_path = "/build/target"
self.sysroot = sysroot_lib.Sysroot(self.sysroot_path)
- self.chroot_path = self.tempdir / "chroot"
- self.chroot_out_path = self.tempdir / "out"
- full_sysroot_path = os.path.join(
- self.chroot_path, self.sysroot_path.lstrip(os.sep)
+ self.chroot = chroot_lib.Chroot(
+ path=self.tempdir / "chroot",
+ out_path=self.tempdir / "out",
)
+ full_sysroot_path = self.chroot.full_path(self.sysroot_path)
osutils.SafeMakedirs(full_sysroot_path)
- osutils.SafeMakedirs(self.chroot_out_path)
+ osutils.SafeMakedirs(self.chroot.path)
+ osutils.SafeMakedirs(self.chroot.out_path)
# All requests use same response type.
self.response = artifacts_pb2.BundleResponse()
@@ -100,7 +101,7 @@
self.target_request = self.BuildTargetRequest(
build_target="target",
output_dir=self.output_dir,
- chroot=str(self.chroot_path),
+ chroot=self.chroot.path,
)
# Sysroot request.
@@ -108,8 +109,8 @@
sysroot=self.sysroot_path,
build_target="target",
output_dir=self.output_dir,
- chroot=self.chroot_path,
- chroot_out=self.chroot_out_path,
+ chroot=self.chroot.path,
+ chroot_out=self.chroot.out_path,
)
self.source_root = self.tempdir
@@ -292,7 +293,7 @@
def testInvalidOutputDir(self):
"""Test invalid output directory argument."""
request = self.SysrootRequest(
- chroot=self.chroot_path, sysroot=self.sysroot_path
+ chroot=self.chroot.path, sysroot=self.sysroot_path
)
with self.assertRaises(cros_build_lib.DieSystemExit):
@@ -303,7 +304,7 @@
def testInvalidSysroot(self):
"""Test no sysroot directory."""
request = self.SysrootRequest(
- chroot=self.chroot_path, output_dir=self.output_dir
+ chroot=self.chroot.path, output_dir=self.output_dir
)
with self.assertRaises(cros_build_lib.DieSystemExit):
@@ -314,7 +315,7 @@
def testSysrootDoesNotExist(self):
"""Test dies when no sysroot does not exist."""
request = self.SysrootRequest(
- chroot=self.chroot_path,
+ chroot=self.chroot.path,
sysroot="/does/not/exist",
output_dir=self.output_dir,
)
@@ -357,10 +358,6 @@
def testBundleTastFiles(self):
"""BundleTastFiles calls service correctly."""
- chroot = chroot_lib.Chroot(
- self.chroot_path, out_path=self.chroot_out_path
- )
-
expected_archive = os.path.join(
self.output_dir, artifacts_svc.TAST_BUNDLE_NAME
)
@@ -378,7 +375,7 @@
self.assertEqual(expected_archive, self.response.artifacts[0].path)
# Make sure the service got called correctly.
bundle_patch.assert_called_once_with(
- chroot, self.sysroot, self.output_dir
+ self.chroot, self.sysroot, self.output_dir
)
@@ -1118,15 +1115,19 @@
def setUp(self):
self.PatchObject(cros_build_lib, "IsInsideChroot", return_value=False)
- self.chroot_path = os.path.join(self.tempdir, "chroot")
- pathlib.Path(self.chroot_path).touch()
+ self.chroot = chroot_lib.Chroot(
+ path=self.tempdir / "chroot",
+ out_path=self.tempdir / "out",
+ )
+ pathlib.Path(self.chroot.path).touch()
+ self.chroot.out_path.touch()
self.expected_filepaths = [
- os.path.join(self.chroot_path, fp)
+ self.chroot.full_path(fp)
for fp in (
- "build/coral/usr/local/build/autotest/autotest_metadata.pb",
- "build/coral/usr/share/tast/metadata/local/cros.pb",
- "build/coral/build/share/tast/metadata/local/crosint.pb",
- "usr/share/tast/metadata/remote/cros.pb",
+ "/build/coral/usr/local/build/autotest/autotest_metadata.pb",
+ "/build/coral/usr/share/tast/metadata/local/cros.pb",
+ "/build/coral/build/share/tast/metadata/local/crosint.pb",
+ "/usr/share/tast/metadata/remote/cros.pb",
)
]
self.PatchObject(cros_build_lib, "AssertOutsideChroot")
@@ -1139,7 +1140,8 @@
if use_sysroot_path:
request.sysroot.path = self.sysroot_path
if use_chroot:
- request.chroot.path = self.chroot_path
+ request.chroot.path = self.chroot.path
+ request.chroot.out_path = str(self.chroot.out_path)
return request
def testValidateOnly(self):