drivefs-uprev: Fix version comparison
Use distutils.version.LooseVersion to compare the DriveFS tags being
ingested by the PUpr builder. Tags are in the form
refs/tags/drivefs_1.2.3 and |LooseVersion| will numerically compare
major, minor then patch version.
BUG=chromium:1118212
TEST=run_pytest
TEST=https://chromeos-swarming.appspot.com/task?id=5081edbc2491c410
Change-Id: Ie34a294c1015f5fd8601cf34b7378797c7bdb3d3
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/chromite/+/2592549
Tested-by: Ben Reich <benreich@chromium.org>
Reviewed-by: Mike Frysinger <vapier@chromium.org>
Reviewed-by: Chris McDonald <cjmcdonald@chromium.org>
Commit-Queue: Chris McDonald <cjmcdonald@chromium.org>
diff --git a/service/packages.py b/service/packages.py
index 53ed741..4a152b9 100644
--- a/service/packages.py
+++ b/service/packages.py
@@ -8,12 +8,14 @@
from __future__ import print_function
import collections
+from distutils.version import LooseVersion
import fileinput
import functools
import json
import os
import re
import sys
+from typing import List
from google.protobuf import json_format
@@ -294,25 +296,20 @@
UprevVersionedPackageResult: The result of updating drivefs ebuilds.
"""
- DRIVEFS_REFS = 'refs/tags/drivefs_'
- DRIVEFS_PATH = 'src/private-overlays/chromeos-overlay/chromeos-base'
+ DRIVEFS_PATH_PREFIX = 'src/private-overlays/chromeos-overlay/chromeos-base'
- # parse like the example PUpr (go/pupr#steps)
- valid_refs = [ref.ref for ref in refs if ref.ref.startswith(DRIVEFS_REFS)]
- if not valid_refs:
- # False alarm, there is no new package release. So do nothing.
+ drivefs_version = get_latest_drivefs_version_from_refs(refs)
+ if not drivefs_version:
+ # No valid DriveFS version is identified.
return None
- # Take the newest release, always.
- target_version = sorted(valid_refs,
- reverse=True)[0][len(DRIVEFS_REFS):]
+ logging.debug('DriveFS version determined from refs: %s', drivefs_version)
result = uprev_lib.UprevVersionedPackageResult()
- pkg_path = os.path.join(DRIVEFS_PATH, 'drivefs')
-
+ pkg_path = os.path.join(DRIVEFS_PATH_PREFIX, 'drivefs')
uprev_result = uprev_lib.uprev_workon_ebuild_to_version(pkg_path,
- target_version,
+ drivefs_version,
chroot)
all_changed_files = []
@@ -321,10 +318,10 @@
all_changed_files.extend(uprev_result.changed_files)
- pkg_path = os.path.join(DRIVEFS_PATH, 'drivefs-ipc')
+ pkg_path = os.path.join(DRIVEFS_PATH_PREFIX, 'drivefs-ipc')
uprev_result = uprev_lib.uprev_workon_ebuild_to_version(pkg_path,
- target_version,
+ drivefs_version,
chroot)
if not uprev_result:
@@ -332,7 +329,7 @@
all_changed_files.extend(uprev_result.changed_files)
- result.add_result(target_version, all_changed_files)
+ result.add_result(drivefs_version, all_changed_files)
return result
@@ -499,6 +496,36 @@
return result.add_result(chrome_version, uprev_manager.modified_ebuilds)
+def get_latest_drivefs_version_from_refs(refs: List[uprev_lib.GitRef]) -> str:
+ """Get the latest DriveFS version from refs
+
+ DriveFS versions follow the tag format of refs/tags/drivefs_1.2.3.
+ Versions are compared using |distutils.version.LooseVersion| and
+ the latest version is returned.
+
+ Args:
+ refs: The tags to parse for the latest DriveFS version.
+
+ Returns:
+ The latest DriveFS version to use.
+ """
+ DRIVEFS_REFS_PREFIX = 'refs/tags/drivefs_'
+
+ valid_refs = []
+ for gitiles in refs:
+ if gitiles.ref.startswith(DRIVEFS_REFS_PREFIX):
+ valid_refs.append(gitiles.ref)
+
+ if not valid_refs:
+ return None
+
+ # Sort by version and take the latest version.
+ target_version_ref = sorted(valid_refs,
+ key=LooseVersion,
+ reverse=True)[0]
+ return target_version_ref.replace(DRIVEFS_REFS_PREFIX, '')
+
+
def _generate_platform_c_files(replication_config, chroot):
"""Generates platform C files from a platform JSON payload.
diff --git a/service/packages_unittest.py b/service/packages_unittest.py
index 9bcae74..ad9183e 100644
--- a/service/packages_unittest.py
+++ b/service/packages_unittest.py
@@ -1054,3 +1054,31 @@
return_value=['key1', 'key2'])
result = packages.get_key_id(self.build_target, 'model')
self.assertEqual(result, None)
+
+
+class GetLatestDrivefsVersionTest(cros_test_lib.TestCase):
+ """Tests for get_latest_drivefs_version_from_refs."""
+
+ def setUp(self):
+ # The tag ref template.
+ ref_tpl = 'refs/tags/drivefs_%s'
+
+ self.latest = '44.0.20'
+ self.versions = ['42.0.1', self.latest, '44.0.19', '39.0.15']
+ self.latest_ref = uprev_lib.GitRef('/path', ref_tpl % self.latest, 'abc123')
+ self.refs = [uprev_lib.GitRef('/path', ref_tpl % v, 'abc123')
+ for v in self.versions]
+
+ def test_single_ref(self):
+ """Test a single ref is supplied."""
+ self.assertEqual(self.latest,
+ packages.get_latest_drivefs_version_from_refs([self.latest_ref]))
+
+ def test_multiple_ref_versions(self):
+ """Test multiple refs supplied."""
+ self.assertEqual(self.latest,
+ packages.get_latest_drivefs_version_from_refs(self.refs))
+
+ def test_no_refs_returns_none(self):
+ """Test no refs supplied."""
+ self.assertEqual(packages.get_latest_drivefs_version_from_refs([]), None)