CI: add openrc subpackages check (MR 1807)

Make calls to 'rc-update add' consistent in post-install scripts, so we
can lint them.

Notes:
* msm-modem-downstream.post-install: it defaults to 'sysinit', not
  'default' as one might expect.
* remove some obvious 'enable FOO openrc service' comments while at it

.gitlab-ci/build_changed_aports.py

@@ -5,6 +5,7 @@ import sys
 
 # Same dir
 import common
+import openrc_subpackages
 
 # pmbootstrap
 import testcases.add_pmbootstrap_to_import_path
@@ -81,3 +82,6 @@ if __name__ == "__main__":
     # Build packages
     print(f"building in strict mode for {arch}: {', '.join(packages)}")
     build_strict(packages, arch)
+
+    # Do tests with built packages
+    openrc_subpackages.run_test(arch, packages)

.gitlab-ci/openrc_subpackages.py

@@ -0,0 +1,344 @@
+#!/usr/bin/env python3
+# Copyright 2020 Oliver Smith
+# SPDX-License-Identifier: GPL-3.0-or-later
+#
+# Find packages that enable openrc services in their post-install scripts,
+# but do not depend on -openrc subpackages:
+# https://wiki.postmarketos.org/wiki/Packaging#OpenRC_subpackages
+#
+# run_test() gets called from build_changed_aports.py in CI. For debugging,
+# start this script like a command-line program.
+import argparse
+import os
+import glob
+import sys
+
+# Same dir
+import common
+
+# pmbootstrap
+import testcases.add_pmbootstrap_to_import_path
+import pmb.parse.arch
+import pmb.helpers.package
+
+# Subset of runlevels we use in postmarketOS
+runlevels_valid = ["boot", "default", "shutdown", "sysinit"]
+
+
+def post_install_parse_services(path):
+    """ Get services as listed with 'rc-update add <service> <runlevel>' lines
+        in post-install scripts.
+        :param path: full path to post-install file
+        :returns: (errors, services):
+                  * errors: list of error strings
+                  * list of services
+    """
+    errors_all = []
+    services = []
+    with open(path, encoding="utf-8") as handle:
+        errors = []
+        for i, line in enumerate(handle.readlines()):
+            # Remove comments
+            line = line.split("#", 2)[0].rstrip()
+            if "rc-update add" not in line \
+                    and "rc-update -q add" not in line:
+                continue
+
+            # Have nothing before the line
+            if not line.startswith("rc-update"):
+                errors.append(f"{i}: line does not start with 'rc-update'")
+
+            # Have no shell magic
+            for char in ["$", "'", "\"", "\\"]:
+                if char in line:
+                    errors.append(f"{i}: line contains illegal character:"
+                                  f" {char}")
+
+            # Parse format 'rc-update add <service> <runlevel>'
+            service = None
+            runlevel = None
+            try:
+                _, _, service, runlevel = line.split(" ")
+            except:
+                errors.append(f"{i}: line does not follow format 'rc-update"
+                              " add <service> <runlevel>'")
+
+            # Validate runlevel
+            if runlevel and runlevel not in runlevels_valid:
+                errors.append(f"{i}: invalid runlevel '{runlevel}', expected"
+                              f" one of: {', '.join(runlevels_valid)}")
+
+            errors_all += errors
+            if not errors:
+                services.append(service)
+    return (errors_all, services)
+
+
+def run_lint():
+    print("linting *.post-install regarding 'rc-update add'")
+    pmaports = common.get_pmaports_dir()
+    pattern = f"{pmaports}/**/*.post-install"
+    count = 0
+    for path in glob.glob(pattern, recursive=True):
+        errors, _ = post_install_parse_services(path)
+
+        if not errors:
+            continue
+
+        count += len(errors)
+        print(f"{os.path.relpath(path, pmaports)}:")
+        for error in errors:
+            print(f"  {error}")
+
+    if count == 0:
+        print("success!")
+        return
+
+    print("")
+    print(f"{count} errors found.")
+    print("")
+    print("pmaports.git has strict rules for 'rc-update add' lines in")
+    print("post-install files. Please fix the errors above.")
+    print("Related: https://wiki.postmarketos.org/wiki/Packaging")
+    print("")
+    sys.exit(1)
+
+
+def get_relevant_post_install_files(packages):
+    """ Get all post-install files from the given packages and their
+        subpackages, which have 'rc-update add' lines. """
+    ret = []
+    for pkgname in packages:
+        pattern = f"{common.get_pmaports_dir()}/**/{pkgname}/*.post-install"
+        for path in glob.glob(pattern, recursive=True):
+            errors, services = post_install_parse_services(path)
+            if services:
+                ret.append(path)
+
+    return ret
+
+
+def get_service_owner(chroot_arg, service):
+    """ Use apk to figure out which package provides a service file.
+        :param chroot_arg: chroot action + suffix argument that will be passed
+                           to pmbootstrap
+        :param service: name of the service to check, e.g. "bluetooth"
+    """
+    path = f"/etc/init.d/{service}"
+    apk_output = common.run_pmbootstrap(["-q"] + chroot_arg + ["--", "apk",
+                                        "info", "-W", path],
+                                        output_return=True)
+
+    # example: '/etc/init.d/tinydm is owned by tinydm-openrc-1.0.2-r0'
+    prefix = f"{path} is owned by "
+    try:
+        assert apk_output.startswith(prefix)
+        split = apk_output[len(prefix):].rstrip().split("-")
+        assert split[-1].startswith("r")
+        ret = "-".join(split[:-2])
+    except:
+        raise RuntimeError(f"Failed to parse 'apk info -W {path}' output:"
+                           f" '{apk_output}'")
+    return ret
+
+
+def get_package_depends(chroot_arg, pkgname):
+    """ Use apk to figure out the depends of a specific package or subpackage.
+
+        Example output from 'apk info -R':
+            postmarketos-ui-phosh-6-r3 depends on:
+            bluez
+            gnome-keyring
+            iio-sensor-proxy
+            phosh
+            polkit-elogind
+
+            postmarketos-ui-phosh-6-r2 depends on:
+            ... (we only care about the top most, newest package)
+
+        :param chroot_arg: chroot action + suffix argument that will be passed
+                           to pmbootstrap
+        :param pkgname: package or subpackage name
+        :returns: list of depends, e.g. ["bluez", "gnome-keyring", ...] """
+    apk_output = common.run_pmbootstrap(["-q"] + chroot_arg + ["--", "apk",
+                                        "info", "-R", pkgname],
+                                        output_return=True)
+    ret = []
+    try:
+        lines = apk_output.split("\n")
+        assert lines[0].startswith(f"{pkgname}-")
+        assert lines[0].endswith(" depends on:")
+        for line in lines[1:]:
+            assert not line.endswith(" depends on:")
+
+            # Only parse the top package from the list
+            if not line:
+                break
+            ret.append(line)
+    except:
+        raise RuntimeError(f"Failed to parse 'apk info -R {pkgname}' output:"
+                           f" '{apk_output}'")
+    return ret
+
+
+def run_test_package(zap, work, arch, pkgname, path):
+    """ Run the test for a single package.
+        :param zap: bool: run 'pmbootstrap zap' before testing the next pkg
+        :param work: path to pmbootstrap work dir
+        :param arch: architecture to test
+        :param pkgname: package name
+        :param path: full path to the .post-install file
+        :returns: (warnings, errors):
+                  * warnings: list of non-fatal warning strings
+                  * errors: list of fatal error strings
+    """
+    errors = []
+    warnings = []
+
+    # Set proper chroot suffix for arch
+    chroot_arg = ["chroot"]
+    suffix = "native"
+    if arch != pmb.parse.arch.alpine_native():
+        chroot_arg += f"-b{arch}"
+        suffix = "buildroot_{arch}"
+
+    # Install package with depends
+    if zap:
+        common.run_pmbootstrap(["-y", "zap"])
+    common.run_pmbootstrap(chroot_arg + ["--", "apk", "add", pkgname])
+
+    # Parse depends
+    depends = get_package_depends(chroot_arg, pkgname)
+    print(f"{pkgname}: depends: {depends}")
+
+    # Verify each service
+    _, services = post_install_parse_services(path)
+    etc_initd = f"{work}/chroot_{suffix}/etc/init.d"
+    for service in services:
+        # init.d file must exist
+        if not os.path.exists(f"{etc_initd}/{service}"):
+            errors.append("service mentioned in post-install was not"
+                          f" installed: '{service}' (missing depend or typo?)")
+            continue
+
+        owner = get_service_owner(chroot_arg, service)
+        print(f"{service}: owned by: {owner}")
+
+        # No need to explicitly add openrc to every package
+        if owner == "openrc":
+            continue
+
+        expected_depends = [owner]
+        if owner.endswith("-openrc"):
+            # Must depend on the same package without '-openrc' too, otherwise
+            # only the service file gets installed but not the daemon itself
+            expected_depends += ["-".join(owner.split("-")[:-1])]
+        else:
+            warnings += [f"service '{service}' is owned by package '{owner}',"
+                         f" which does not end in '-openrc'"]
+
+        for depend in expected_depends:
+            if depend == pkgname:
+                continue
+            if depend not in depends:
+                errors += [f"{pkgname} must depend on '{depend}', because it"
+                           f" has 'rc-update add {service}' in post-install"]
+
+    return warnings, errors
+
+
+def print_summary(category, pkgname_strings):
+    """ Print a summary of errors or warnings.
+        :param category: "error" or "warning"
+        :param pkgname_strings: dict like: {"pkgname1": ["first", ...] """
+    print()
+    count = sum([len(pkgname_strings[p]) for p in pkgname_strings])
+    print(f"=== {count} {category}(s) ===")
+    for pkgname in pkgname_strings:
+        print(f"{pkgname}:")
+        for string in pkgname_strings[pkgname]:
+            print(f"* {string}")
+
+
+def run_test(arch, packages, zap=True):
+    """ Run the test on multiple packages
+        :param arch: architecture to test
+        :param packages: list of pkgnames to test
+        :param zap: bool: run 'pmbootstrap zap' before testing the next pkg
+    """
+    print(f"running missing -openrc subpackages check for {arch}:"
+          f" {', '.join(packages)}")
+
+    # Get pmbootstrap's args for args.work
+    sys.argv = ["pmbootstrap", "chroot"]
+    args = pmb.parse.arguments()
+
+    paths = get_relevant_post_install_files(packages)
+    count = len(paths)
+
+    if not count:
+        print("no post-install with 'rc-update add' found, nothing to do!")
+        return
+
+    errors_all = {}
+    warnings_all = {}
+    for i, path in enumerate(paths):
+        # Parse pkgname from .post-install filename (main or subpackage)
+        pkgname, _ = os.path.splitext(os.path.basename(path))
+        print(f"=== ({i+1}/{count}) {pkgname} ===")
+        warnings, errors = run_test_package(zap, args.work, arch, pkgname,
+                                            path)
+
+        if errors:
+            errors_all[pkgname] = errors
+        if warnings:
+            warnings_all[pkgname] = warnings
+
+    # Print summaries
+    if warnings_all:
+        print_summary("warning", warnings_all)
+        print("NOTE: if you add -openrc subpackages to Alpine's aports, add"
+              " the -openrc dependency to the postmarketOS packages which"
+              " enable the package's service in post-install right"
+              " afterwards.")
+    if errors_all:
+        print_summary("error", errors_all)
+        sys.exit(1)
+
+    print("=== check passed ===")
+
+
+if __name__ == "__main__":
+    parser = argparse.ArgumentParser(description="Run all *.post-install files"
+                                     " from pmaports through run_lint()")
+    parser.add_argument("--test-pkgs", nargs="*", metavar="PKGNAME",
+                        help="additionally, do run_test() for these packages,"
+                             " or all packages if no PKGNAME is specified")
+    parser.add_argument("--test-arch", default="x86_64", metavar="ARCH",
+                        help="architecture for run_test() (default: x86_64)")
+    parser.add_argument("--test-no-zap", dest="zap", action="store_false",
+                        help="disable 'pmbootstrap zap' before installing"
+                             " the next package to test")
+    args = parser.parse_args()
+
+    run_lint()
+
+    if getattr(args, "test_pkgs", None) is None:
+        sys.exit(0)
+
+    print("---")
+    pkgs = args.test_pkgs
+
+    # Default to all packages for given arch
+    if not pkgs:
+        sys.argv = ["pmbootstrap", "chroot"]
+        pmb_args = pmb.parse.arguments()
+        pmaports = common.get_pmaports_dir()
+
+        for path in glob.glob(f"{pmaports}/**/*.post-install", recursive=True):
+            pkgname = os.path.basename(os.path.dirname(path))
+            if pmb.helpers.package.check_arch(pmb_args, pkgname,
+                                              args.test_arch, binary=False):
+                pkgs += [pkgname]
+
+    run_test(args.test_arch, pkgs, args.zap)

