pmb.parse._apkbuild: Extend APKBUILD parser to work for subpackages (!1866)

At the moment we have a simple subpkgdesc() function that can only
parse "pkgdesc" from subpackages, without support for any variables.
But we have a quite nice variable parser now that can be extended
to work for subpackages.

Simply put this works by:
  - Finding the lines that belong to the subpackage function
  - Stripping indentation (tab)
  - Parsing relevant attributes similar to the apkbuild() function

The "subpackages" in the parsed APKBUILD are replaced by a dict
of subpkgname: {"pkgdesc": "...", "depends": "..."} which are
parsed from the subpackage function (if found).
This makes it possible to get the "depends" of a subpackage.

pmb/build/_package.py

@@ -120,7 +120,7 @@ def get_depends(args, apkbuild):
     ret = sorted(set(ret))
 
     # Don't recurse forever when a package depends on itself (#948)
-    for pkgname in [apkbuild["pkgname"]] + list(apkbuild["subpackages"]):
+    for pkgname in [apkbuild["pkgname"]] + list(apkbuild["subpackages"].keys()):
         if pkgname in ret:
             logging.verbose(apkbuild["pkgname"] + ": ignoring dependency on"
                             " itself: " + pkgname)

pmb/config/__init__.py

@@ -214,20 +214,24 @@ necessary_kconfig_options = {
 # PARSE
 #
 
+# Variables belonging to a package or subpackage in APKBUILD files
+apkbuild_package_attributes = {
+    "pkgdesc": {},
+    "depends": {"array": True},
+    "provides": {"array": True},
+}
+
 # Variables in APKBUILD files, that get parsed
 apkbuild_attributes = {
     "arch": {"array": True},
-    "depends": {"array": True},
     "depends_dev": {"array": True},
     "makedepends": {"array": True},
     "checkdepends": {"array": True},
     "options": {"array": True},
     "pkgname": {},
-    "pkgdesc": {},
     "pkgrel": {},
     "pkgver": {},
-    "provides": {"array": True},
+    "subpackages": {},
-    "subpackages": {"array": True},
     "url": {},
 
     # cross-compilers
@@ -250,6 +254,8 @@ apkbuild_attributes = {
     "_commit": {},
     "source": {"array": True},
 }
+# **apkbuild_package_attributes above would be nicer, but requires Python 3.5+
+apkbuild_attributes.update(apkbuild_package_attributes)
 
 # Variables from deviceinfo. Reference: <https://postmarketos.org/deviceinfo>
 deviceinfo_attributes = [

pmb/config/init.py

@@ -205,7 +205,7 @@ def ask_for_device_nonfree(args, device):
 
     # Only run when there is a "nonfree" subpackage
     nonfree_found = False
-    for subpackage in apkbuild["subpackages"]:
+    for subpackage in apkbuild["subpackages"].keys():
         if subpackage.startswith("device-" + device + "-nonfree"):
             nonfree_found = True
     if not nonfree_found:
@@ -221,10 +221,11 @@ def ask_for_device_nonfree(args, device):
     # Ask for firmware and userland individually
     for type in ["firmware", "userland"]:
         subpkgname = "device-" + device + "-nonfree-" + type
-        if subpkgname in apkbuild["subpackages"]:
+        subpkg = apkbuild["subpackages"].get(subpkgname, {})
-            subpkgdesc = pmb.parse._apkbuild.subpkgdesc(apkbuild_path,
+        if subpkg is None:
-                                                        "nonfree_" + type)
+            raise RuntimeError("Cannot find subpackage function for " + subpkgname)
-            logging.info(subpkgname + ": " + subpkgdesc)
+        if subpkg:
+            logging.info(subpkgname + ": " + subpkg["pkgdesc"])
             ret[type] = pmb.helpers.cli.confirm(args, "Enable this package?",
                                                 default=ret[type])
     return ret

pmb/helpers/pmaports.py

@@ -134,10 +134,9 @@ def find(args, package, must_exist=True):
                 found = False
 
                 # Subpackages
-                for subpackage_i in apkbuild["subpackages"]:
+                if package in apkbuild["subpackages"]:
-                    if package == subpackage_i.split(":", 1)[0]:
+                    found = True
-                        found = True
+                    break
-                        break
 
                 # Provides (cut off before equals sign for entries like
                 # "mkbootimg=0.0.1")

pmb/parse/_apkbuild.py

@@ -19,6 +19,7 @@ along with pmbootstrap.  If not, see <http://www.gnu.org/licenses/>.
 import logging
 import os
 import re
+from collections import OrderedDict
 
 import pmb.config
 import pmb.parse.version
@@ -100,18 +101,6 @@ def replace_variable(apkbuild, value: str) -> str:
     return value
 
 
-def cut_off_function_names(apkbuild):
-    """
-    For subpackages: only keep the subpackage name, without the internal
-    function name, that tells how to build the subpackage.
-    """
-    sub = apkbuild["subpackages"]
-    for i in range(len(sub)):
-        sub[i] = sub[i].split(":", 1)[0]
-    apkbuild["subpackages"] = sub
-    return apkbuild
-
-
 def function_body(path, func):
     """
     Get the body of a function in an APKBUILD.
@@ -209,6 +198,100 @@ def parse_attribute(attribute, lines, i, path):
                        " attribute '" + attribute + "' in: " + path)
 
 
+def _parse_attributes(path, lines, apkbuild_attributes, ret):
+    """
+    Parse attributes from a list of lines. Variables are replaced with values
+    from ret (if found) and split into the format configured in apkbuild_attributes.
+
+    :param lines: the lines to parse
+    :param apkbuild_attributes: the attributes to parse
+    :param ret: a dict to update with new parsed variable
+    """
+    for i in range(len(lines)):
+        for attribute, options in apkbuild_attributes.items():
+            found, value, i = parse_attribute(attribute, lines, i, path)
+            if not found:
+                continue
+
+            ret[attribute] = replace_variable(ret, value)
+
+    if "subpackages" in apkbuild_attributes:
+        subpackages = OrderedDict()
+        for subpkg in ret["subpackages"].split(" "):
+            if subpkg:
+                _parse_subpackage(path, lines, ret, subpackages, subpkg)
+        ret["subpackages"] = subpackages
+
+    # Split attributes
+    for attribute, options in apkbuild_attributes.items():
+        if options.get("array", False):
+            # Split up arrays, delete empty strings inside the list
+            ret[attribute] = list(filter(None, ret[attribute].split(" ")))
+
+
+def _parse_subpackage(path, lines, apkbuild, subpackages, subpkg):
+    """
+    Attempt to parse attributes from a subpackage function.
+    This will attempt to locate the subpackage function in the APKBUILD and
+    update the given attributes with values set in the subpackage function.
+
+    :param path: path to APKBUILD
+    :param lines: the lines to parse
+    :param apkbuild: dict of attributes already parsed from APKBUILD
+    :param subpackages: the subpackages dict to update
+    :param subpkg: the subpackage to parse
+                   (may contain subpackage function name separated by :)
+    """
+    subpkgparts = subpkg.split(":")
+    subpkgname = subpkgparts[0]
+    subpkgsplit = subpkgname[subpkgname.rfind("-") + 1:]
+    if len(subpkgparts) > 1:
+        subpkgsplit = subpkgparts[1]
+
+    # Find start and end of package function
+    start = end = 0
+    prefix = subpkgsplit + "() {"
+    for i in range(len(lines)):
+        if lines[i].startswith(prefix):
+            start = i + 1
+        elif start and lines[i].startswith("}"):
+            end = i
+            break
+
+    if not start:
+        # Unable to find subpackage function in the APKBUILD.
+        # The subpackage function could be actually missing, or this is a problem
+        # in the parser. For now we also don't handle subpackages with default
+        # functions (e.g. -dev or -doc).
+        # In the future we may want to specifically handle these, and throw
+        # an exception here for all other missing subpackage functions.
+        subpackages[subpkgname] = None
+        logging.verbose("{}: subpackage function '{}' for subpackage '{}' not found, ignoring"
+                        "".format(apkbuild["pkgname"], subpkgsplit, subpkgname))
+        return
+
+    if not end:
+        raise RuntimeError("Could not find end of subpackage function, no line starts"
+                           " with '}' after '" + prefix + "' in " + path)
+
+    lines = lines[start:end]
+    # Strip tabs before lines in function
+    lines = [line.strip() + "\n" for line in lines]
+
+    # Copy variables
+    apkbuild = apkbuild.copy()
+    apkbuild["subpkgname"] = subpkgname
+
+    # Parse relevant attributes for the subpackage
+    _parse_attributes(path, lines, pmb.config.apkbuild_package_attributes, apkbuild)
+
+    # Return only properties interesting for subpackages
+    ret = {}
+    for key in pmb.config.apkbuild_package_attributes:
+        ret[key] = apkbuild[key]
+    subpackages[subpkgname] = ret
+
+
 def apkbuild(args, path, check_pkgver=True, check_pkgname=True):
     """
     Parse relevant information out of the APKBUILD file. This is not meant
@@ -233,21 +316,7 @@ def apkbuild(args, path, check_pkgver=True, check_pkgname=True):
 
     # Parse all attributes from the config
     ret = {key: "" for key in pmb.config.apkbuild_attributes.keys()}
-    for i in range(len(lines)):
+    _parse_attributes(path, lines, pmb.config.apkbuild_attributes, ret)
-        for attribute, options in pmb.config.apkbuild_attributes.items():
-            found, value, i = parse_attribute(attribute, lines, i, path)
-            if not found:
-                continue
-
-            ret[attribute] = replace_variable(ret, value)
-
-    # Split attributes
-    for attribute, options in pmb.config.apkbuild_attributes.items():
-        if options.get("array", False):
-            # Split up arrays, delete empty strings inside the list
-            ret[attribute] = list(filter(None, ret[attribute].split(" ")))
-
-    ret = cut_off_function_names(ret)
 
     # Sanity check: pkgname
     suffix = "/" + ret["pkgname"] + "/APKBUILD"
@@ -275,39 +344,6 @@ def apkbuild(args, path, check_pkgver=True, check_pkgname=True):
     return ret
 
 
-def subpkgdesc(path, function):
-    """
-    Get the pkgdesc of a subpackage in an APKBUILD.
-
-    :param path: to the APKBUILD file
-    :param function: name of the subpackage (e.g. "nonfree_userland")
-    :returns: the subpackage's pkgdesc
-    """
-    # Read all lines
-    lines = read_file(path)
-
-    # Prefixes
-    prefix_function = function + "() {"
-    prefix_pkgdesc = "\tpkgdesc=\""
-
-    # Find the pkgdesc
-    in_function = False
-    for line in lines:
-        if in_function:
-            if line.startswith(prefix_pkgdesc):
-                return line[len(prefix_pkgdesc):-2]
-        elif line.startswith(prefix_function):
-            in_function = True
-
-    # Failure
-    if not in_function:
-        raise RuntimeError("Could not find subpackage function, no line starts"
-                           " with '" + prefix_function + "' in " + path)
-    raise RuntimeError("Could not find pkgdesc of subpackage function '" +
-                       function + "' (spaces used instead of tabs?) in " +
-                       path)
-
-
 def kernels(args, device):
     """
     Get the possible kernels from a device-* APKBUILD.
@@ -328,15 +364,13 @@ def kernels(args, device):
     # Read kernels from subpackages
     ret = {}
     subpackage_prefix = "device-" + device + "-kernel-"
-    for subpackage in subpackages:
+    for subpkgname, subpkg in subpackages.items():
-        if not subpackage.startswith(subpackage_prefix):
+        if not subpkgname.startswith(subpackage_prefix):
             continue
-        name = subpackage[len(subpackage_prefix):]
+        if subpkg is None:
-        # FIXME: We should use the specified function name here,
+            raise RuntimeError("Cannot find subpackage function for: " + subpkgname)
-        # but it's removed in cut_off_function_names()
+        name = subpkgname[len(subpackage_prefix):]
-        func = "kernel_" + name.replace('-', '_')
+        ret[name] = subpkg["pkgdesc"]
-        desc = pmb.parse._apkbuild.subpkgdesc(apkbuild_path, func)
-        ret[name] = desc
 
     # Return
     if ret:

test/test_build_package.py

@@ -136,7 +136,7 @@ def test_check_build_for_arch(monkeypatch, args):
 def test_get_depends(monkeypatch):
     func = pmb.build._package.get_depends
     apkbuild = {"pkgname": "test", "depends": ["a"], "makedepends": ["c", "b"],
-                "checkdepends": "e", "subpackages": ["d"], "options": []}
+                "checkdepends": "e", "subpackages": {"d": None}, "options": []}
 
     # Depends + makedepends
     args = args_patched(monkeypatch, ["pmbootstrap", "build", "test"])
@@ -163,7 +163,7 @@ def test_build_depends(args, monkeypatch):
     # Shortcut and fake apkbuild
     func = pmb.build._package.build_depends
     apkbuild = {"pkgname": "test", "depends": ["a"], "makedepends": ["b"],
-                "checkdepends": [], "subpackages": ["d"], "options": []}
+                "checkdepends": [], "subpackages": {"d": None}, "options": []}
 
     # No depends built (first makedepends + depends, then only makedepends)
     monkeypatch.setattr(pmb.build._package, "package", return_none)
@@ -178,7 +178,7 @@ def test_build_depends_no_binary_error(args, monkeypatch):
     # Shortcut and fake apkbuild
     func = pmb.build._package.build_depends
     apkbuild = {"pkgname": "test", "depends": ["some-invalid-package-here"],
-                "makedepends": [], "checkdepends": [], "subpackages": [],
+                "makedepends": [], "checkdepends": [], "subpackages": {},
                 "options": []}
 
     # pmbootstrap build --no-depends

test/test_helpers_package.py

@@ -49,7 +49,7 @@ def test_helpers_package_get_pmaports_and_cache(args, monkeypatch):
                 "provides": ["testprovide"],
                 "options": [],
                 "checkdepends": [],
-                "subpackages": [],
+                "subpackages": {},
                 "makedepends": [],
                 "pkgver": "1.0",
                 "pkgrel": "1"}

test/test_parse_apkbuild.py

@@ -38,26 +38,36 @@ def args(tmpdir, request):
     return args
 
 
-def test_subpkgdesc():
+def test_subpackages(args):
-    func = pmb.parse._apkbuild.subpkgdesc
     testdata = pmb_src + "/test/testdata"
 
+    path = testdata + "/apkbuild/APKBUILD.subpackages"
+    apkbuild = pmb.parse.apkbuild(args, path, check_pkgname=False)
+
+    subpkg = apkbuild["subpackages"]["simple"]
+    assert subpkg["pkgdesc"] == ""
+    # Inherited from parent package
+    assert subpkg["depends"] == ["postmarketos-base"]
+
+    subpkg = apkbuild["subpackages"]["custom"]
+    assert subpkg["pkgdesc"] == "This is one of the custom subpackages"
+    assert subpkg["depends"] == ["postmarketos-base", "glibc"]
+
     # Successful extraction
     path = (testdata + "/init_questions_device/aports/device/"
             "device-nonfree-firmware/APKBUILD")
-    pkgdesc = "firmware description"
+    apkbuild = pmb.parse.apkbuild(args, path)
-    assert func(path, "nonfree_firmware") == pkgdesc
+    subpkg = apkbuild["subpackages"]["device-nonfree-firmware-nonfree-firmware"]
-
+    assert subpkg["pkgdesc"] == "firmware description"
-    # Can't find the function
-    with pytest.raises(RuntimeError) as e:
-        func(path, "invalid_function")
-    assert str(e.value).startswith("Could not find subpackage function")
 
     # Can't find the pkgdesc in the function
     path = testdata + "/apkbuild/APKBUILD.missing-pkgdesc-in-subpackage"
-    with pytest.raises(RuntimeError) as e:
+    apkbuild = pmb.parse.apkbuild(args, path, check_pkgname=False)
-        func(path, "subpackage")
+    subpkg = apkbuild["subpackages"]["missing-pkgdesc-in-subpackage-subpackage"]
-    assert str(e.value).startswith("Could not find pkgdesc of subpackage")
+    assert subpkg["pkgdesc"] == ""
+
+    # Can't find the function
+    assert apkbuild["subpackages"]["invalid-function"] is None
 
 
 def test_kernels(args):
@@ -135,5 +145,10 @@ def test_parse_attributes():
 def test_variable_replacements(args):
     path = pmb_src + "/test/testdata/apkbuild/APKBUILD.variable-replacements"
     apkbuild = pmb.parse.apkbuild(args, path, check_pkgname=False)
+    assert apkbuild["pkgdesc"] == "this should not affect variable replacement"
     assert apkbuild["url"] == "replacements variable string-replacements"
-    assert apkbuild["subpackages"] == ["replacements", "test"]
+    assert list(apkbuild["subpackages"].keys()) == ["replacements", "test"]
+
+    assert apkbuild["subpackages"]["replacements"] is None
+    test_subpkg = apkbuild["subpackages"]["test"]
+    assert test_subpkg["pkgdesc"] == "this should not affect variable replacement"

test/testdata/apkbuild/APKBUILD.missing-pkgdesc-in-subpackage

@@ -1,6 +1,7 @@
 # Reference: <https://postmarketos.org/devicepkg>
 pkgname="missing-pkgdesc-in-subpackage"
-subpackages="$pkgname-subpackage"
+arch="noarch"
+subpackages="$pkgname-subpackage invalid-function:does_not_exist"
 
 subpackage() {
 	# this function does not have a pkgdesc

test/testdata/apkbuild/APKBUILD.subpackages

@@ -0,0 +1,13 @@
+pkgname="subpackages"
+arch="noarch"
+subpackages="simple custom:custom_function"
+depends="postmarketos-base"
+
+simple() {
+	mkdir "$subpkgdir"
+}
+
+custom_function() {
+	pkgdesc="This is one of the custom $pkgname"
+	depends="$depends glibc"
+}

test/testdata/apkbuild/APKBUILD.variable-replacements

@@ -4,5 +4,9 @@ pkgrel=0
 arch="armhf"
 pkgdesc="$pkgdesc$pkgname test"
 url="${pkgname/variable-} ${pkgname/-replacements/} ${pkgname/variable/string}"
-subpackages="${pkgdesc#variable-}"
+subpackages="${pkgdesc#variable-}:test_subpkg_func"
-pkgdesc="this should not affect anything"
+pkgdesc="this should not affect variable replacement"
+
+test_subpkg_func() {
+	mkdir "$subpkgdir"
+}

test/testdata/init_questions_device/aports/device/device-wileyfox-crackling/APKBUILD

@@ -12,7 +12,7 @@ makedepends="devicepkg-dev"
 subpackages="
 	$pkgname-kernel-mainline:kernel_mainline
         $pkgname-kernel-mainline-modem:kernel_mainline_modem
-	$pkgname-kernel-downstream:kernel_downstream
+	$pkgname-kernel-downstream:downstream
 	$pkgname-nonfree-firmware:nonfree_firmware
         $pkgname-nonfree-firmware-modem:nonfree_firmware_modem
 "
@@ -39,7 +39,8 @@ kernel_mainline_modem() {
 	devicepkg_subpackage_kernel $startdir $pkgname $subpkgname
 }
 
-kernel_downstream() {
+# This is renamed to test the APKBUILD parser. In reality they should always use proper names.
+downstream() {
 	pkgdesc="Downstream kernel"
 	depends="linux-wileyfox-crackling mesa-dri-swrast mdss-fb-init-hack"
 	devicepkg_subpackage_kernel $startdir $pkgname $subpkgname