llvm_tools: Fix manifest update
Usually we don't need the manifest update with llvm-next
because pgo is not default there.
Remove default manifest packages from llvm-next.
Add unit test for main.
BUG=None
TEST=unit test
Change-Id: I1ef78be8184985e047db8ae68eda2f01c989a7a5
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/third_party/toolchain-utils/+/3932864
Tested-by: Denis Nikitin <denik@chromium.org>
Commit-Queue: Denis Nikitin <denik@chromium.org>
Reviewed-by: Manoj Gupta <manojgupta@chromium.org>
diff --git a/llvm_tools/update_chromeos_llvm_hash.py b/llvm_tools/update_chromeos_llvm_hash.py
index 31a1086..c52c732 100755
--- a/llvm_tools/update_chromeos_llvm_hash.py
+++ b/llvm_tools/update_chromeos_llvm_hash.py
@@ -17,7 +17,7 @@
from pathlib import Path
import re
import subprocess
-from typing import Dict, List
+from typing import Dict, Iterable
import chroot
import failure_modes
@@ -99,7 +99,7 @@
parser.add_argument(
"--manifest_packages",
- default=",".join(DEFAULT_MANIFEST_PACKAGES),
+ default="",
help="Comma-separated ebuilds to update manifests for "
"(default: %(default)s)",
)
@@ -506,7 +506,7 @@
return commit_messages
-def UpdateManifests(packages: List[str], chroot_path: Path):
+def UpdateManifests(packages: Iterable[str], chroot_path: Path):
"""Updates manifest files for packages.
Args:
@@ -524,8 +524,8 @@
def UpdatePackages(
- packages,
- manifest_packages: List[str],
+ packages: Iterable[str],
+ manifest_packages: Iterable[str],
llvm_variant,
git_hash,
svn_version,
@@ -672,7 +672,7 @@
def UpdatePackagesPatchMetadataFile(
chroot_path: Path,
svn_version: int,
- packages: List[str],
+ packages: Iterable[str],
mode: failure_modes.FailureModes,
) -> Dict[str, patch_utils.PatchInfo]:
"""Updates the packages metadata file.
@@ -761,8 +761,14 @@
git_hash_source
)
- packages = args_output.update_packages.split(",")
- manifest_packages = args_output.manifest_packages.split(",")
+ # Filter out empty strings. For example "".split{",") returns [""].
+ packages = set(p for p in args_output.update_packages.split(",") if p)
+ manifest_packages = set(
+ p for p in args_output.manifest_packages.split(",") if p
+ )
+ if not manifest_packages and not args_output.is_llvm_next:
+ # Set default manifest packages only for the current llvm.
+ manifest_packages = set(DEFAULT_MANIFEST_PACKAGES)
change_list = UpdatePackages(
packages=packages,
manifest_packages=manifest_packages,
diff --git a/llvm_tools/update_chromeos_llvm_hash_unittest.py b/llvm_tools/update_chromeos_llvm_hash_unittest.py
index 9bed271..b758538 100755
--- a/llvm_tools/update_chromeos_llvm_hash_unittest.py
+++ b/llvm_tools/update_chromeos_llvm_hash_unittest.py
@@ -12,6 +12,7 @@
import os
from pathlib import Path
import subprocess
+import sys
import unittest
import unittest.mock as mock
@@ -19,7 +20,6 @@
import failure_modes
import get_llvm_hash
import git
-import subprocess_helpers
import test_helpers
import update_chromeos_llvm_hash
@@ -1017,6 +1017,130 @@
mock_delete_repo.assert_called_once_with(path_to_package_dir, branch)
+ @mock.patch.object(chroot, "VerifyOutsideChroot")
+ @mock.patch.object(get_llvm_hash, "GetLLVMHashAndVersionFromSVNOption")
+ @mock.patch.object(update_chromeos_llvm_hash, "UpdatePackages")
+ def testMainDefaults(
+ self, mock_update_packages, mock_gethash, mock_outside_chroot
+ ):
+ git_hash = "1234abcd"
+ svn_version = 5678
+ mock_gethash.return_value = (git_hash, svn_version)
+ argv = [
+ "./update_chromeos_llvm_hash_unittest.py",
+ "--llvm_version",
+ "google3",
+ ]
+
+ with mock.patch.object(sys, "argv", argv) as mock.argv:
+ update_chromeos_llvm_hash.main()
+
+ expected_packages = set(update_chromeos_llvm_hash.DEFAULT_PACKAGES)
+ expected_manifest_packages = set(
+ update_chromeos_llvm_hash.DEFAULT_MANIFEST_PACKAGES,
+ )
+ expected_llvm_variant = update_chromeos_llvm_hash.LLVMVariant.current
+ expected_chroot = update_chromeos_llvm_hash.defaultCrosRoot()
+ mock_update_packages.assert_called_once_with(
+ packages=expected_packages,
+ manifest_packages=expected_manifest_packages,
+ llvm_variant=expected_llvm_variant,
+ git_hash=git_hash,
+ svn_version=svn_version,
+ chroot_path=expected_chroot,
+ mode=failure_modes.FailureModes.FAIL,
+ git_hash_source="google3",
+ extra_commit_msg=None,
+ )
+ mock_outside_chroot.assert_called()
+
+ @mock.patch.object(chroot, "VerifyOutsideChroot")
+ @mock.patch.object(get_llvm_hash, "GetLLVMHashAndVersionFromSVNOption")
+ @mock.patch.object(update_chromeos_llvm_hash, "UpdatePackages")
+ def testMainLlvmNext(
+ self, mock_update_packages, mock_gethash, mock_outside_chroot
+ ):
+ git_hash = "1234abcd"
+ svn_version = 5678
+ mock_gethash.return_value = (git_hash, svn_version)
+ argv = [
+ "./update_chromeos_llvm_hash_unittest.py",
+ "--llvm_version",
+ "google3",
+ "--is_llvm_next",
+ ]
+
+ with mock.patch.object(sys, "argv", argv) as mock.argv:
+ update_chromeos_llvm_hash.main()
+
+ expected_packages = set(update_chromeos_llvm_hash.DEFAULT_PACKAGES)
+ expected_llvm_variant = update_chromeos_llvm_hash.LLVMVariant.next
+ expected_chroot = update_chromeos_llvm_hash.defaultCrosRoot()
+ # llvm-next upgrade does not update manifest by default.
+ mock_update_packages.assert_called_once_with(
+ packages=expected_packages,
+ manifest_packages=set(),
+ llvm_variant=expected_llvm_variant,
+ git_hash=git_hash,
+ svn_version=svn_version,
+ chroot_path=expected_chroot,
+ mode=failure_modes.FailureModes.FAIL,
+ git_hash_source="google3",
+ extra_commit_msg=None,
+ )
+ mock_outside_chroot.assert_called()
+
+ @mock.patch.object(chroot, "VerifyOutsideChroot")
+ @mock.patch.object(get_llvm_hash, "GetLLVMHashAndVersionFromSVNOption")
+ @mock.patch.object(update_chromeos_llvm_hash, "UpdatePackages")
+ def testMainAllArgs(
+ self, mock_update_packages, mock_gethash, mock_outside_chroot
+ ):
+ packages_to_update = "test-packages/package1,test-libs/lib1"
+ manifest_packages = "test-libs/lib1,test-libs/lib2"
+ failure_mode = failure_modes.FailureModes.REMOVE_PATCHES
+ chroot_path = Path("/some/path/to/chroot")
+ llvm_ver = 435698
+ git_hash = "1234abcd"
+ svn_version = 5678
+ mock_gethash.return_value = (git_hash, svn_version)
+
+ argv = [
+ "./update_chromeos_llvm_hash_unittest.py",
+ "--llvm_version",
+ str(llvm_ver),
+ "--is_llvm_next",
+ "--chroot_path",
+ str(chroot_path),
+ "--update_packages",
+ packages_to_update,
+ "--manifest_packages",
+ manifest_packages,
+ "--failure_mode",
+ failure_mode.value,
+ "--patch_metadata_file",
+ "META.json",
+ ]
+
+ with mock.patch.object(sys, "argv", argv) as mock.argv:
+ update_chromeos_llvm_hash.main()
+
+ expected_packages = {"test-packages/package1", "test-libs/lib1"}
+ expected_manifest_packages = {"test-libs/lib1", "test-libs/lib2"}
+ expected_llvm_variant = update_chromeos_llvm_hash.LLVMVariant.next
+ mock_update_packages.assert_called_once_with(
+ packages=expected_packages,
+ manifest_packages=expected_manifest_packages,
+ llvm_variant=expected_llvm_variant,
+ git_hash=git_hash,
+ svn_version=svn_version,
+ chroot_path=chroot_path,
+ mode=failure_mode,
+ git_hash_source=llvm_ver,
+ extra_commit_msg=None,
+ )
+ mock_outside_chroot.assert_called()
+
@mock.patch.object(subprocess, "check_output", return_value=None)
@mock.patch.object(get_llvm_hash, "GetLLVMMajorVersion")
def testEnsurePackageMaskContainsExisting(