.gitlab-ci/run_testcases.sh

@@ -31,5 +31,9 @@ if ! [ -e $deviceinfo ]; then
 	exit 1
 fi
 
+# Lint OpenRC services in *.post-install early (another test from this script
+# can only be done after building, gets called from build_change_aports.py)
+"$pmaports"/.gitlab-ci/openrc_subpackages.py
+
 # Run testcases
 pytest -vv -x --tb=native "$pmaports/.gitlab-ci/testcases" "$@"

device/community/device-nokia-n900/APKBUILD

@@ -3,7 +3,7 @@
 # Co-Maintainer: Daniele Debernardi <drebrez@gmail.com>
 pkgname=device-nokia-n900
 pkgver=7
-pkgrel=0
+pkgrel=1
 pkgdesc="Nokia N900"
 url="https://postmarketos.org"
 arch="armv7"

device/community/device-nokia-n900/device-nokia-n900.post-install

@@ -5,14 +5,11 @@ for i in $(awk '{if($NF && ($1 !~ /^#/)) print $NF}' /etc/acpi.map); do
         ln -s /etc/acpi/handler.sh /etc/acpi/$i
 done
 
-# Enable acpid
 rc-update add acpid default
 # hwdrivers is required for acpid to 'see' all devices
 rc-update add hwdrivers boot
