commit edcc797f36e82d2c1406dc10d044cd9201068a14
author Mike Frysinger <vapier@chromium.org> Thu Nov 21 15:25:33 2019 -0500
committer Commit Bot <commit-bot@chromium.org> Fri Nov 22 10:34:42 2019 +0000
tree 57af15003bc4087d760c488bea8c3c0c496712f2
parent e3c0df695330359dbf26bfe98a87233195bc4014

portage_util: improve Classify return value and add tests

The return value of this func is getting a bit out of hand as we add
more classifications, so turn it into a named tuple.  The func lacks
direct testing, so add some unittests.

In the process, notice that we mishandle trailing comments on KEYWORDS
lines, so fix that too.

BUG=chromium:997354
TEST=`./run_tests` passes

Change-Id: Id9090fd7b92f272e6e1ac6cb1f44288223d4d2c7
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/chromite/+/1929751
Reviewed-by: Alex Klein <saklein@chromium.org>
Commit-Queue: Mike Frysinger <vapier@chromium.org>
Tested-by: Mike Frysinger <vapier@chromium.org>

lib/portage_util.py

diff --git a/lib/portage_util.py b/lib/portage_util.py
index d3c7bcc..57d1e2f 100644
--- a/lib/portage_util.py
+++ b/lib/portage_util.py
@@ -352,6 +352,11 @@
     super(EbuildFormatIncorrectError, self).__init__(message)
 
 
+# Container for Classify return values.
+EBuildClassifyAttributes = collections.namedtuple('EBuildClassifyAttributes', (
+    'is_workon', 'is_stable', 'is_blacklisted', 'has_test'))
+
+
 class EBuild(object):
   """Wrapper class for information about an ebuild."""
 
@@ -540,7 +545,10 @@
         if EBuild._ECLASS_IMPLIES_TEST & eclasses:
           has_test = True
       elif line.startswith('KEYWORDS='):
-        for keyword in line.split('=', 1)[1].strip('"\'').split():
+        # Strip off the comments, then extract the value of the variable, then
+        # strip off any quotes.
+        line = line.split('#', 1)[0].split('=', 1)[1].strip('"\'')
+        for keyword in line.split():
           if not keyword.startswith('~') and keyword != '-*':
             is_stable = True
       elif line.startswith('CROS_WORKON_BLACKLIST='):
@@ -549,7 +557,8 @@
             line.startswith('platform_pkg_test()')):
         has_test = True
     fileinput.close()
-    return is_workon, is_stable, is_blacklisted, has_test
+    return EBuildClassifyAttributes(
+        is_workon, is_stable, is_blacklisted, has_test)
 
   def _ReadEBuild(self, path):
     """Determine the settings of `is_workon`, `is_stable` and is_blacklisted

lib/portage_util_unittest.py

diff --git a/lib/portage_util_unittest.py b/lib/portage_util_unittest.py
index 1cf987c..cca890f 100644
--- a/lib/portage_util_unittest.py
+++ b/lib/portage_util_unittest.py
@@ -243,6 +243,44 @@
     self.assertFalse(
         AlmostSameEBuilds(self._EBUILD_BASE, self._EBUILD_DIFFERENT_CONTENT))
 
+  def testClassifySimple(self):
+    """Test Classify on a simple ebuild."""
+    ebuild_path = os.path.join(self.tempdir, 'foo-1.ebuild')
+    osutils.WriteFile(ebuild_path, '')
+    attrs = portage_util.EBuild.Classify(ebuild_path)
+    self.assertFalse(attrs.is_workon)
+    self.assertFalse(attrs.is_stable)
+    self.assertFalse(attrs.is_blacklisted)
+    self.assertFalse(attrs.has_test)
+
+  def testClassifyUnstable(self):
+    """Test Classify handling of non-stable KEYWORDS."""
+    ebuild_path = os.path.join(self.tempdir, 'foo-1.ebuild')
+    TESTS = (
+        'KEYWORDS=',
+        'KEYWORDS= # Yep.',
+        'KEYWORDS="-*"',
+        'KEYWORDS="-* ~arm"',
+        'KEYWORDS="~*"',
+    )
+    for keywords in TESTS:
+      osutils.WriteFile(ebuild_path, keywords)
+      attrs = portage_util.EBuild.Classify(ebuild_path)
+      self.assertFalse(attrs.is_stable, msg='Failing: %s' % (keywords,))
+
+  def testClassifyStable(self):
+    """Test Classify handling of stable KEYWORDS."""
+    ebuild_path = os.path.join(self.tempdir, 'foo-1.ebuild')
+    TESTS = (
+        'KEYWORDS="*"',
+        'KEYWORDS="*" # Yep.',
+        'KEYWORDS="-* arm"',
+    )
+    for keywords in TESTS:
+      osutils.WriteFile(ebuild_path, keywords)
+      attrs = portage_util.EBuild.Classify(ebuild_path)
+      self.assertTrue(attrs.is_stable, msg='Failing: %s' % (keywords,))
+
 
 class ProjectAndPathTest(cros_test_lib.MockTempDirTestCase):
   """Project and Path related tests."""

service/dependency.py

diff --git a/service/dependency.py b/service/dependency.py
index 4b6d2da..72e08d3 100644
--- a/service/dependency.py
+++ b/service/dependency.py
@@ -201,11 +201,11 @@
   buildroot = os.path.join(constants.CHROOT_SOURCE_ROOT, 'src')
   manifest = git.ManifestCheckout.Cached(buildroot)
   for package, ebuild_path in packages_to_ebuild_paths.items():
-    is_workon, _, is_blacklisted, _ = portage_util.EBuild.Classify(ebuild_path)
-    if (not is_workon or
+    attrs = portage_util.EBuild.Classify(ebuild_path)
+    if (not attrs.is_workon or
         # Blacklisted ebuild is pinned to a specific git sha1, so change in
         # that repo matter to the ebuild.
-        is_blacklisted):
+        attrs.is_blacklisted):
       continue
     ebuild = portage_util.EBuild(ebuild_path)
     workon_subtrees = ebuild.GetSourceInfo(buildroot, manifest).subtrees