From b8f35d45b87f0a08b6c77fa90f9c5a52d110bd2f Mon Sep 17 00:00:00 2001 From: Oliver Smith Date: Sun, 11 Mar 2018 14:18:21 +0000 Subject: [PATCH] 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 | 2 +- pmb/aportgen/binutils.py | 8 ++-- pmb/aportgen/core.py | 46 ++++++++++++++++++++++ pmb/aportgen/gcc.py | 14 ++----- pmb/chroot/apk_static.py | 11 ++---- pmb/helpers/frontend.py | 18 ++++++++- pmb/helpers/repo.py | 23 +++++++++++ pmb/parse/_apkbuild.py | 16 ++++---- pmb/parse/arguments.py | 2 + test/test_aportgen.py | 56 +++++++++++++++++++++------ test/test_chroot_interactive_shell.py | 13 +++++++ test/test_repo.py | 19 +++++++++ test/test_upstream_compatibility.py | 17 ++------ test/testcases_fast.sh | 16 +++++--- 14 files changed, 199 insertions(+), 62 deletions(-) diff --git a/.travis.yml b/.travis.yml index 6bd4679c..66b2a87a 100644 --- a/.travis.yml +++ b/.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: diff --git a/pmb/aportgen/binutils.py b/pmb/aportgen/binutils.py index c35589b2..875ec3f4 100644 --- a/pmb/aportgen/binutils.py +++ b/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 = (args.work + "/cache_git/aports_upstream/" + path_original) - pmb.helpers.git.clone(args, "aports_upstream") + upstream = pmb.aportgen.core.get_upstream_aport(args, "main/binutils") 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", - replace_functions) + pmb.aportgen.core.rewrite(args, pkgname, "main/binutils", fields, + "binutils", replace_functions) diff --git a/pmb/aportgen/core.py b/pmb/aportgen/core.py index ee38e089..912703eb 100644 --- a/pmb/aportgen/core.py +++ b/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 . """ 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'") diff --git a/pmb/aportgen/gcc.py b/pmb/aportgen/gcc.py index eed5f6f8..f3e2f7f5 100644 --- a/pmb/aportgen/gcc.py +++ b/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 = (args.work + "/cache_git/aports_upstream/" + path_original) - pmb.helpers.git.clone(args, "aports_upstream") + upstream = pmb.aportgen.core.get_upstream_aport(args, "main/gcc") 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( - args, - pkgname, - path_original, - fields, - replace_simple=replace_simple, - below_header=below_header) + pmb.aportgen.core.rewrite(args, pkgname, "main/gcc", fields, + replace_simple=replace_simple, + below_header=below_header) diff --git a/pmb/chroot/apk_static.py b/pmb/chroot/apk_static.py index 9041f6eb..b54485af 100644 --- a/pmb/chroot/apk_static.py +++ b/pmb/chroot/apk_static.py @@ -159,16 +159,13 @@ def init(args): """ Download, verify, extract $WORK/apk.static. """ - # Get the APKINDEX - pmb.helpers.repo.update(args, args.arch_native) - 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 + # Get and parse the APKINDEX + apkindex = pmb.helpers.repo.alpine_apkindex_path(args, "main") 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: diff --git a/pmb/helpers/frontend.py b/pmb/helpers/frontend.py index b5c28c1f..fd5b4615 100644 --- a/pmb/helpers/frontend.py +++ b/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): diff --git a/pmb/helpers/repo.py b/pmb/helpers/repo.py index 95201220..18785799 100644 --- a/pmb/helpers/repo.py +++ b/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" diff --git a/pmb/parse/_apkbuild.py b/pmb/parse/_apkbuild.py index c6fdf046..911834c5 100644 --- a/pmb/parse/_apkbuild.py +++ b/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): - logging.info("Folder: '" + os.path.dirname(path) + "'") - logging.info("Pkgname: '" + ret["pkgname"] + "'") - raise RuntimeError("The pkgname must be equal to the name of" - " the folder, that contains the APKBUILD!") + if check_pkgname: + if not os.path.realpath(path).endswith(suffix): + logging.info("Folder: '" + os.path.dirname(path) + "'") + logging.info("Pkgname: '" + ret["pkgname"] + "'") + 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"]): diff --git a/pmb/parse/arguments.py b/pmb/parse/arguments.py index ca379f3c..804e2358 100644 --- a/pmb/parse/arguments.py +++ b/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]: diff --git a/test/test_aportgen.py b/test/test_aportgen.py index 27bff901..57e27fc6 100644 --- a/test/test_aportgen.py +++ b/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 - pkgnames = [] - for arch in pmb.config.build_device_architectures: - # gcc twice, so the output folder already exists -> different code path - for pkgname in ["binutils", "musl", "busybox-static", "gcc", "gcc"]: - pkgnames.append(pkgname + "-" + arch) + # Generate all valid packages (gcc twice -> different code path) + pkgnames = ["binutils-armhf", "musl-armhf", "busybox-static-armhf", + "gcc-armhf", "gcc-armhf"] 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") diff --git a/test/test_chroot_interactive_shell.py b/test/test_chroot_interactive_shell.py index be4f23fa..1e88246d 100644 --- a/test/test_chroot_interactive_shell.py +++ b/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' diff --git a/test/test_repo.py b/test/test_repo.py index 809f477b..9a9da784 100644 --- a/test/test_repo.py +++ b/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 . """ 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 diff --git a/test/test_upstream_compatibility.py b/test/test_upstream_compatibility.py index bf27582f..3e16310e 100644 --- a/test/test_upstream_compatibility.py +++ b/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") - 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 = pmb.helpers.repo.alpine_apkindex_path(args, "community", "armhf") 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") - 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") + index = pmb.helpers.repo.alpine_apkindex_path(args, "main", "armhf") # 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 diff --git a/test/testcases_fast.sh b/test/testcases_fast.sh index 8e6f2064..c80f7aef 100755 --- a/test/testcases_fast.sh +++ b/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 -# aportgen: clones Alpine's aports repo -disabled=" - aport_in_sync_with_git - aportgen -" +disabled="qemu_running_processes" + +# Optionally enable all test cases +if [ "$1" = "--all" ]; then + 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")/.."