Clayton Craft: 2 parse.apkbuild.parse_subpackage: don't inherit pmb_recommends pmb.install: support pmb_recommends for any package 7 files changed, 81 insertions(+), 56 deletions(-)
pmbootstrap/patches/.build.yml: SUCCESS in 14m15s [parse.apkbuild.parse_subpackage: don't inherit pmb_recommends][0] from [Clayton Craft][1] [0]: https://lists.sr.ht/~postmarketos/pmbootstrap-devel/patches/48226 [1]: mailto:clayton@craftyguy.net ✓ #1125247 SUCCESS pmbootstrap/patches/.build.yml https://builds.sr.ht/~postmarketos/job/1125247
Copy & paste the following snippet into your terminal to import this patchset into git:
curl -s https://lists.sr.ht/~postmarketos/pmbootstrap-devel/patches/48226/mbox | git am -3Learn more about email & git
--- pmb/parse/_apkbuild.py | 6 ++++++ 1 file changed, 6 insertions(+)
Nice! Reviewed-by: Oliver Smith <ollieparanoid@postmarketos.org>
diff --git a/pmb/parse/_apkbuild.py b/pmb/parse/_apkbuild.py index 6e9d004f..a76dedc0 100644 --- a/pmb/parse/_apkbuild.py +++ b/pmb/parse/_apkbuild.py @@ -271,6 +271,12 @@ def _parse_subpackage(path, lines, apkbuild, subpackages, subpkg): # Copy variables apkbuild = apkbuild.copy() apkbuild["subpkgname"] = subpkgname + # Don't inherit pmb_recommends from the top-level package. + # There are two reasons for this: + # 1) the subpackage may specify its own pmb_recommends + # 2) the top-level package may list the subpackage as a pmb_recommends, + # thereby creating a circular dependency + apkbuild["_pmb_recommends"] = "" # Parse relevant attributes for the subpackage _parse_attributes( -- 2.43.0
Applied, thanks!
[1/2] parse.apkbuild.parse_subpackage: don't inherit pmb_recommends
commit: 0c0f05caabb919e0a90fb6d74176a2a291bd2893
[2/2] pmb.install: support pmb_recommends for any package
commit: 19f8a3b8c8d54abebf6762a25de549e5215a3e98
Best regards
This refactors the get_recommends function that was originally used for UI packages to support pmb_recommends for any package (and subpackage). Extending pmb_recommends will, for example, help us create better generic device packages [1] and can be used to improve packaging for UIs with shared pmb_recommends[2]. 1. https://gitlab.com/postmarketOS/pmaports/-/merge_requests/4673 2. https://gitlab.com/postmarketOS/pmaports/-/merge_requests/3700
Very nice patch, what a great improvement! Thank you so much :D Minor style/comment thing, but I'll fix it as I merge it. Reviewed-by: Oliver Smith <ollieparanoid@postmarketos.org>
--- pmb/config/__init__.py | 6 +-- pmb/install/_install.py | 49 ++++++++++++++++++- pmb/install/ui.py | 30 ------------ test/test_install.py | 36 ++++++-------- .../main/postmarketos-ui-test/APKBUILD | 2 +- .../pmb_recommends/main/test-app/APKBUILD | 8 +++ 6 files changed, 75 insertions(+), 56 deletions(-) create mode 100644 test/testdata/pmb_recommends/main/test-app/APKBUILD diff --git a/pmb/config/__init__.py b/pmb/config/__init__.py index 05fba846..e51d511d 100644 --- a/pmb/config/__init__.py +++ b/pmb/config/__init__.py @@ -696,9 +696,9 @@ apkbuild_package_attributes = { "install": {"array": True}, "triggers": {"array": True}, - # UI meta-packages can specify apps in "_pmb_recommends" to be explicitly - # installed by default, and not implicitly as dependency of the UI meta- - # package ("depends"). This makes these apps uninstallable, without + # Packages can specify soft dependencies in "_pmb_recommends" to be + # explicitly installed by default, and not implicitly as a hard dependency + # of the package ("depends"). This makes these apps uninstallable, without # removing the meta-package. (#1933). To disable this feature, use: # "pmbootstrap install --no-recommends". "_pmb_recommends": {"array": True}, diff --git a/pmb/install/_install.py b/pmb/install/_install.py index 1adfe789..e9d65471 100644 --- a/pmb/install/_install.py +++ b/pmb/install/_install.py @@ -1054,6 +1054,50 @@ def get_selected_providers(args, packages): return providers +def get_recommends(args, packages): + """ + Look through the specified packages and collect additional packages
Style nitpick: other files either use """ comment start or """ comment start
+ specified under _pmb_recommends in them. This is recursive, so it will + dive into packages that are listed under recommends to collect any + packages they might also have listed under their own _pmb_recommends. + + If unable to find a given package in aports, it is skipped and no error + is raised. This is because the given package might be in a different + aports than the one searched by this function. This function makes no + attempt to validate that a given package is in *any* available aports + repos at installation time.
I didn't understand what you mean with this comment at first, and thought it would not return aports that it cannot find. But what the function does is, it returns all _pmb_recommends it finds and just doesn't recurse into the packages mentioned there that are not in pmaports. How you implemnted it makes sense! I'll add a commit that tweaks the comment when merging this patch.
+ + If running with pmbootstrap install --no-recommends, this function + returns an empty list. + + :returns: list of pkgnames, e.g. ["chatty", "gnome-contacts"] """ + ret = [] + if not args.install_recommends: + return ret + + for package in packages: + # Note that this ignores packages that don't exist. This means they + # aren't in pmaports. This is fine, with the assumption that + # installation will fail later in some other method if they truly don't + # exist in any repo. + apkbuild = pmb.helpers.pmaports.get(args, package, must_exist=False) + if not apkbuild: + continue + if package in apkbuild["subpackages"]: + # Just focus on the subpackage + apkbuild = apkbuild["subpackages"][package] + recommends = apkbuild["_pmb_recommends"] + if recommends: + logging.debug(f"{package}: install _pmb_recommends:" + f" {', '.join(recommends)}") + ret += recommends + # Call recursively in case recommends have pmb_recommends of their + # own. + ret += get_recommends(args, recommends) + + return ret + + def create_device_rootfs(args, step, steps): # List all packages to be installed (including the ones specified by --add) # and upgrade the installed packages/apkindexes @@ -1082,8 +1126,6 @@ def create_device_rootfs(args, step, steps): if args.ui.lower() != "none": if args.ui_extras: install_packages += ["postmarketos-ui-" + args.ui + "-extras"] - if args.install_recommends: - install_packages += pmb.install.ui.get_recommends(args) if args.extra_packages.lower() != "none": install_packages += args.extra_packages.split(",") if args.add: @@ -1112,6 +1154,9 @@ def create_device_rootfs(args, step, steps): pmb.helpers.repo.update(args, args.deviceinfo["arch"]) + # Install uninstallable "dependencies" by default + install_packages += get_recommends(args, install_packages) + # Explicitly call build on the install packages, to re-build them or any # dependency, in case the version increased if args.build_pkgs_on_install: diff --git a/pmb/install/ui.py b/pmb/install/ui.py index d311b611..95d0ec47 100644 --- a/pmb/install/ui.py +++ b/pmb/install/ui.py @@ -5,36 +5,6 @@ import logging import pmb.helpers.pmaports -def get_recommends(args): - """ Get all packages listed in _pmb_recommends of the UI and UI-extras - package, unless running with pmbootstrap install --no-recommends. - - :returns: list of pkgnames, e.g. ["chatty", "gnome-contacts"] """ - ret = [] - if not args.install_recommends or args.ui == "none": - return ret - - # UI package - meta = f"postmarketos-ui-{args.ui}" - apkbuild = pmb.helpers.pmaports.get(args, meta) - recommends = apkbuild["_pmb_recommends"] - if recommends: - logging.debug(f"{meta}: install _pmb_recommends:" - f" {', '.join(recommends)}") - ret += recommends - - # UI-extras subpackage - meta_extras = f"{meta}-extras" - if args.ui_extras and meta_extras in apkbuild["subpackages"]: - recommends = apkbuild["subpackages"][meta_extras]["_pmb_recommends"] - if recommends: - logging.debug(f"{meta_extras}: install _pmb_recommends:" - f" {', '.join(recommends)}") - ret += recommends - - return ret - - def get_groups(args): """ Get all groups to which the user additionally must be added. The list of groups are listed in _pmb_groups of the UI and diff --git a/test/test_install.py b/test/test_install.py index 7ead2670..7b379107 100644 --- a/test/test_install.py +++ b/test/test_install.py @@ -50,37 +50,33 @@ def test_get_nonfree_packages(args): def test_get_recommends(args): args.aports = pmb_test.const.testdata + "/pmb_recommends" - func = pmb.install.ui.get_recommends + func = pmb.install._install.get_recommends # UI: none args.install_recommends = True - args.ui = "none" - assert func(args) == [] + assert func(args, ["postmarketos-ui-none"]) == [] # UI: test, --no-recommends args.install_recommends = False - args.ui = "test" - assert func(args) == [] + assert func(args, ["postmarketos-ui-test"]) == [] - # UI: test, without -extras + # UI: test args.install_recommends = True - args.ui = "test" - args.ui_extras = False - assert func(args) == ["plasma-camera", "plasma-angelfish"] + assert func(args, ["postmarketos-ui-test"]) == ["plasma-camera", + "plasma-angelfish"] - # UI: test, with -extras + # UI: test + test-extras args.install_recommends = True - args.ui = "test" - args.ui_extras = True - assert func(args) == ["plasma-camera", "plasma-angelfish", "buho", - "kaidan"] - - # UI: invalid + assert func(args, ["postmarketos-ui-test", + "postmarketos-ui-test-extras"]) == ["plasma-camera", + "plasma-angelfish", + "buho", "kaidan", + "test-app", "foot", + "htop"] + # Non-UI package args.install_recommends = True - args.ui = "invalid" - with pytest.raises(RuntimeError) as e: - func(args) - assert str(e.value).startswith("Could not find aport for package") + args.ui_extras = False + assert func(args, ["test-app"]) == ["foot", "htop"] def test_get_groups(args): diff --git a/test/testdata/pmb_recommends/main/postmarketos-ui-test/APKBUILD b/test/testdata/pmb_recommends/main/postmarketos-ui-test/APKBUILD index 5ca94068..94f6a652 100644 --- a/test/testdata/pmb_recommends/main/postmarketos-ui-test/APKBUILD +++ b/test/testdata/pmb_recommends/main/postmarketos-ui-test/APKBUILD @@ -9,5 +9,5 @@ arch="all" _pmb_recommends="plasma-camera plasma-angelfish" extras() { - _pmb_recommends="buho kaidan" + _pmb_recommends="buho kaidan test-app" } diff --git a/test/testdata/pmb_recommends/main/test-app/APKBUILD b/test/testdata/pmb_recommends/main/test-app/APKBUILD new file mode 100644 index 00000000..1dbf3a99 --- /dev/null +++ b/test/testdata/pmb_recommends/main/test-app/APKBUILD @@ -0,0 +1,8 @@ +pkgname=test-app +pkgver=1 +pkgrel=0 +license="GPL-3.0-or-later" +depends="" +arch="all" + +_pmb_recommends="foot htop" -- 2.43.0
builds.sr.ht <builds@sr.ht>pmbootstrap/patches/.build.yml: SUCCESS in 14m15s [parse.apkbuild.parse_subpackage: don't inherit pmb_recommends][0] from [Clayton Craft][1] [0]: https://lists.sr.ht/~postmarketos/pmbootstrap-devel/patches/48226 [1]: mailto:clayton@craftyguy.net ✓ #1125247 SUCCESS pmbootstrap/patches/.build.yml https://builds.sr.ht/~postmarketos/job/1125247