aportgen: Gracefully handle old aports_upstream (#1291)

In order to get cross-compilers, we generate a few aports (e.g.
binutils-armhf, gcc-armhf) automatically from Alpine's aports.
pmbootstrap was already able to perform a git checkout of Alpine's
aports repository. But it needed to be manually updated. Otherwise
the `pmbootstrap aportgen` command could actually downgrade the aport
instead of updating it to the current version.

After thinking about adding a dedicated pmbootstrap command for
updating git repositories, I thought it would be better to not open
that can of worms (pmbootstrap as general git wrapper? no thanks).

The solution implemented here compares the upstream aport version of
the git checkout of a certain package (e.g. gcc for gcc-armhf) with the
version in Alpine's binary package APKINDEX. When the aport version is
lower than the binary package version, it shows the user how to update
the git repository with just one command:

    pmbootstrap chroot --add=git --user -- \
        git -C /mnt/pmbootstrap-git/aports_upstream pull

Changes:
* `pmb.aportgen.core.get_upstream_aport()`: new function, that returns
  the absolute path to the upstream aport on disk, after checking the
  version of the aport against the binary package.
* Use that new function in pmb.aportgen.gcc and pmb.aportgen.binutils
* New function `pmb.helpers.repo.alpine_apkindex_path()`: updates the
  APKINDEX if necessary and returns the absolute path to the APKINDEX.
  This code was basically present already, but not as function, so now
  we have a bit less overhead there.
* `pmbootstrap chroot`: new `--user` argument
* `pmb.parse.apkbuild`: make pkgname check optional, as it fails with
  the official gcc APKBUILD before we modify it (the current APKBUILD
  parser is not meant to be perfect, as this would require a full shell
  parsing implementation).
* Extended `test_aportgen.py` and enabled it by default in
  `testcases_fast.sh`.  Previously it was disabled due to traffic
  concerns (cloning the aports repo, but then again we do a full KDE
  plasma mobile installation in Travis now, so that shouldn't matter
  too much).
* `testcases_fast.sh`: With "test_aport_in_sync_with_git" removed
  from the disabled-by-default list (left over from timestamp based
  rebuilds), there were no more test cases disabled by default. I've
  changed it, so now the qemu_running_processes test case is disabled,
  and added an `--all` parameter to the script to disable no test
  cases. Travis runs with the `--all` parameter while it's useful to
  do a quick local test without `--all` in roughly 2 minutes instead of
  10.
* `aports/cross/binutils-*`: Fix `_mirror` variable to point to current
  default Alpine mirror (so the aportgen testcase runs through).

.travis.yml

@@ -13,7 +13,7 @@ script:
   - yes "" | ./pmbootstrap.py init
   - ./pmbootstrap.py kconfig_check
   - test/check_checksums.py --build
-  - test/testcases_fast.sh
+  - test/testcases_fast.sh --all
 after_success:
   - coveralls
 after_failure:

pmb/aportgen/binutils.py

@@ -24,9 +24,7 @@ import pmb.helpers.run
 def generate(args, pkgname):
     # Copy original aport
     arch = pkgname.split("-")[1]
-    path_original = "main/binutils"
+    upstream = pmb.aportgen.core.get_upstream_aport(args, "main/binutils")
-    upstream = (args.work + "/cache_git/aports_upstream/" + path_original)
-    pmb.helpers.git.clone(args, "aports_upstream")
     pmb.helpers.run.user(args, ["cp", "-r", upstream, args.work + "/aportgen"])
 
     # Architectures to build this package for
@@ -74,5 +72,5 @@ def generate(args, pkgname):
         "gold": None,
     }
 
-    pmb.aportgen.core.rewrite(args, pkgname, path_original, fields, "binutils",
+    pmb.aportgen.core.rewrite(args, pkgname, "main/binutils", fields,
-                              replace_functions)
+                              "binutils", replace_functions)

pmb/aportgen/core.py

