cros_flash: Fix the issue with /test xbuddy path
Currently there is some weirdness in the implementation of xbuddy path
translations and unfortunately the xbuddy links like
remote/reef/latest/test is broken. This CL simplifies the path
translation a bit better.
Also deprecates TranslatedPathToLocalPath.
BUG=b:172083615
TEST=both inside and outside chroot:
cros flash --debug --no-ping --no-wipe --no-reboot [$dut] xbuddy://remote/reef/latest-dev
cros flash --debug --no-ping --no-wipe --no-reboot [$dut] xbuddy://remote/reef/latest-dev/test
cros flash --debug --no-ping --no-wipe --no-reboot [$dut] xbuddy://local/reef/latest/
cros analyze-image --debug --board=reef --version=R88-13575.0.0
TEST=inside chromium checkout
~/chromium/src/third_party/chromite $ ./bin/cros flash --debug --no-ping --no-wipe --no-reboot [$dut] xbuddy://remote/latest
Change-Id: I0b77472f5ef1f8f4d5475e94a1215154b085096f
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/chromite/+/2515359
Tested-by: Amin Hassani <ahassani@chromium.org>
Reviewed-by: Achuith Bhandarkar <achuith@chromium.org>
Reviewed-by: Mike Frysinger <vapier@chromium.org>
Commit-Queue: Amin Hassani <ahassani@chromium.org>
diff --git a/cli/cros/cros_analyze_image.py b/cli/cros/cros_analyze_image.py
index ac29a81..a0777bf 100644
--- a/cli/cros/cros_analyze_image.py
+++ b/cli/cros/cros_analyze_image.py
@@ -136,9 +136,8 @@
Returns:
Local path to image file.
"""
- translated_path, _ = ds_wrapper.GetImagePathWithXbuddy(
+ _, image_path = ds_wrapper.GetImagePathWithXbuddy(
'xBuddy://remote', board, version)
- image_path = ds_wrapper.TranslatedPathToLocalPath(translated_path)
if local_path:
try:
diff --git a/cli/flash.py b/cli/flash.py
index 2028516..2a6071d 100644
--- a/cli/flash.py
+++ b/cli/flash.py
@@ -268,7 +268,7 @@
def _GetImagePath(self):
"""Returns the image path to use."""
- image_path = translated_path = None
+ image_path = None
if os.path.isfile(self.image):
if not self.yes and not _IsFilePathGPTDiskImage(self.image):
# TODO(wnwen): Open the tarball and if there is just one file in it,
@@ -284,11 +284,10 @@
image_path = _ChooseImageFromDirectory(self.image)
else:
# Translate the xbuddy path to get the exact image to use.
- translated_path, _ = ds_wrapper.GetImagePathWithXbuddy(
+ _, image_path = ds_wrapper.GetImagePathWithXbuddy(
self.image, self.board, self.version)
- image_path = ds_wrapper.TranslatedPathToLocalPath(translated_path)
- logging.info('Using image %s', translated_path or image_path)
+ logging.info('Using image %s', image_path)
return image_path
def Run(self):
@@ -454,16 +453,14 @@
(device.board, self.board))
logging.info('Board is %s', self.board)
-
# TODO(crbug.com/872441): Once devserver code has been moved to chromite,
# use xbuddy library directly instead of the devserver_wrapper.
# Fetch the full payload and properties, and stateful files. If this
# fails, fallback to downloading the image.
try:
- translated_path, _ = ds_wrapper.GetImagePathWithXbuddy(
+ _, local_path = ds_wrapper.GetImagePathWithXbuddy(
os.path.join(self.image, artifact_info.FULL_PAYLOAD),
self.board, self.version, silent=True)
- local_path = ds_wrapper.TranslatedPathToLocalPath(translated_path)
payload_dir, rootfs_filename = os.path.split(local_path)
ds_wrapper.GetImagePathWithXbuddy(
@@ -477,9 +474,8 @@
# We didn't find the full_payload, attempt to download the image.
if fetch_image:
- translated_path, _ = ds_wrapper.GetImagePathWithXbuddy(
+ _, image_path = ds_wrapper.GetImagePathWithXbuddy(
self.image, self.board, self.version)
- image_path = ds_wrapper.TranslatedPathToLocalPath(translated_path)
payload_dir = os.path.join(os.path.dirname(image_path), 'payloads')
logging.notice('Using image path %s and payload directory %s',
image_path, payload_dir)
diff --git a/lib/dev_server_wrapper.py b/lib/dev_server_wrapper.py
index 9085011..f751ea4 100644
--- a/lib/dev_server_wrapper.py
+++ b/lib/dev_server_wrapper.py
@@ -132,9 +132,7 @@
silent: Suppress error messages.
Returns:
- A tuple consisting of a translated path to the image
- (build-id/version/image_name) as well as the fully resolved XBuddy path (in
- the case where |path| is an XBuddy alias).
+ A tuple consisting of the build id and full path to the image.
"""
# Since xbuddy often wants to use gsutil from $PATH, make sure our local copy
# shows up first.
@@ -155,9 +153,7 @@
static_dir=static_dir)
path_list = GetXbuddyPath(path).rsplit(os.path.sep)
try:
- build_id, file_name = xb.Get(path_list)
- resolved_path, _ = xb.LookupAlias(os.path.sep.join(path_list))
- return os.path.join(build_id, file_name), resolved_path
+ return xb.Get(path_list)
except xbuddy.XBuddyException as e:
if not silent:
logging.error('Locating image "%s" failed. The path might not be valid '
@@ -194,25 +190,6 @@
raise ValueError('Does not support xbuddy request type %s' % req_type)
-def TranslatedPathToLocalPath(translated_path, static_dir=DEFAULT_STATIC_DIR):
- """Convert the translated path to a local path to the image file.
-
- Args:
- translated_path: the translated xbuddy path
- (e.g., peppy-release/R36-5760.0.0/chromiumos_image).
- static_dir: The static directory used by the devserver.
-
- Returns:
- A local path to the image file.
- """
- real_path = osutils.ExpandPath(os.path.join(static_dir, translated_path))
-
- if os.path.exists(real_path):
- return real_path
- else:
- return path_util.FromChrootPath(real_path)
-
-
def GenerateUpdateId(target, src, key, for_vm):
"""Returns a simple representation id of |target| and |src| paths.
diff --git a/lib/dev_server_wrapper_unittest.py b/lib/dev_server_wrapper_unittest.py
index fcb9cb3..d8ec26d 100644
--- a/lib/dev_server_wrapper_unittest.py
+++ b/lib/dev_server_wrapper_unittest.py
@@ -7,11 +7,6 @@
from __future__ import print_function
-import os
-
-import mock
-
-from chromite.lib import constants
from chromite.lib import cros_test_lib
from chromite.lib import dev_server_wrapper
from chromite.lib import partial_mock
@@ -73,33 +68,6 @@
translated_path),
'local/taco/R36-5600.0.0/dev')
- @mock.patch('chromite.lib.cros_build_lib.IsInsideChroot', return_value=True)
- def testTranslatedPathToLocalPath(self, _mock1):
- """Tests that we convert a translated path to a local path correctly."""
- translated_path = 'peppy-release/R33-5116.87.0/chromiumos_image.bin'
- base_path = os.path.join(self.tempdir, 'peppy-release/R33-5116.87.0')
-
- local_path = os.path.join(base_path, 'chromiumos_image.bin')
- self.assertEqual(
- dev_server_wrapper.TranslatedPathToLocalPath(translated_path,
- self.tempdir),
- local_path)
-
- @mock.patch('chromite.lib.cros_build_lib.IsInsideChroot', return_value=False)
- def testTranslatedPathToLocalPathOutsideChroot(self, _mock1):
- """Tests that we convert a translated path when outside the chroot."""
- translated_path = 'peppy-release/R33-5116.87.0/chromiumos_image.bin'
- chroot_dir = os.path.join(constants.SOURCE_ROOT,
- constants.DEFAULT_CHROOT_DIR)
- static_dir = os.path.join('devserver', 'static')
- chroot_static_dir = os.path.join('/', static_dir)
-
- local_path = os.path.join(chroot_dir, static_dir, translated_path)
- self.assertEqual(
- dev_server_wrapper.TranslatedPathToLocalPath(
- translated_path, chroot_static_dir),
- local_path)
-
class TestGetIPv4Address(cros_test_lib.RunCommandTestCase):
"""Tests the GetIPv4Address function."""
diff --git a/lib/xbuddy/build_artifact.py b/lib/xbuddy/build_artifact.py
index 65fcb69..d142b43 100644
--- a/lib/xbuddy/build_artifact.py
+++ b/lib/xbuddy/build_artifact.py
@@ -411,6 +411,17 @@
self.marker_name = self._SanitizeName(
'_'.join(['.' + self.name] + self._files_to_extract))
+ def StagedFiles(self):
+ """Returns the installed/staged files for this artifact.
+
+ If there are any files to unzip, only returns the extracted files since
+ there is usually no need for the compressed files.
+ """
+ files = super(BundledArtifact, self).StagedFiles()
+ if self._files_to_extract:
+ return [x for x in files if os.path.basename(x) in self._files_to_extract]
+ return files
+
def _RunUnzip(self, list_only):
# Unzip is weird. It expects its args before any excludes and expects its
# excludes in a list following the -x.
diff --git a/lib/xbuddy/xbuddy.py b/lib/xbuddy/xbuddy.py
index 9bc94a5..c618bbe 100644
--- a/lib/xbuddy/xbuddy.py
+++ b/lib/xbuddy/xbuddy.py
@@ -865,11 +865,11 @@
if image_type == ANY:
image_type = self._FindAny(artifact_dir)
- file_name = LOCAL_ALIAS_TO_FILENAME[image_type]
- artifact_path = os.path.join(artifact_dir, file_name)
- if not os.path.exists(artifact_path):
+ file_name = os.path.join(artifact_dir,
+ LOCAL_ALIAS_TO_FILENAME[image_type])
+ if not os.path.exists(file_name):
raise XBuddyException('Local %s artifact not in static_dir at %s' %
- (image_type, artifact_path))
+ (image_type, file_name))
else:
# Get a remote image.
if image_type not in GS_ALIASES:
@@ -950,5 +950,5 @@
# TODO(joyc): Run in separate thread.
self.CleanCache()
- _Log('Returning path to payload: %s/%s', build_id, file_name)
+ _Log('Returning build id: %s and path to payload: %s', build_id, file_name)
return build_id, file_name
diff --git a/lib/xbuddy/xbuddy_unittest.py b/lib/xbuddy/xbuddy_unittest.py
index d4456da..2b93f83 100644
--- a/lib/xbuddy/xbuddy_unittest.py
+++ b/lib/xbuddy/xbuddy_unittest.py
@@ -369,3 +369,18 @@
images_expected = ['a', 'b', 'f', 'e', 'd']
for i in range(5):
self.assertEqual(result[i][0], '%s-release/R0' % images_expected[i])
+
+ @mock.patch.object(xbuddy.XBuddy, '_Download',
+ return_value=[['/path/to/image.bin']])
+ @mock.patch.object(xbuddy.XBuddy, '_ResolveVersionToBuildIdAndChannel',
+ return_value=('reef/R1-1.2.3', 'stable'))
+ # pylint: disable=unused-argument
+ def testGet(self, resolve_mock, downloader_mock):
+ """Tests _GetArtifact method."""
+ self.assertEqual(
+ self.mock_xb.Get(['remote', 'reef', 'R1-1.2.3', 'test']),
+ ('reef/R1-1.2.3', '/path/to/image.bin'))
+
+ self.assertEqual(
+ self.mock_xb.Get(['remote', 'reef', 'R1-1.2.3']),
+ ('reef/R1-1.2.3', '/path/to/image.bin'))