-# Enable ofono
 rc-update add ofono default
-# Enable swclock
-rc-update -q add swclock boot
+rc-update add swclock boot
 
 # Load nokia-modem module on boot
 NMC=/etc/modules-load.d/nokia-modem.conf

device/testing/device-samsung-i9070/APKBUILD

@@ -1,7 +1,7 @@
 # Reference: <https://postmarketos.org/devicepkg>
 pkgname=device-samsung-i9070
 pkgver=1
-pkgrel=32
+pkgrel=33
 pkgdesc="Samsung Galaxy S Advance"
 url="https://postmarketos.org"
 arch="armhf"

device/testing/device-samsung-i9070/device-samsung-i9070.post-install

@@ -1,6 +1,5 @@
 #!/bin/sh
 
-# Enable swclock OpenRC service
-rc-update -q add swclock boot
+rc-update add swclock boot
 
 exit 0

device/testing/device-samsung-klte/APKBUILD

@@ -2,7 +2,7 @@
 pkgname=device-samsung-klte
 pkgdesc="Samsung Galaxy S5"
 pkgver=0.3
-pkgrel=13
+pkgrel=14
 url="https://postmarketos.org"
 license="MIT"
 arch="armv7"

device/testing/device-samsung-klte/device-samsung-klte-kernel-downstream.post-install