@@ -17,6 +17,8 @@ You should have received a copy of the GNU General Public License
 along with pmbootstrap.  If not, see <http://www.gnu.org/licenses/>.
 """
 import fnmatch
+import logging
+import pmb.helpers.git
 
 
 def format_function(name, body, remove_indent=4):
@@ -117,3 +119,47 @@ def rewrite(args, pkgname, path_original, fields={}, replace_pkgname=None,
         handle.seek(0)
         handle.write("".join(lines_new))
         handle.truncate()
+
+
+def get_upstream_aport(args, upstream_path):
+    """
+    Perform a git checkout of Alpine's aports and get the path to the aport.
+
+    :param upstream_path: where the aport is in the git repository, e.g.
+                          "main/gcc"
+    :returns: absolute path on disk where the Alpine aport is checked out
+              example: /opt/pmbootstrap_work/cache_git/aports/upstream/main/gcc
+    """
+    # APKBUILD
+    pmb.helpers.git.clone(args, "aports_upstream")
+    aport_path = (args.work + "/cache_git/aports_upstream/" + upstream_path)
+    apkbuild = pmb.parse.apkbuild(args, aport_path + "/APKBUILD",
+                                  check_pkgname=False)
+    apkbuild_version = apkbuild["pkgver"] + "-r" + apkbuild["pkgrel"]
+
+    # Binary package
+    split = upstream_path.split("/", 1)
+    repo = split[0]
+    pkgname = split[1]
+    index_path = pmb.helpers.repo.alpine_apkindex_path(args, repo)
+    package = pmb.parse.apkindex.package(args, pkgname, indexes=[index_path])
+
+    # Compare version (return when equal)
+    compare = pmb.parse.version.compare(apkbuild_version, package["version"])
+    if compare == 0:
+        return aport_path
+
+    # Different version message
+    logging.error("ERROR: Package '" + pkgname + "' has a different version in"
+                  " local checkout of Alpine's aports (" + apkbuild_version +
+                  ") compared to Alpine's binary package (" +
+                  package["version"] + ")!")
+
+    # APKBUILD < binary
+    if compare == -1:
+        raise RuntimeError("You can update your local checkout with:"
+                           " 'pmbootstrap chroot --add=git --user -- git -C"
+                           " /mnt/pmbootstrap-git/aports_upstream pull'")
+    # APKBUILD > binary
+    raise RuntimeError("You can force an update of your binary package"
+                       " APKINDEX files with: 'pmbootstrap update'")

pmb/aportgen/gcc.py

@@ -24,9 +24,7 @@ import pmb.helpers.run
 def generate(args, pkgname):
     # Copy original aport
     arch = pkgname.split("-")[1]
-    path_original = "main/gcc"
+    upstream = pmb.aportgen.core.get_upstream_aport(args, "main/gcc")
-    upstream = (args.work + "/cache_git/aports_upstream/" + path_original)
-    pmb.helpers.git.clone(args, "aports_upstream")
     pmb.helpers.run.user(args, ["cp", "-r", upstream, args.work + "/aportgen"])
 
     # Architectures to build this package for
@@ -105,10 +103,6 @@ def generate(args, pkgname):
         '*package() {*': "_package() {"
     }
 
-    pmb.aportgen.core.rewrite(
+    pmb.aportgen.core.rewrite(args, pkgname, "main/gcc", fields,
-        args,
+                              replace_simple=replace_simple,
-        pkgname,
+                              below_header=below_header)
-        path_original,
-        fields,
-        replace_simple=replace_simple,
-        below_header=below_header)

pmb/chroot/apk_static.py

@@ -159,16 +159,13 @@ def init(args):
     """
     Download, verify, extract $WORK/apk.static.
     """
-    # Get the APKINDEX
+    # Get and parse the APKINDEX
-    pmb.helpers.repo.update(args, args.arch_native)
+    apkindex = pmb.helpers.repo.alpine_apkindex_path(args, "main")
-    url = args.mirror_alpine + args.alpine_version + "/main"
-    apkindex = (args.work + "/cache_apk_" + args.arch_native + "/APKINDEX." +
-                pmb.helpers.repo.hash(url) + ".tar.gz")
-
-    # Extract and verify the apk-tools-static version
     index_data = pmb.parse.apkindex.package(args, "apk-tools-static",
                                             indexes=[apkindex])
     version = index_data["version"]
+
+    # Extract and verify the apk-tools-static version
     version_min = pmb.config.apk_tools_static_min_version
     apk_name = "apk-tools-static-" + version + ".apk"
     if pmb.parse.version.compare(version, version_min) == -1:

pmb/helpers/frontend.py

@@ -147,12 +147,26 @@ def checksum(args):
 
 
 def chroot(args):
+    # Suffix
     suffix = _parse_suffix(args)
+    if (args.user and suffix != "native" and
+            not suffix.startswith("buildroot_")):
+        raise RuntimeError("--user is only supported for native or"
+                           " buildroot_* chroots.")
+
+    # apk: check minimum version, install packages
     pmb.chroot.apk.check_min_version(args, suffix)
     if args.add:
         pmb.chroot.apk.install(args, args.add.split(","), suffix)
-    logging.info("(" + suffix + ") % " + " ".join(args.command))
+
-    pmb.chroot.root(args, args.command, suffix, log=False)
+    # Run the command as user/root
+    if args.user:
+        logging.info("(" + suffix + ") % su pmos -c '" +
+                     " ".join(args.command) + "'")
+        pmb.chroot.user(args, args.command, suffix, log=False)
+    else:
+        logging.info("(" + suffix + ") % " + " ".join(args.command))
+        pmb.chroot.root(args, args.command, suffix, log=False)
 
 
 def config(args):

pmb/helpers/repo.py

@@ -170,3 +170,26 @@ def update(args, arch=None, force=False, existing_only=False):
         pmb.helpers.run.root(args, ["cp", temp, target])
 
     return True
+
+
+def alpine_apkindex_path(args, repo="main", arch=None):
+    """
+    Get the path to a specific Alpine APKINDEX file on disk and download it if
+    necessary.
+
+    :param repo: Alpine repository name (e.g. "main")
+    :param arch: Alpine architecture (e.g. "armhf"), defaults to native arch.
+    :returns: full path to the APKINDEX file
+    """
+    # Repo sanity check
+    if repo not in ["main", "community", "testing", "non-free"]:
+        raise RuntimeError("Invalid Alpine repository: " + repo)
+
+    # Download the file
+    update(args, arch)
+
+    # Find it on disk
+    arch = arch or args.arch_native
+    repo_link = args.mirror_alpine + args.alpine_version + "/" + repo
+    cache_folder = args.work + "/cache_apk_" + arch
+    return cache_folder + "/APKINDEX." + hash(repo_link) + ".tar.gz"

pmb/parse/_apkbuild.py

@@ -81,7 +81,7 @@ def cut_off_function_names(apkbuild):
     return apkbuild
 
 
-def apkbuild(args, path, check_pkgver=True):
+def apkbuild(args, path, check_pkgver=True, check_pkgname=True):
     """
     Parse relevant information out of the APKBUILD file. This is not meant
     to be perfect and catch every edge case (for that, a full shell parser
@@ -89,7 +89,8 @@ def apkbuild(args, path, check_pkgver=True):
     covered by pmbootstrap and not take too long.
 
     :param path: full path to the APKBUILD
-    :param version_check: verify that the pkgver is valid.
+    :param check_pkgver: verify that the pkgver is valid.
+    :param check_pkgname: the pkgname must match the name of the aport folder
     :returns: relevant variables from the APKBUILD. Arrays get returned as
               arrays.
     """
@@ -152,11 +153,12 @@ def apkbuild(args, path, check_pkgver=True):
 
     # Sanity check: pkgname
     suffix = "/" + ret["pkgname"] + "/APKBUILD"
-    if not os.path.realpath(path).endswith(suffix):
+    if check_pkgname:
-        logging.info("Folder: '" + os.path.dirname(path) + "'")
+        if not os.path.realpath(path).endswith(suffix):
-        logging.info("Pkgname: '" + ret["pkgname"] + "'")
+            logging.info("Folder: '" + os.path.dirname(path) + "'")
-        raise RuntimeError("The pkgname must be equal to the name of"
+            logging.info("Pkgname: '" + ret["pkgname"] + "'")
-                           " the folder, that contains the APKBUILD!")
+            raise RuntimeError("The pkgname must be equal to the name of"
+                               " the folder, that contains the APKBUILD!")
 
     # Sanity check: arch
     if not len(ret["arch"]):

pmb/parse/arguments.py

@@ -265,6 +265,8 @@ def arguments():
     chroot = sub.add_parser("chroot", help="start shell in chroot")
     chroot.add_argument("--add", help="build/install comma separated list of"
                         " packages in the chroot before entering it")
+    chroot.add_argument("--user", help="run the command as user, not as root",
+                        action="store_true")
     chroot.add_argument("command", default=["sh"], help="command"
                         " to execute inside the chroot. default: sh", nargs='*')
     for action in [build_init, chroot]:

test/test_aportgen.py

@@ -37,26 +37,25 @@ def args(tmpdir, request):
     args.log = args.work + "/log_testsuite.txt"
     pmb.helpers.logging.init(args)
     request.addfinalizer(args.logfd.close)
-    setattr(args, "_aports_real", args.aports)
-    args.aports = str(tmpdir)
-    pmb.helpers.run.user(args, ["mkdir", "-p", str(tmpdir) + "/cross"])
     return args
 
 
-def test_aportgen(args):
+def test_aportgen(args, tmpdir):
+    # Fake aports folder in tmpdir
+    aports_real = args.aports
+    args.aports = str(tmpdir)
+    pmb.helpers.run.user(args, ["mkdir", "-p", str(tmpdir) + "/cross"])
+
     # Create aportgen folder -> code path where it still exists
     pmb.helpers.run.user(args, ["mkdir", "-p", args.work + "/aportgen"])
 
-    # Generate all valid packages
+    # Generate all valid packages (gcc twice -> different code path)
-    pkgnames = []
+    pkgnames = ["binutils-armhf", "musl-armhf", "busybox-static-armhf",
-    for arch in pmb.config.build_device_architectures:
+                "gcc-armhf", "gcc-armhf"]
-        # gcc twice, so the output folder already exists -> different code path
-        for pkgname in ["binutils", "musl", "busybox-static", "gcc", "gcc"]:
-            pkgnames.append(pkgname + "-" + arch)
     for pkgname in pkgnames:
         pmb.aportgen.generate(args, pkgname)
         path_new = args.aports + "/cross/" + pkgname + "/APKBUILD"
-        path_old = args._aports_real + "/cross/" + pkgname + "/APKBUILD"
+        path_old = aports_real + "/cross/" + pkgname + "/APKBUILD"
         assert os.path.exists(path_new)
         assert filecmp.cmp(path_new, path_old, False)
 
@@ -65,3 +64,38 @@ def test_aportgen_invalid_generator(args):
     with pytest.raises(ValueError) as e:
         pmb.aportgen.generate(args, "pkgname-with-no-generator")
     assert "No generator available" in str(e.value)
+
+
+def test_aportgen_get_upstream_aport(args, monkeypatch):
+
+    # Fake pmb.parse.apkbuild()
+    def fake_apkbuild(*args, **kwargs):
+        return apkbuild
+    monkeypatch.setattr(pmb.parse, "apkbuild", fake_apkbuild)
+
+    # Fake pmb.parse.apkindex.package()
+    def fake_package(*args, **kwargs):
+        return package
+    monkeypatch.setattr(pmb.parse.apkindex, "package", fake_package)
+
+    # Equal version
+    func = pmb.aportgen.core.get_upstream_aport
+    upstream = "main/gcc"
+    upstream_full = args.work + "/cache_git/aports_upstream/" + upstream
+    apkbuild = {"pkgver": "2.0", "pkgrel": "0"}
+    package = {"version": "2.0-r0"}
+    assert func(args, upstream) == upstream_full
+
+    # APKBUILD < binary
+    apkbuild = {"pkgver": "1.0", "pkgrel": "0"}
+    package = {"version": "2.0-r0"}
+    with pytest.raises(RuntimeError) as e:
+        func(args, upstream)
+    assert str(e.value).startswith("You can update your local checkout with")
+
+    # APKBUILD > binary
+    apkbuild = {"pkgver": "3.0", "pkgrel": "0"}
+    package = {"version": "2.0-r0"}
+    with pytest.raises(RuntimeError) as e:
+        func(args, upstream)
+    assert str(e.value).startswith("You can force an update of your binary")

test/test_chroot_interactive_shell.py

@@ -32,6 +32,19 @@ def test_chroot_interactive_shell():
     assert ret == "hello_world\n"
 
 
+def test_chroot_interactive_shell_user():
+    """
+    Open a shell with 'pmbootstrap chroot' as user, and test the resulting ID.
+    """
+    pmb_src = os.path.realpath(os.path.join(os.path.dirname(__file__) + "/.."))
+    os.chdir(pmb_src)
+    ret = subprocess.check_output(["./pmbootstrap.py", "-q", "chroot",
+                                  "--user"], timeout=300, input="id -un",
+                                  universal_newlines=True,
+                                  stderr=subprocess.STDOUT)
+    assert ret == "pmos\n"
+
+
 def test_chroot_arguments():
     """
     Open a shell with 'pmbootstrap chroot' for every architecture, pass 'uname -m\n'

test/test_repo.py

@@ -17,6 +17,7 @@ You should have received a copy of the GNU General Public License
 along with pmbootstrap.  If not, see <http://www.gnu.org/licenses/>.
 """
 import os
+import pytest
 import sys
 
 # Import from parent directory
@@ -25,7 +26,25 @@ sys.path.append(pmb_src)
 import pmb.helpers.repo
 
 
+@pytest.fixture
+def args(tmpdir, request):
+    import pmb.parse
+    sys.argv = ["pmbootstrap.py", "chroot"]
+    args = pmb.parse.arguments()
+    args.log = args.work + "/log_testsuite.txt"
+    pmb.helpers.logging.init(args)
+    request.addfinalizer(args.logfd.close)
+    return args
+
+
 def test_hash():
     url = "https://nl.alpinelinux.org/alpine/edge/testing"
     hash = "865a153c"
     assert pmb.helpers.repo.hash(url, 8) == hash
+
+
+def test_alpine_apkindex_path(args):
+    func = pmb.helpers.repo.alpine_apkindex_path
+    args.mirror_alpine = "http://dl-cdn.alpinelinux.org/alpine/"
+    ret = args.work + "/cache_apk_armhf/APKINDEX.30e6f5af.tar.gz"
+    assert func(args, "testing", "armhf") == ret

test/test_upstream_compatibility.py

@@ -46,13 +46,9 @@ def test_qt_versions(args):
     qt5-qtbase version.
     """
     # Upstream version
-    pmb.helpers.repo.update(args, "armhf")
+    index = pmb.helpers.repo.alpine_apkindex_path(args, "community", "armhf")
-    repository = args.mirror_alpine + args.alpine_version + "/community"
-    hash = pmb.helpers.repo.hash(repository)
-    index_path = (args.work + "/cache_apk_armhf/APKINDEX." + hash +
-                  ".tar.gz")
     index_data = pmb.parse.apkindex.package(args, "qt5-qtbase",
-                                            indexes=[index_path])
+                                            indexes=[index])
     pkgver_upstream = index_data["version"].split("-r")[0]
 
     # Iterate over our packages
