From e91dbefd16e945b66968c8bdcf4ba897ab22a50f Mon Sep 17 00:00:00 2001 From: Oliver Smith Date: Sat, 28 May 2022 12:44:56 +0200 Subject: [PATCH] pmb.chroot.apk.install_{is_necessary -> build} (MR 2185) install_is_necessary used to do the following things: 1. Error out if there's no binary package but pmb was invoked as "pmbootstrap install" and build_pkgs_on_install is disabled. 2. Build the package if necessary. 3. Return if a package "needs to be installed" (Boolean or Float). The only caller of the function is pmb.chroot.apk.install. It would not add the package to the long "apk add" command if according to 3. it does not need to be installed. When I implemented this a few years ago, I probably thought it would be useful to not unnecessarily pass packages to apk. But this actually makes it more complicated and doesn't have a benefit, apk is perfectly capable of recognizing which packages it had already installed. Replace the function with a much simpler pmb.chroot.apk.install_build, which only does 1. and 2. Change the order of the package, arch arguments to match called functions pmb.parse.apkindex.package and pmb.build.package. --- pmb/chroot/apk.py | 83 +++++++++++------------------------------------ test/test_apk.py | 47 ++++++++++++++++++++------- 2 files changed, 55 insertions(+), 75 deletions(-) diff --git a/pmb/chroot/apk.py b/pmb/chroot/apk.py index 6f30e8e9..a0ceb5bb 100644 --- a/pmb/chroot/apk.py +++ b/pmb/chroot/apk.py @@ -87,74 +87,30 @@ def check_min_version(args, suffix="native"): pmb.helpers.other.cache["apk_min_version_checked"].append(suffix) -def install_is_necessary(args, build, arch, package, packages_installed): +def install_build(args, package, arch): """ - This function optionally builds an out of date package, and checks if the - version installed inside a chroot is up to date. - :param build: Set to true to build the package, if the binary packages are - out of date, and it is in the aports folder. - :param packages_installed: Return value from installed(). - :returns: True if the package needs to be installed/updated, - False otherwise. - """ - # For packages to be removed we can do the test immediately - if package.startswith("!"): - return package[1:] in packages_installed + Build an outdated package unless pmbootstrap was invoked with + "pmbootstrap install" and the option to build packages during pmb install + is disabled. - # User may have disabled buiding packages during "pmbootstrap install" - build_disabled = False + :param package: name of the package to build + :param arch: architecture of the package to build + """ + # User may have disabled building packages during "pmbootstrap install" if args.action == "install" and not args.build_pkgs_on_install: - build_disabled = True - - # Build package - if build and not build_disabled: - pmb.build.package(args, package, arch) - - # No further checks when not installed - if package not in packages_installed: - return True - - # Make sure that we really have a binary package - data_repo = pmb.parse.apkindex.package(args, package, arch, False) - if not data_repo: - if build_disabled: + if not pmb.parse.apkindex.package(args, package, arch, False): raise RuntimeError(f"{package}: no binary package found for" f" {arch}, and compiling packages during" " 'pmbootstrap install' has been disabled." " Consider changing this option in" " 'pmbootstrap init'.") - logging.warning("WARNING: Internal error in pmbootstrap," - f" package '{package}' for {arch}" - " has not been built yet, but it should have" - " been. Rebuilding it with force. Please " - " report this, if there is no ticket about this" - " yet!") - pmb.build.package(args, package, arch, True) - return install_is_necessary(args, build, arch, package, - packages_installed) + # Use the existing binary package + return - # Compare the installed version vs. the version in the repos - data_installed = packages_installed[package] - compare = pmb.parse.version.compare(data_installed["version"], - data_repo["version"]) - # a) Installed newer (should not happen normally) - if compare == 1: - logging.info(f"WARNING: {arch} package '{package}'" - f" installed version {data_installed['version']}" - " is newer, than the version in the repositories:" - f" {data_repo['version']}" - " See also: ") - return False - - # b) Repo newer - elif compare == -1: - return True - - # c) Same version, look at last modified - elif compare == 0: - time_installed = float(data_installed["timestamp"]) - time_repo = float(data_repo["timestamp"]) - return time_repo > time_installed + # Build the package if it's in pmaports and there is no binary package + # with the same pkgver and pkgrel. This check is done in + # pmb.build.is_necessary, which gets called in pmb.build.package. + return pmb.build.package(args, package, arch) def replace_aports_packages_with_path(args, packages, suffix, arch): @@ -211,13 +167,12 @@ def install(args, packages, suffix="native", build=True): to_add = [] to_del = [] for package in packages_with_depends: - if not install_is_necessary( - args, build, arch, package, packages_installed): - continue if package.startswith("!"): to_del.append(package.lstrip("!")) - else: - to_add.append(package) + continue + if build: + install_build(args, package, arch) + to_add.append(package) if not len(to_add) and not len(to_del): return diff --git a/test/test_apk.py b/test/test_apk.py index 2a63c3d1..737cf020 100644 --- a/test/test_apk.py +++ b/test/test_apk.py @@ -18,15 +18,40 @@ def args(tmpdir, request): return args -def test_install_is_necessary(args): - # osk-sdl not installed, nothing to do - ret = pmb.chroot.apk.install_is_necessary(args, False, "aarch64", - "!osk-sdl", - {"unl0kr": {"unl0kr": {}}}) - assert not ret +def test_install_build(monkeypatch, args): + func = pmb.chroot.apk.install_build + ret_apkindex_package = None - # osk-sdl installed, (un)install necessary - ret = pmb.chroot.apk.install_is_necessary(args, False, "aarch64", - "!osk-sdl", - {"osk-sdl": {"osk-sdl": {}}}) - assert ret + def fake_build_package(args, package, arch): + return "build-pkg" + monkeypatch.setattr(pmb.build, "package", fake_build_package) + + def fake_apkindex_package(args, package, arch, must_exist): + return ret_apkindex_package + monkeypatch.setattr(pmb.parse.apkindex, "package", fake_apkindex_package) + + package = "hello-world" + arch = "x86_64" + + # invoked as pmb install, build_pkgs_on_install disabled + args.action = "install" + args.build_pkgs_on_install = False + with pytest.raises(RuntimeError) as e: + func(args, package, arch) + assert "no binary package found" in str(e.value) + + # invoked as pmb install, build_pkgs_on_install disabled, binary exists + args.action = "install" + args.build_pkgs_on_install = False + ret_apkindex_package = {"pkgname": "hello-world"} + assert func(args, package, arch) is None + + # invoked as pmb install, build_pkgs_on_install enabled + args.action = "install" + args.build_pkgs_on_install = True + assert func(args, package, arch) == "build-pkg" + + # invoked as not pmb install + args.action = "chroot" + args.build_pkgs_on_install = False + assert func(args, package, arch) == "build-pkg"