@@ -1,7 +1,7 @@
 #!/bin/sh
 
 # Enable autostart of deferred initcalls (needed to bring Wi-Fi online)
-rc-update -q add deferred-initcalls sysinit
+rc-update add deferred-initcalls sysinit
 
 # Explicitly specify wlan device. Without this, p2p0 will be
 # selected automatically instead of wlan0, and Wi-Fi will not work.
@@ -11,5 +11,5 @@ fi
 
 # Replace sshd with dropbear (workaround for sshd failure on downstream
 # kernels, see issue #808 https://gitlab.com/postmarketOS/pmaports/-/issues/808 )
-rc-update -q del sshd default
-rc-update -q add dropbear default
+rc-update del sshd default
+rc-update add dropbear default

device/testing/device-xiaomi-armani/APKBUILD

@@ -2,7 +2,7 @@
 pkgname=device-xiaomi-armani
 pkgdesc="Redmi 1S"
 pkgver=0.1
-pkgrel=11
+pkgrel=12
 url="https://postmarketos.org"
 license="MIT"
 arch="armhf"

device/testing/device-xiaomi-armani/device-xiaomi-armani.post-install

@@ -1,3 +1,5 @@
 #!/bin/sh
-# Enable openrc service to load wlan module for wifi functionality
-rc-update add wlanarmani default || true
+
+rc-update add wlanarmani default
+
+exit 0

device/testing/firmware-samsung-kminilte/APKBUILD

@@ -1,6 +1,6 @@
 pkgname=firmware-samsung-kminilte
 pkgver=1
-pkgrel=0
+pkgrel=1
 pkgdesc="Samsung Galaxy S5 Mini firmware"
 url="https://github.com/cm-3470/android_vendor_samsung_kminilte"
 arch="armv7"

device/testing/firmware-samsung-kminilte/firmware-samsung-kminilte.post-install

@@ -1,7 +1,7 @@
 #!/bin/sh
 
 # Enable autostart of deferred initcalls (needed to bring Wi-Fi online)
-rc-update -q add deferred-initcalls sysinit
+rc-update add deferred-initcalls sysinit
 
 # Explicitly specify wlan device. Without this, p2p0 will be
 # selected automatically instead of wlan0, and Wi-Fi will not work.

main/msm-fb-refresher/APKBUILD

@@ -1,7 +1,7 @@
 # Maintainer: Clayton Craft <clayton@craftyguy.net>
 pkgname=msm-fb-refresher
 pkgver=0.2
