From 903ed4ee304a1305229b43c90674811673f8ace4 Mon Sep 17 00:00:00 2001
From: Oliver Smith <ollieparanoid@postmarketos.org>
Date: Sun, 29 May 2022 10:13:48 +0200
Subject: [PATCH] pmb.chroot.apk.packages_split_to_add_del: new func (MR 2185)

Make the code easier to read by moving split_to_add_del() to a separate
function and do some related refactoring. A future patch will use it
twice in install().

Move "arch = ..." to the top of the function while at it, since it's
needed later in the function in 2 places and is not needed for figuring
out packages_with_depends, to_add and to_del.

Remove "# Add depends to packages" because it's obvious from the
packages_with_depends variable name, and getting to_add/to_del is a
different action that stood under the same comment.
---
 pmb/chroot/apk.py | 39 +++++++++++++++++++++++++++------------
 test/test_apk.py  |  8 ++++++++
 2 files changed, 35 insertions(+), 12 deletions(-)

diff --git a/pmb/chroot/apk.py b/pmb/chroot/apk.py
index e3e5c907..15354e61 100644
--- a/pmb/chroot/apk.py
+++ b/pmb/chroot/apk.py
@@ -113,6 +113,27 @@ def install_build(args, package, arch):
     return pmb.build.package(args, package, arch)
 
 
+def packages_split_to_add_del(packages):
+    """
+    Sort packages into "to_add" and "to_del" lists depending on their pkgname
+    starting with an exclamation mark.
+
+    :param packages: list of pkgnames
+    :returns: (to_add, to_del) - tuple of lists of pkgnames, e.g.
+              (["hello-world", ...], ["some-conflict-pkg", ...])
+    """
+    to_add = []
+    to_del = []
+
+    for package in packages:
+        if package.startswith("!"):
+            to_del.append(package.lstrip("!"))
+        else:
+            to_add.append(package)
+
+    return (to_add, to_del)
+
+
 def replace_aports_packages_with_path(args, packages, suffix, arch):
     """
     apk will only re-install packages with the same pkgname,
@@ -187,6 +208,8 @@ def install(args, packages, suffix="native", build=True):
                   special case that all packages are expected to be in Alpine's
                   repositories, set this to False for performance optimization.
     """
+    arch = pmb.parse.arch.from_chroot_suffix(args, suffix)
+
     if not packages:
         logging.verbose("pmb.chroot.apk.install called with empty packages list,"
                         " ignoring")
@@ -196,21 +219,12 @@ def install(args, packages, suffix="native", build=True):
     check_min_version(args, suffix)
     pmb.chroot.init(args, suffix)
 
-    # Add depends to packages
-    arch = pmb.parse.arch.from_chroot_suffix(args, suffix)
     packages_with_depends = pmb.parse.depends.recurse(args, packages, suffix)
+    to_add, to_del = packages_split_to_add_del(packages_with_depends)
 
-    # Filter outdated packages (build them if required)
-    packages_installed = installed(args, suffix)
-    to_add = []
-    to_del = []
-    for package in packages_with_depends:
-        if package.startswith("!"):
-            to_del.append(package.lstrip("!"))
-            continue
-        if build:
+    if build:
+        for package in to_add:
             install_build(args, package, arch)
-        to_add.append(package)
 
     # Sanitize packages: don't allow '--allow-untrusted' and other options
     # to be passed to apk!
@@ -219,6 +233,7 @@ def install(args, packages, suffix="native", build=True):
             raise ValueError(f"Invalid package name: {package}")
 
     # Readable install message without dependencies
+    packages_installed = installed(args, suffix)
     message = f"({suffix}) install"
     for pkgname in packages:
         if pkgname not in packages_installed:
diff --git a/test/test_apk.py b/test/test_apk.py
index 737cf020..ae4b743f 100644
--- a/test/test_apk.py
+++ b/test/test_apk.py
@@ -55,3 +55,11 @@ def test_install_build(monkeypatch, args):
     args.action = "chroot"
     args.build_pkgs_on_install = False
     assert func(args, package, arch) == "build-pkg"
+
+
+def test_packages_split_to_add_del():
+    packages = ["hello", "!test", "hello2", "test2", "!test3"]
+
+    to_add, to_del = pmb.chroot.apk.packages_split_to_add_del(packages)
+    assert to_add == ["hello", "hello2", "test2"]
+    assert to_del == ["test", "test3"]