[pacman-dev] [PATCH] Add [ignored] to -Qu output for packages in repos that are not Usage = Upgrade
The behaviour of "pacman -Qu" was very strange... It would only consider packages from repos with Usage = Search (or All), and ignore those with Usage = Sync, Install or Upgrade.
This is because alpm_sync_newversion() used ALPM_DB_USAGE_SEARCH for its filtering. Given this function is documented (at least in the source) to "Check for new version of pkg in sync repos", I would expect that to look at all repos. However, just changing this parameter, would result in a fairly silent change in behaviour of this function. To counter that, add a parameter to the function that tells it which databases usage levels to consider.
Finally, list all available updates in -Qu output, but include [ignored] beside those that will not be updated in a -Su operation due to thier repo Usage value (in addition to those that are Ignored).
Fixes FS#59854.
With thanks to the following who provided initial patches to print [ignored] on -Qu operations, which highlighted the larger problem: morganamilo morganamilo@gmail.com Michael Straube michael.straube@posteo.de
Signed-off-by: Allan McRae allan@archlinux.org ---
In comments on earlier patches, there was debate about what the alpm_sync_newversion() is doing. Although I expect users of this function to likely always pass ALPM_DB_USAGE_ALL, I think changing the function signature is the safest way to handle this change.
Comments?
lib/libalpm/alpm.h | 2 +- lib/libalpm/sync.c | 4 ++-- src/pacman/query.c | 11 ++++++++--- 3 files changed, 11 insertions(+), 6 deletions(-)
diff --git a/lib/libalpm/alpm.h b/lib/libalpm/alpm.h index 597e11bd..67f9e8f5 100644 --- a/lib/libalpm/alpm.h +++ b/lib/libalpm/alpm.h @@ -1452,7 +1452,7 @@ alpm_list_t *alpm_find_group_pkgs(alpm_list_t *dbs, const char *name); * Sync */
-alpm_pkg_t *alpm_sync_newversion(alpm_pkg_t *pkg, alpm_list_t *dbs_sync); +alpm_pkg_t *alpm_sync_newversion(alpm_pkg_t *pkg, alpm_list_t *dbs_sync, int usage);
/** @addtogroup alpm_api_trans Transaction Functions * Functions to manipulate libalpm transactions diff --git a/lib/libalpm/sync.c b/lib/libalpm/sync.c index 05f58fad..3443bd71 100644 --- a/lib/libalpm/sync.c +++ b/lib/libalpm/sync.c @@ -51,7 +51,7 @@ /** Check for new version of pkg in sync repos * (only the first occurrence is considered in sync) */ -alpm_pkg_t SYMEXPORT *alpm_sync_newversion(alpm_pkg_t *pkg, alpm_list_t *dbs_sync) +alpm_pkg_t SYMEXPORT *alpm_sync_newversion(alpm_pkg_t *pkg, alpm_list_t *dbs_sync, int usage) { alpm_list_t *i; alpm_pkg_t *spkg = NULL; @@ -61,7 +61,7 @@ alpm_pkg_t SYMEXPORT *alpm_sync_newversion(alpm_pkg_t *pkg, alpm_list_t *dbs_syn
for(i = dbs_sync; !spkg && i; i = i->next) { alpm_db_t *db = i->data; - if(!(db->usage & ALPM_DB_USAGE_SEARCH)) { + if(!(db->usage & usage)) { continue; }
diff --git a/src/pacman/query.c b/src/pacman/query.c index 00c39638..1679662f 100644 --- a/src/pacman/query.c +++ b/src/pacman/query.c @@ -287,7 +287,8 @@ static int filter(alpm_pkg_t *pkg) } /* check if this pkg is outdated */ if(config->op_q_upgrade && (alpm_sync_newversion(pkg, - alpm_get_syncdbs(config->handle)) == NULL)) { + alpm_get_syncdbs(config->handle), + ALPM_DB_USAGE_ALL) == NULL)) { return 0; } return 1; @@ -325,10 +326,14 @@ static int display(alpm_pkg_t *pkg) colstr->version, alpm_pkg_get_version(pkg), colstr->nocolor);
if(config->op_q_upgrade) { - alpm_pkg_t *newpkg = alpm_sync_newversion(pkg, alpm_get_syncdbs(config->handle)); + int usage; + alpm_pkg_t *newpkg = alpm_sync_newversion(pkg, alpm_get_syncdbs(config->handle), ALPM_DB_USAGE_ALL); + alpm_db_t *db = alpm_pkg_get_db(newpkg); + alpm_db_get_usage(db, &usage); + printf(" -> %s%s%s", colstr->version, alpm_pkg_get_version(newpkg), colstr->nocolor);
- if(alpm_pkg_should_ignore(config->handle, pkg)) { + if(alpm_pkg_should_ignore(config->handle, pkg) || !(usage & ALPM_DB_USAGE_UPGRADE)) { printf(" %s", _("[ignored]")); } }
On 4/1/19 2:21 pm, Allan McRae wrote:
The behaviour of "pacman -Qu" was very strange... It would only consider packages from repos with Usage = Search (or All), and ignore those with Usage = Sync, Install or Upgrade.
This is because alpm_sync_newversion() used ALPM_DB_USAGE_SEARCH for its filtering. Given this function is documented (at least in the source) to "Check for new version of pkg in sync repos", I would expect that to look at all repos. However, just changing this parameter, would result in a fairly silent change in behaviour of this function. To counter that, add a parameter to the function that tells it which databases usage levels to consider.
Finally, list all available updates in -Qu output, but include [ignored] beside those that will not be updated in a -Su operation due to thier repo Usage value (in addition to those that are Ignored).
Fixes FS#59854.
With thanks to the following who provided initial patches to print [ignored] on -Qu operations, which highlighted the larger problem: morganamilo morganamilo@gmail.com Michael Straube michael.straube@posteo.de
Signed-off-by: Allan McRae allan@archlinux.org
In comments on earlier patches, there was debate about what the alpm_sync_newversion() is doing. Although I expect users of this function to likely always pass ALPM_DB_USAGE_ALL, I think changing the function signature is the safest way to handle this change.
Comments?
For reference, here are other threads about this:
https://patchwork.archlinux.org/patch/704/ https://patchwork.archlinux.org/patch/752/
On 01/04/19 at 02:21pm, Allan McRae wrote:
The behaviour of "pacman -Qu" was very strange... It would only consider packages from repos with Usage = Search (or All), and ignore those with Usage = Sync, Install or Upgrade.
This is because alpm_sync_newversion() used ALPM_DB_USAGE_SEARCH for its filtering. Given this function is documented (at least in the source) to "Check for new version of pkg in sync repos", I would expect that to look at all repos. However, just changing this parameter, would result in a fairly silent change in behaviour of this function. To counter that, add a parameter to the function that tells it which databases usage levels to consider.
Finally, list all available updates in -Qu output, but include [ignored] beside those that will not be updated in a -Su operation due to thier repo Usage value (in addition to those that are Ignored).
Fixes FS#59854.
With thanks to the following who provided initial patches to print [ignored] on -Qu operations, which highlighted the larger problem: morganamilo morganamilo@gmail.com Michael Straube michael.straube@posteo.de
Signed-off-by: Allan McRae allan@archlinux.org
In comments on earlier patches, there was debate about what the alpm_sync_newversion() is doing. Although I expect users of this function to likely always pass ALPM_DB_USAGE_ALL, I think changing the function signature is the safest way to handle this change.
Comments?
I was initially on board with this approach, but the more I think about it, the more convinced I am that having a function that takes an explicit list of db's filter them based on usage is a fundamentally bad design. As I said in the initial discussion, these are low-level functions that may be used in different contexts, in which case different usage filters would be appropriate. For example, this patch uses newversion with USAGE_ALL because it's being used for purely informational purposes. Manjaro's syncfirst patch, however, uses it to actually prepare an upgrade, making USAGE_UPGRADE the correct filter.
But, requiring the caller to provide a usage everywhere would defeat the purpose of making repo usages a back-end feature. We might as well just have the caller do the filtering. Given that there are only a handful of usage levels, the filtered lists could even be cached, completely saving us the hassle of iterating over db's that won't get used.
I think the best way to fulfill the original goal of adding repo usages would be to move the usage filtering to higher level functions that have more narrowly defined purposes and take a handle rather than a db list. This would allow callers that just want to "do the right thing" to not have to worry about usages while allowing callers with more specific needs to use the low-level functions without having to worry about what extra filtering alpm might be doing behind the scenes. It would also result in a cleaner API that doesn't have functions ignoring databases they were explicitly told to use.
On 4/1/19 7:32 pm, Andrew Gregory wrote:
On 01/04/19 at 02:21pm, Allan McRae wrote:
The behaviour of "pacman -Qu" was very strange... It would only consider packages from repos with Usage = Search (or All), and ignore those with Usage = Sync, Install or Upgrade.
This is because alpm_sync_newversion() used ALPM_DB_USAGE_SEARCH for its filtering. Given this function is documented (at least in the source) to "Check for new version of pkg in sync repos", I would expect that to look at all repos. However, just changing this parameter, would result in a fairly silent change in behaviour of this function. To counter that, add a parameter to the function that tells it which databases usage levels to consider.
Finally, list all available updates in -Qu output, but include [ignored] beside those that will not be updated in a -Su operation due to thier repo Usage value (in addition to those that are Ignored).
Fixes FS#59854.
With thanks to the following who provided initial patches to print [ignored] on -Qu operations, which highlighted the larger problem: morganamilo morganamilo@gmail.com Michael Straube michael.straube@posteo.de
Signed-off-by: Allan McRae allan@archlinux.org
In comments on earlier patches, there was debate about what the alpm_sync_newversion() is doing. Although I expect users of this function to likely always pass ALPM_DB_USAGE_ALL, I think changing the function signature is the safest way to handle this change.
Comments?
I was initially on board with this approach, but the more I think about it, the more convinced I am that having a function that takes an explicit list of db's filter them based on usage is a fundamentally bad design. As I said in the initial discussion, these are low-level functions that may be used in different contexts, in which case different usage filters would be appropriate. For example, this patch uses newversion with USAGE_ALL because it's being used for purely informational purposes. Manjaro's syncfirst patch, however, uses it to actually prepare an upgrade, making USAGE_UPGRADE the correct filter.
I was assuming that is a hypothetical! But the Manjaro patch really does use alpm_sync_newversion(). They are lucky that no-one probably used USAGE_SEARCH in their pacman.conf!
But, requiring the caller to provide a usage everywhere would defeat the purpose of making repo usages a back-end feature. We might as well just have the caller do the filtering. Given that there are only a handful of usage levels, the filtered lists could even be cached, completely saving us the hassle of iterating over db's that won't get used.
I think the best way to fulfill the original goal of adding repo usages would be to move the usage filtering to higher level functions that have more narrowly defined purposes and take a handle rather than a db list. This would allow callers that just want to "do the right thing" to not have to worry about usages while allowing callers with more specific needs to use the low-level functions without having to worry about what extra filtering alpm might be doing behind the scenes. It would also result in a cleaner API that doesn't have functions ignoring databases they were explicitly told to use.
I have sat and tried to figure out a cleaner API following these ideas and have failed... Can you give a brief prototype of what you mean?
Thanks, Allan
participants (2)
-
Allan McRae
-
Andrew Gregory