luci-app-package-manager: count only one provider for alternative deps#8804
luci-app-package-manager: count only one provider for alternative deps#8804maxskokov wants to merge 1 commit into
Conversation
openwrt-ai
left a comment
There was a problem hiding this comment.
Reviewed 1 new commit. One inline question about the download-size accounting; the change is otherwise sound.
Generated by Claude Code
|
|
||
| li.appendChild(E('span', { 'data-tooltip': pkg.description }, | ||
| [ text, ' ', pkgStatus(pkg, vop, ver, info) ])); | ||
| [ text, ' ', pkgStatus(pkg, vop, ver, info, !satisfied && i === effective) ])); |
There was a problem hiding this comment.
count only gates the Not installed push below (line 562); the Needs upgrade branch at info.install.push(repl)`` is not gated by it. So if a versioned virtual dependency has two already-installed providers that both need upgrading, this loop calls pkgStatus for each (both with `count === false`) and both `repl` packages are still pushed to `info.install`, re-inflating the size estimate this PR aims to fix. This is an unusual state (apk normally keeps only one provider of a virtual installed), so it may be out of scope — but is the omission intentional, or should the `repl` push also be limited to the effective provider?
Generated by Claude Code
There was a problem hiding this comment.
Intentional. The repl push is an upgrade of an already installed provider, so it is a real download that should still count even when the virtual is otherwise satisfied. Gating it on count would drop that upgrade size for the effective provider, since count is false whenever any provider is installed. The double count needs two installed providers of the same virtual at once, which apk does not keep. The inflation from #8411 comes from the not-installed push, which is the one gated here.
I also updated the fallback to follow the apk solver's own selection order: installed provider, otherwise the highest provider-priority, otherwise the first available one. The field is already in the model since the backend queries with --fields all, and the build system emits it from PRIORITY in include/package-pack.mk.
When a package depends on a virtual name with several providers, the dependency details listed every provider and added each one to the download size estimate and the subdependency tree. apk installs only a single provider, so the estimate was inflated and, once one provider was already installed, the dependency was still shown as pulling in the others. Pick one effective provider per dependency, following the apk solver selection order: an installed one if present, otherwise the available one with the highest provider-priority, otherwise the first available one. Count only that provider towards the size estimate and subdependencies. provider-priority is already part of the model since the backend queries with --fields all and the build system emits it from PRIORITY (include/package-pack.mk). The install command itself is unchanged; it passes the target package to apk, which resolves providers on its own. Fixes: openwrt#8411 Signed-off-by: Maxim Skokov <skokov.m020709@gmail.com>
27f8cf6 to
0b3a805
Compare
openwrt-ai
left a comment
There was a problem hiding this comment.
Reviewed 1 new commit; no new issues found.
The provider-priority fallback resolves the prior review's question and mirrors the apk solver order (installed → highest provider-priority → first available). The provider-priority field name matches the dashed-key convention used elsewhere in the parser (file-size, auto-installed) and the --info "provider-priority:..." emit in include/package-pack.mk.
Generated by Claude Code
Pull request details
Description
Fixes #8411.
When a package depends on a virtual name with several providers (for example a dependency on
wget-any, provided bywget-ssl,wget-nosslanduclient-fetch), the dependency details listed every provider and added each one to the download size estimate and the subdependency tree. apk installs only a single provider, so the size was overstated, and once one provider was already installed the dependency still showed the others as pending.This picks one effective provider per dependency, an installed one if present, otherwise the first, and counts only that towards the size estimate and subdependencies. The install command is unchanged: it passes the target package to apk, which resolves providers itself, so
info.installonly ever drove the displayed estimate.When no provider is installed yet the first one is used as the estimate. The exact provider apk would pick is not reproduced, since that would mean duplicating its preference rules, but the estimate now reflects a single provider instead of the sum of all of them.
Maintainer
@jow-
Tested on
Not tested on a device. The dependency selection and size accounting (
renderDependencies,renderDependencyItem,pkgStatus) were run headless against fixtures shaped like the internal package model, using the reporter'sapk-mbedtls->wget-anycase:eslint passes on the changed file.
Checklist
Signed-off-byrow.<package name>: titlefirst line subject.