-pkgrel=3
+pkgrel=4
 pkgdesc="Display refresher for Qualcomm devices"
 arch="all"
 url="https://github.com/AsteroidOS/msm-fb-refresher"

main/msm-fb-refresher/msm-fb-refresher.post-install

@@ -1,3 +1,3 @@
 #!/bin/sh
 
-rc-update -q add msm-fb-refresher boot
+rc-update add msm-fb-refresher boot

main/postmarketos-base/APKBUILD

@@ -1,6 +1,6 @@
 pkgname=postmarketos-base
-pkgver=5
-pkgrel=3
+pkgver=6
+pkgrel=0
 pkgdesc="Meta package for minimal postmarketOS base"
 url="https://postmarketos.org"
 arch="noarch"

main/postmarketos-base/postmarketos-base.post-install

@@ -1,26 +1,44 @@
 #!/bin/sh -e
+# === OpenRC services ===
+# * Order by runlevel, then alphabetically
+# * Not written as loop, so we can verify that related -openrc packages are in
+#   depends: https://wiki.postmarketos.org/wiki/Packaging#OpenRC_subpackages
 
-# Enable udev OpenRC services and runlevels
-# https://github.com/alpinelinux/aports/blob/master/main/eudev/setup-udev
-setup-udev -n
+# sysinit
+rc-update add devfs sysinit
+rc-update add devmappings sysinit
+rc-update add dmesg sysinit
+rc-update add udev sysinit
+rc-update add udev-trigger sysinit
 
-# Enable other OpenRC services
-for service in devfs dmesg devmappings; do
-	rc-update -q add $service sysinit
-done
-for service in hwclock modules sysctl hostname bootmisc syslog; do
-	rc-update -q add $service boot
-done
-for service in dbus haveged sshd swapfile wpa_supplicant chronyd local networkmanager rfkill; do
-	rc-update -q add $service default
-done
-for service in mount-ro killprocs savecache; do
-	rc-update -q add $service shutdown
-done
+# boot
+rc-update add bootmisc boot
+rc-update add hostname boot
+rc-update add hwclock boot
+rc-update add modules boot
+rc-update add sysctl boot
+rc-update add syslog boot
 
+# default
+rc-update add chronyd default
+rc-update add dbus default
+rc-update add haveged default
+rc-update add local default
+rc-update add networkmanager default
+rc-update add rfkill default
+rc-update add sshd default
+rc-update add swapfile default
+rc-update add udev-postmount default
+rc-update add wpa_supplicant default
+
+# shutdown
+rc-update add mount-ro shutdown
+rc-update add killprocs shutdown
+rc-update add savecache shutdown
+
+# === Other ===
 # Prime swclock with a reasonable date/time on first boot
 mkdir -p /run/openrc
 touch /run/openrc/shutdowntime
 
 exit 0
-

main/watchdog-kick/APKBUILD

@@ -1,6 +1,6 @@
 pkgname=watchdog-kick
 pkgver=0.1
-pkgrel=1
+pkgrel=2
 pkgdesc="Periodically kich watchdogs"
 url="https://postmarketos.org"
 arch="noarch"

main/watchdog-kick/watchdog-kick.post-install

@@ -1,3 +1,3 @@
 #!/bin/sh
 
-rc-update -q add watchdog-kick sysinit
+rc-update add watchdog-kick sysinit

modem/msm-modem/APKBUILD

@@ -1,6 +1,6 @@
 pkgname=msm-modem
 pkgver=3
-pkgrel=0
+pkgrel=1
 pkgdesc="Common support for Qualcomm MSM modems"
 url="https://postmarketos.org/"
 arch="armhf armv7 aarch64"

modem/msm-modem/msm-modem-downstream.post-install

@@ -1,2 +1,2 @@
 #!/bin/sh
-rc-update add msm-modem-downstream
+rc-update add msm-modem-downstream sysinit

sxmo/sxmo-utils/APKBUILD

@@ -1,7 +1,7 @@
 # Maintainer: Miles Alan <m@milesalan.com>
 pkgname=sxmo-utils
 pkgver=1.1.11
-pkgrel=1
+pkgrel=2
 pkgdesc="Utility scripts, programs, and configs that hold the Sxmo UI environment together"
 url="https://git.sr.ht/~mil/sxmo-utils"
 arch="all"

sxmo/sxmo-utils/sxmo-utils.post-install

@@ -1,3 +1,5 @@
 #!/bin/sh
 
-rc-update add modemmanager default || echo "please add modemmanager to the default runlevel"
+rc-update add modemmanager default
+
+exit 0