[pacman-dev] [PATCH] Fix warning messages when syncing installed package
The patch fixes the warning messages when syncing a package where the local version is newer that the db version. Also, all messages are suppressed when using --print-uris and messages about upgrading or reinstalling are suppressed when using --downloadonly Signed-off-by: Allan McRae <mcrae_allan@hotmail.com> --- lib/libalpm/sync.c | 33 +++++++++++++++++++++++++++------ 1 files changed, 27 insertions(+), 6 deletions(-) diff --git a/lib/libalpm/sync.c b/lib/libalpm/sync.c index 3dc54d0..f10e1b3 100644 --- a/lib/libalpm/sync.c +++ b/lib/libalpm/sync.c @@ -333,19 +333,40 @@ int _alpm_sync_addtarget(pmtrans_t *trans, pmdb_t *db_local, alpm_list_t *dbs_sy local = _alpm_db_get_pkgfromcache(db_local, alpm_pkg_get_name(spkg)); if(local) { - if(_alpm_pkg_compare_versions(local, spkg) == 0) { - /* spkg is NOT an upgrade */ + int cmp = alpm_pkg_vercmp(alpm_pkg_get_version(spkg), alpm_pkg_get_version(local)); + + if(cmp == -1) { + /* local is newer than spkg */ + _alpm_db_read(spkg->origin_data.db, spkg, INFRQ_DESC); + if(spkg->force) { + if(!(trans->flags & (PM_TRANS_FLAG_DOWNLOADONLY | PM_TRANS_FLAG_PRINTURIS))) { + _alpm_log(PM_LOG_WARNING, _("%s: forcing upgrade to version %s\n"), + alpm_pkg_get_name(spkg), alpm_pkg_get_version(spkg)); + } + } else { + /* we are downgrading */ + if(!(trans->flags & (PM_TRANS_FLAG_DOWNLOADONLY | PM_TRANS_FLAG_PRINTURIS))) { + pmdb_t *db = spkg->origin_data.db; + _alpm_log(PM_LOG_WARNING, _("%s-%s is newer than in %s -- downgrading\n"), + alpm_pkg_get_name(local), alpm_pkg_get_version(local), + alpm_db_get_name(db)); + } + } + } else if(cmp == 0) { + /* spkg has same version as local */ if(trans->flags & PM_TRANS_FLAG_NEEDED) { - _alpm_log(PM_LOG_WARNING, _("%s-%s is up to date -- skipping\n"), - alpm_pkg_get_name(local), alpm_pkg_get_version(local)); + if(!(trans->flags & PM_TRANS_FLAG_PRINTURIS)) { + _alpm_log(PM_LOG_WARNING, _("%s-%s is up to date -- skipping\n"), + alpm_pkg_get_name(local), alpm_pkg_get_version(local)); + } return(0); } else { - if(!(trans->flags & PM_TRANS_FLAG_DOWNLOADONLY)) { + if(!(trans->flags & (PM_TRANS_FLAG_DOWNLOADONLY | PM_TRANS_FLAG_PRINTURIS))) { _alpm_log(PM_LOG_WARNING, _("%s-%s is up to date -- reinstalling\n"), alpm_pkg_get_name(local), alpm_pkg_get_version(local)); } } - } + } /* else cmp == 1 - spkg is newer than local */ } /* add the package to the transaction */ -- 1.5.5.3
Allan McRae wrote:
The patch fixes the warning messages when syncing a package where the local version is newer that the db version. Also, all messages are suppressed when using --print-uris and messages about upgrading or reinstalling are suppressed when using --downloadonly <snip>
Note that I have been unable to find a convenient package to test the forced upgrade section on due to the whole DB regeneration/force flag issue. There was no change in pactest results and I assume that it is covered there... Allan
On Sat, Jun 7, 2008 at 6:05 PM, Allan McRae <mcrae_allan@hotmail.com> wrote:
The patch fixes the warning messages when syncing a package where the local version is newer that the db version. Also, all messages are suppressed when using --print-uris and messages about upgrading or reinstalling are suppressed when using --downloadonly
Signed-off-by: Allan McRae <mcrae_allan@hotmail.com> ---
Did you see this : http://archlinux.org/pipermail/pacman-dev/2008-June/011994.html ? I changed the value of pkg_vercmp to provide more informations. I also removed all these messages because I finally found them useless. But if you disagree, you might be able to use the more detailed result of pkg_vercmp to provide a better output.
Xavier wrote:
On Sat, Jun 7, 2008 at 6:05 PM, Allan McRae <mcrae_allan@hotmail.com> wrote:
The patch fixes the warning messages when syncing a package where the local version is newer that the db version. Also, all messages are suppressed when using --print-uris and messages about upgrading or reinstalling are suppressed when using --downloadonly
Signed-off-by: Allan McRae <mcrae_allan@hotmail.com> ---
Did you see this : http://archlinux.org/pipermail/pacman-dev/2008-June/011994.html ? I changed the value of pkg_vercmp to provide more informations. I also removed all these messages because I finally found them useless. But if you disagree, you might be able to use the more detailed result of pkg_vercmp to provide a better output.
Um, no.... I missed that completely. I think your version is the much cleaner way to do this. However, it still prints the warning messages ("forcing upgrade" & "is newer than") in _alpm_pkg_compare_versions when using --print-uris and --downloadonly. Allan
On Mon, Jun 9, 2008 at 10:48 AM, Allan McRae <mcrae_allan@hotmail.com> wrote:
I think your version is the much cleaner way to do this. However, it still prints the warning messages ("forcing upgrade" & "is newer than") in _alpm_pkg_compare_versions when using --print-uris and --downloadonly.
On the other hand, I don't really like all these specific handling for these 2 options. If we don't want logging when these options are used, why not just disabling the logging altogether? Something like this maybe : diff --git a/src/pacman/sync.c b/src/pacman/sync.c index 47ab4eb..90f2129 100644 --- a/src/pacman/sync.c +++ b/src/pacman/sync.c @@ -766,6 +766,10 @@ int pacman_sync(alpm_list_t *targets) { alpm_list_t *sync_dbs = NULL; + if(config->flags & (PM_TRANS_FLAG_DOWNLOADONLY | PM_TRANS_FLAG_PRINTURIS)) { + config->logmask = 0; + } + /* clean the cache */ if(config->op_s_clean) { int ret = 0;
Xavier wrote:
On the other hand, I don't really like all these specific handling for these 2 options. If we don't want logging when these options are used, why not just disabling the logging altogether? Something like this maybe :
diff --git a/src/pacman/sync.c b/src/pacman/sync.c index 47ab4eb..90f2129 100644 --- a/src/pacman/sync.c +++ b/src/pacman/sync.c @@ -766,6 +766,10 @@ int pacman_sync(alpm_list_t *targets) { alpm_list_t *sync_dbs = NULL;
+ if(config->flags & (PM_TRANS_FLAG_DOWNLOADONLY | PM_TRANS_FLAG_PRINTURIS)) { + config->logmask = 0; + } + /* clean the cache */ if(config->op_s_clean) { int ret = 0;
Sure. Is there any warning message that is actually useful when using those two options? I can't think of anything but you and Dan know the code-base far better than I do.
On Mon, Jun 9, 2008 at 12:02 PM, Allan McRae <mcrae_allan@hotmail.com> wrote:
Sure. Is there any warning message that is actually useful when using those two options? I can't think of anything but you and Dan know the code-base far better than I do.
There might be some useful warning / error messages in case something is wrong, like a corrupted database or other problems like that. But I would think that most of them would happen on normal -S operations too, so it might not be a big problem. I am not sure though.
On Mon, Jun 9, 2008 at 6:07 AM, Xavier <shiningxc@gmail.com> wrote:
On Mon, Jun 9, 2008 at 12:02 PM, Allan McRae <mcrae_allan@hotmail.com> wrote:
Sure. Is there any warning message that is actually useful when using those two options? I can't think of anything but you and Dan know the code-base far better than I do.
There might be some useful warning / error messages in case something is wrong, like a corrupted database or other problems like that. But I would think that most of them would happen on normal -S operations too, so it might not be a big problem. I am not sure though.
Why not screen it down to errors only? That way we don't hid those if they occur, but the warnings that these messages omit will be gone. I would rather not have special case handling for these in the backend code. -Dan
On Mon, Jun 9, 2008 at 1:43 PM, Dan McGee <dpmcgee@gmail.com> wrote:
Why not screen it down to errors only? That way we don't hid those if they occur, but the warnings that these messages omit will be gone.
I would rather not have special case handling for these in the backend code.
Ah yes of course, that sounds even better. If warning and errors messages are defined correctly (as they should), then that is just fine; warnings can be silently ignored. But otherwise, that is what is wrong about the way -Sp and -Sw are currently implemented : it is full of special case handling everywhere in the code. And trying to fix that is.. impossible.
On Mon, Jun 9, 2008 at 10:48 AM, Allan McRae <mcrae_allan@hotmail.com> wrote:
Um, no.... I missed that completely.
I think your version is the much cleaner way to do this. However, it still prints the warning messages ("forcing upgrade" & "is newer than") in _alpm_pkg_compare_versions when using --print-uris and --downloadonly.
But note that my patch totally removes the "uptodate -- skipping" and "uptodate -- reinstalling" messages. I am still wondering whether we want them or not. They might be informative when there is a small number of targets, but when you want to install / reinstall a group : pacman -S base or to complete a group with the missing packages : pacman -S --needed base then it is totally unreadable and useless. Maybe an indicator which shows which targets are already up-to-date would be more useful. Or even always showing the currently installed version of each targets, which is imo a very useful information. Like that column based output did : http://www.archlinux.org/pipermail/pacman-dev/2008-March/011315.html
Xavier wrote:
On Mon, Jun 9, 2008 at 10:48 AM, Allan McRae <mcrae_allan@hotmail.com> wrote:
Um, no.... I missed that completely.
I think your version is the much cleaner way to do this. However, it still prints the warning messages ("forcing upgrade" & "is newer than") in _alpm_pkg_compare_versions when using --print-uris and --downloadonly.
But note that my patch totally removes the "uptodate -- skipping" and "uptodate -- reinstalling" messages. I am still wondering whether we want them or not. They might be informative when there is a small number of targets, but when you want to install / reinstall a group : pacman -S base or to complete a group with the missing packages : pacman -S --needed base then it is totally unreadable and useless.
Maybe an indicator which shows which targets are already up-to-date would be more useful. Or even always showing the currently installed version of each targets, which is imo a very useful information. Like that column based output did : http://www.archlinux.org/pipermail/pacman-dev/2008-March/011315.html
Should these sort of messages be in the backend at all?
On Tue, Jun 10, 2008 at 2:51 PM, Allan McRae <mcrae_allan@hotmail.com> wrote:
Should these sort of messages be in the backend at all?
All these messages coming from the backend seem a bit weird conceptually to me, even though it uses a callback defined in the frontend. But well, this is done everywhere in libalpm, and it is often more practical, so I don't know. Anyway, I think these uptodate messages could be dropped for now, and then maybe we could find a better way to display this information in the future.
participants (3)
-
Allan McRae
-
Dan McGee
-
Xavier