@@ -83,13 +79,8 @@ def test_aportgen_versions(args):
     the same version (pkgver *and* pkgrel!) as the upstream packages
     they are based on.
     """
-
     # Get Alpine's "main" repository APKINDEX path
-    pmb.helpers.repo.update(args, "armhf")
+    index = pmb.helpers.repo.alpine_apkindex_path(args, "main", "armhf")
-    repository = args.mirror_alpine + args.alpine_version + "/main"
-    hash = pmb.helpers.repo.hash(repository)
-    index_path = (args.work + "/cache_apk_armhf/APKINDEX." + hash +
-                  ".tar.gz")
 
     # Alpine packages and patterns for our derivatives
     map = {"binutils": "binutils-*",
@@ -103,7 +94,7 @@ def test_aportgen_versions(args):
     for pkgname, pattern in map.items():
         # Upstream version
         index_data = pmb.parse.apkindex.package(args, pkgname,
-                                                indexes=[index_path])
+                                                indexes=[index])
         version_upstream = index_data["version"]
 
         # Iterate over our packages

test/testcases_fast.sh

@@ -1,12 +1,16 @@
 #!/bin/sh -e
+# usage: testcases_fast.sh [--all]
 
 # Disable slow testcases
-# aport_in_sync_with_git: clones Alpine's aports repo
+disabled="qemu_running_processes"
-# aportgen: clones Alpine's aports repo
+
-disabled="
+# Optionally enable all test cases
-	aport_in_sync_with_git
+if [ "$1" = "--all" ]; then
-	aportgen
+    disabled=""
-"
+else
+    echo "Disabled test case(s): $disabled"
+    echo "Use '$(basename "$0") --all' to enable all test cases."
+fi
 
 # Make sure we have a valid device (#1128)
 cd "$(dirname "$0")/.."