portage_util: remove usages of cros_build_lib.Die
Methods in chromite/libs/ shouldn't uses cros_build_lib.Die, the reason
is this raises SystemExit underneath & make us we lose all stack traces.
A potential cons of this is it may make some of the CLI reusing these
methods have uglier error messaging for the users. If this does happen
though, we can simply update the call sites in the CLI tools to do
try-catch & makes the error message more user-friendly.
BUG=chromium:917174
TEST=unittests
Change-Id: Id002893a433e64ae06884fbc6ee3957c27425394
Reviewed-on: https://chromium-review.googlesource.com/1447859
Commit-Ready: Ned Nguyen <nednguyen@google.com>
Tested-by: Ned Nguyen <nednguyen@google.com>
Reviewed-by: Mike Frysinger <vapier@chromium.org>
diff --git a/lib/portage_util.py b/lib/portage_util.py
index 516d1ff..5f575eb 100644
--- a/lib/portage_util.py
+++ b/lib/portage_util.py
@@ -768,6 +768,10 @@
Returns:
EBuild.SourceInfo namedtuple.
+
+ Raises:
+ raise Error if there are errors with extracting/validating data required
+ for constructing SourceInfo.
"""
localnames = self.cros_workon_vars.localname
projects = self.cros_workon_vars.project
@@ -791,7 +795,7 @@
os.path.dirname(self._unstable_ebuild_path))))
srcbase = os.path.join(base_dir, 'src')
if not os.path.isdir(srcbase):
- cros_build_lib.Die('_SRCPATH used but source path not found.')
+ raise Error('_SRCPATH used but source path not found.')
subdir_paths = []
subtree_paths = []
@@ -801,7 +805,7 @@
if srcpath:
subdir_path = os.path.join(srcbase, srcpath)
if not os.path.isdir(subdir_path):
- cros_build_lib.Die('Source for package %s not found.' % self.pkgname)
+ raise Error('Source for package %s not found.' % self.pkgname)
if self.subdir_support and subdir:
subdir_path = os.path.join(subdir_path, subdir)
@@ -815,16 +819,16 @@
subdir_path = os.path.join(subdir_path, subdir)
if not os.path.isdir(subdir_path):
- cros_build_lib.Die('Source repository %s '
- 'for project %s does not exist.' % (subdir_path,
- self.pkgname))
+ raise Error('Source repository %s '
+ 'for project %s does not exist.' % (subdir_path,
+ self.pkgname))
# Verify that we're grabbing the commit id from the right project name.
real_project = manifest.FindCheckoutFromPath(subdir_path)['name']
if project != real_project:
- cros_build_lib.Die('Project name mismatch for %s '
- '(found %s, expected %s)' % (subdir_path,
- real_project,
- project))
+ raise Error('Project name mismatch for %s '
+ '(found %s, expected %s)' % (subdir_path,
+ real_project,
+ project))
subdir_paths.append(subdir_path)
subtree_paths.extend(
@@ -836,10 +840,17 @@
subtrees=subtree_paths)
def GetCommitId(self, srcdir):
- """Get the commit id for this ebuild."""
+ """Get the commit id for this ebuild.
+
+ Returns:
+ Commit id (string) for this ebuild.
+
+ Raises:
+ raise Error if git fails to return the HEAD commit id.
+ """
output = self._RunGit(srcdir, ['rev-parse', 'HEAD'])
if not output:
- cros_build_lib.Die('Cannot determine HEAD commit for %s' % srcdir)
+ raise Error('Cannot determine HEAD commit for %s' % srcdir)
return output.rstrip()
def GetTreeId(self, path):
@@ -850,6 +861,9 @@
Given path can point a regular file, not a directory. If it does not exist,
None is returned.
+
+ Raises:
+ raise Error if git fails to determine the HEAD tree hash.
"""
if not os.path.exists(path):
return None
@@ -861,7 +875,7 @@
relpath = os.path.basename(path)
output = self._RunGit(basedir, ['rev-parse', 'HEAD:./%s' % relpath])
if not output:
- cros_build_lib.Die('Cannot determine HEAD tree hash for %s' % path)
+ raise Error('Cannot determine HEAD tree hash for %s' % path)
return output.rstrip()
def GetVersion(self, srcroot, manifest, default):
@@ -869,6 +883,10 @@
The version is provided by the ebuild through a specific script in
the $FILESDIR (chromeos-version.sh).
+
+ Raises:
+ raise Error when chromeos-version.sh script fails to return the raw
+ version number.
"""
vers_script = os.path.join(os.path.dirname(self._ebuild_path_no_version),
'files', 'chromeos-version.sh')
@@ -898,7 +916,7 @@
error_code_ok=True)
# TODO(crbug.com/917099): Remove this long-term because it's verbose.
ls_out = self._RunCommand(['ls', '-lRa'] + srcdirs, error_code_ok=True)
- cros_build_lib.Die(
+ raise Error(
'Package %s has a chromeos-version.sh script but failed:\n'
'return code = %s\nstdout = %s\nstderr = %s\ndir listing = %s\n'
'git fsck = %s\nsrcdirs = %s\n',
@@ -1022,9 +1040,9 @@
logging.info('Determining whether to create new ebuild %s',
new_stable_ebuild_path)
- if not os.path.exists(self._unstable_ebuild_path):
- cros_build_lib.Die('Missing unstable ebuild: %s' %
- self._unstable_ebuild_path)
+
+ assert os.path.exists(self._unstable_ebuild_path), (
+ 'Missing unstable ebuild: %s' % self._unstable_ebuild_path)
self.MarkAsStable(self._unstable_ebuild_path, new_stable_ebuild_path,
variables, redirect_file)
@@ -1338,6 +1356,9 @@
allow_blacklisted: If False, discard blacklisted packages.
subdir_support: Support obsolete CROS_WORKON_SUBDIR.
Intended for branchs older than 10363.0.0.
+
+ Raises:
+ raise Error if there is error with validating the ebuild files.
"""
stable_ebuilds = []
unstable_ebuilds = []
@@ -1350,8 +1371,8 @@
continue
if ebuild.is_stable:
if ebuild.version == WORKON_EBUILD_VERSION:
- cros_build_lib.Die('KEYWORDS in %s ebuild should not be stable %s'
- % (WORKON_EBUILD_VERSION, path))
+ raise Error('KEYWORDS in %s ebuild should not be stable %s'
+ % (WORKON_EBUILD_VERSION, path))
stable_ebuilds.append(ebuild)
else:
unstable_ebuilds.append(ebuild)
@@ -1361,13 +1382,12 @@
if not unstable_ebuilds:
if stable_ebuilds:
path = os.path.dirname(stable_ebuilds[0].ebuild_path)
- cros_build_lib.Die(
- 'Missing %s ebuild in %s' % (WORKON_EBUILD_VERSION, path))
+ raise Error('Missing %s ebuild in %s' % (WORKON_EBUILD_VERSION, path))
return None
path = os.path.dirname(unstable_ebuilds[0].ebuild_path)
- if len(unstable_ebuilds) > 1:
- cros_build_lib.Die('Found multiple unstable ebuilds in %s' % path)
+ assert len(unstable_ebuilds) <= 1, (
+ 'Found multiple unstable ebuilds in %s' % path)
if not stable_ebuilds:
logging.warning('Missing stable ebuild in %s', path)
@@ -1382,7 +1402,7 @@
message = 'Found multiple stable ebuild versions in %s:' % path
for version in stable_versions:
message += '\n %s-%s' % (package, version)
- cros_build_lib.Die(message)
+ raise Error(message)
uprev_ebuild = max(stable_ebuilds, key=lambda eb: eb.current_revision)
for ebuild in stable_ebuilds:
diff --git a/lib/portage_util_unittest.py b/lib/portage_util_unittest.py
index 34ed0dd..5218414 100644
--- a/lib/portage_util_unittest.py
+++ b/lib/portage_util_unittest.py
@@ -734,7 +734,8 @@
# Reject no output.
run.return_value = cros_build_lib.CommandResult(
returncode=0, output='', error='STDERR')
- self.assertRaises(SystemExit, self.m_ebuild.GetVersion, None, None, '1234')
+ self.assertRaises(portage_util.Error,
+ self.m_ebuild.GetVersion, None, None, '1234')
# Sanity check.
self.assertEqual(exists.call_count, 1)
exists.reset_mock()
@@ -742,7 +743,8 @@
# Reject simple output.
run.return_value = cros_build_lib.CommandResult(
returncode=0, output='\n', error='STDERR')
- self.assertRaises(SystemExit, self.m_ebuild.GetVersion, None, None, '1234')
+ self.assertRaises(portage_util.Error,
+ self.m_ebuild.GetVersion, None, None, '1234')
# Sanity check.
self.assertEqual(exists.call_count, 1)
exists.reset_mock()
@@ -750,7 +752,8 @@
# Reject error.
run.return_value = cros_build_lib.CommandResult(
returncode=1, output='FAIL\n', error='STDERR')
- self.assertRaises(SystemExit, self.m_ebuild.GetVersion, None, None, '1234')
+ self.assertRaises(portage_util.Error,
+ self.m_ebuild.GetVersion, None, None, '1234')
# Sanity check.
self.assertEqual(exists.call_count, 1)
