[pacman-dev] bogus reinstalling message
This issue has been known for ages, and it is easy to fix it, so I would like to do it. http://archlinux.org/pipermail/pacman-dev/2008-May/011885.html Example of a bogus output taken from the forums (the thread does not talk about this issue though) : http://bbs.archlinux.org/viewtopic.php?pid=343166#p343166 My attached solution simply removes the messages, which are in my opinion useless. But I asked if it was a problem here : http://archlinux.org/pipermail/pacman-dev/2008-June/012095.html If this is a problem, the rest of my patch will allow me to easily reintroduce these messages when they are relevant.
On Fri, Aug 22, 2008 at 12:03 PM, Xavier
This issue has been known for ages, and it is easy to fix it, so I would like to do it. http://archlinux.org/pipermail/pacman-dev/2008-May/011885.html Example of a bogus output taken from the forums (the thread does not talk about this issue though) : http://bbs.archlinux.org/viewtopic.php?pid=343166#p343166
My attached solution simply removes the messages, which are in my opinion useless. But I asked if it was a problem here : http://archlinux.org/pipermail/pacman-dev/2008-June/012095.html If this is a problem, the rest of my patch will allow me to easily reintroduce these messages when they are relevant.
To be clearer, here are the 3 situations possible : 1) local version (release 1) is older than sync (2) myhost% LANG=C sudo pacman -S bash resolving dependencies... looking for inter-conflicts... Targets (1): bash-3.2.039-2 2) local and sync are the same : 2 myhost% LANG=C sudo pacman -S bash warning: bash-3.2.039-2 is up to date -- reinstalling resolving dependencies... looking for inter-conflicts... Targets (1): bash-3.2.039-2 3) local (3) is newer than sync (2) myhost% LANG=C sudo pacman -S bash warning: bash: local (3.2.039-3) is newer than core (3.2.039-2) warning: bash-3.2.039-3 is up to date -- reinstalling resolving dependencies... looking for inter-conflicts... Targets (1): bash-3.2.039-2 1) and 2) are already fine. Only 3) is wrong : here we don't want to display the "up to date" message.
On Fri, Aug 22, 2008 at 12:03 PM, Xavier
wrote: This issue has been known for ages, and it is easy to fix it, so I would like to do it. http://archlinux.org/pipermail/pacman-dev/2008-May/011885.html Example of a bogus output taken from the forums (the thread does not talk about this issue though) : http://bbs.archlinux.org/viewtopic.php?pid=343166#p343166
My attached solution simply removes the messages, which are in my opinion useless. But I asked if it was a problem here : http://archlinux.org/pipermail/pacman-dev/2008-June/012095.html If this is a problem, the rest of my patch will allow me to easily reintroduce these messages when they are relevant.
To be clearer, here are the 3 situations possible :
1) local version (release 1) is older than sync (2)
myhost% LANG=C sudo pacman -S bash resolving dependencies... looking for inter-conflicts...
Targets (1): bash-3.2.039-2
2) local and sync are the same : 2
myhost% LANG=C sudo pacman -S bash warning: bash-3.2.039-2 is up to date -- reinstalling resolving dependencies... looking for inter-conflicts...
Targets (1): bash-3.2.039-2
3) local (3) is newer than sync (2)
myhost% LANG=C sudo pacman -S bash warning: bash: local (3.2.039-3) is newer than core (3.2.039-2) warning: bash-3.2.039-3 is up to date -- reinstalling resolving dependencies... looking for inter-conflicts...
Targets (1): bash-3.2.039-2
1) and 2) are already fine. Only 3) is wrong : here we don't want to display the "up to date" message. _______________________________________________
0. I don't like this _alpm_pkg_compare_versions() function at all, it has some annoying warning messages (why is the "forcing upgrade..." message important?), these reduce its usability (hardwired "upgrade" word etc.). This function is just a vercmp + force check + printing some (useless?) warnings. (The second "downgrade" warning also seems a bit odd in "-S target" operation because of mentioning the repo.) 1. After your patch we won't display warning in 2) case neither (as you wrote in patch description). I am not sure this is better. I would simply inform user about both downgrade (but not in pkg_compare_versions!) and reinstall instead. 2. The patch silently modifies (fixes?) --needed behavior (probably this is why you changed its description, but it wasn't clear to me first): Before your patch --needed prevented us from _downgrade_ too, now it will allow it. Bye
On Fri, Aug 22, 2008 at 2:56 PM, Nagy Gabor
First, thanks for the feedback!
0. I don't like this _alpm_pkg_compare_versions() function at all, it has some annoying warning messages (why is the "forcing upgrade..." message important?), these reduce its usability (hardwired "upgrade" word etc.). This function is just a vercmp + force check + printing some (useless?) warnings. (The second "downgrade" warning also seems a bit odd in "-S target" operation because of mentioning the repo.)
I agree that these warning messages are annoying and rather useless. However, I like the idea of having a simple wrapper to vercmp handling the force flag (and maybe the epoch idea in 10 years :P) Finally, why is it odd to mention the repo?
1. After your patch we won't display warning in 2) case neither (as you wrote in patch description). I am not sure this is better. I would simply inform user about both downgrade (but not in pkg_compare_versions!) and reinstall instead.
Could you be much more precise here? There are 3 different values returned by compare_versions, and 2 values for the needed flag, which makes 6 different combination. downgrade only applies when compare_versions == -1 and with both values of needed (because of point 2. below) reinstall happens when compare_versions == 0 and needed == 0 So you want to display these messages only in these cases? And nothing in the other cases? (like skipping when compare_versions == 0 and needed == 1)
2. The patch silently modifies (fixes?) --needed behavior (probably this is why you changed its description, but it wasn't clear to me first): Before your patch --needed prevented us from _downgrade_ too, now it will allow it.
Good point, I made this patch a while ago and I don't even know if I realized that. At least I don't remember anymore. Thinking about it now, I prefer the new behavior and see my change as a fix. But I agree this should clearly appear in the commit log.
On Fri, Aug 22, 2008 at 2:56 PM, Nagy Gabor
wrote: First, thanks for the feedback!
0. I don't like this _alpm_pkg_compare_versions() function at all, it has some annoying warning messages (why is the "forcing upgrade..." message important?), these reduce its usability (hardwired "upgrade" word etc.). This function is just a vercmp + force check + printing some (useless?) warnings. (The second "downgrade" warning also seems a bit odd in "-S target" operation because of mentioning the repo.)
I agree that these warning messages are annoying and rather useless.
However, I like the idea of having a simple wrapper to vercmp handling the force flag (and maybe the epoch idea in 10 years :P)
Btw, in your patch (without downgrade messages) we shouldn't check force in sync_addtarget, because force cannot affect "vercmp != 0" result. (I assume that --needed filters out reinstalls only). So pkg_vercmp (or strcmp;-) is enough here imho (without upgrade/downgrade distinction).
Finally, why is it odd to mention the repo?
OK. This is not odd, just strange. Clearly, this was designed for -Su. No other -S messages uses this "from repo extra" phrasing.
1. After your patch we won't display warning in 2) case neither (as you wrote in patch description). I am not sure this is better. I would simply inform user about both downgrade (but not in pkg_compare_versions!) and reinstall instead.
Could you be much more precise here?
There are 3 different values returned by compare_versions, and 2 values for the needed flag, which makes 6 different combination.
downgrade only applies when compare_versions == -1 and with both values of needed (because of point 2. below)
reinstall happens when compare_versions == 0 and needed == 0
So you want to display these messages only in these cases? And nothing in the other cases? (like skipping when compare_versions == 0 and needed == 1)
I am talking about "-S target" not about -Su (sync_newversion). 1. Target is newer: usual case, no warning 2. Reinstall was requested (local_ver == target_ver): Skipping/Reinstalling foo... (1.0-1 => 1.0-1) 3. Target is older: Downgrading foo... (2.0-1 => 1.0-1) [These => notations can be confusing with force flagged packages.] I am a bit unsure about downgrade warning idea (after thinking a bit). (However, atm if pkg_compare_versions is used, we get a downgrade warning.) It would be cool (imho), but in fact we use downgrade/upgrade distinction nowhere else. (see -U, resolvedeps added packages (edge), many event/debug messages and so on.) Bye
On Fri, Aug 22, 2008 at 4:55 PM, Nagy Gabor
I am talking about "-S target" not about -Su (sync_newversion). 1. Target is newer: usual case, no warning 2. Reinstall was requested (local_ver == target_ver): Skipping/Reinstalling foo... (1.0-1 => 1.0-1) 3. Target is older: Downgrading foo... (2.0-1 => 1.0-1)
Ok then, I guess we more or less keep the same behavior. We just need to make a better distinction between 2 and 3, which is why I changed the _alpm_pkg_compare_versions function. Here is a new patch (inlined for comments, attached to be sure that it will apply cleanly)
From 0cbe750a2fbaf0e43055d8c2c9a1b3dcec7089b9 Mon Sep 17 00:00:00 2001 From: Xavier Chantry
Date: Sat, 31 May 2008 15:06:30 +0200 Subject: [PATCH] Cleanup of _alpm_pkg_compare_versions.
* Change the return values to be more informative.
It was previously boolean, only indicating if one package was newer than the
other or not.
Now it is a simple wrapper to vercmp, handling the force flag.
* Remove the verbose output. This belongs somewhere else.
* Don't display the "up to date -- skipping" and "up to date -- reinstalling"
messages, when the local version is newer than the sync one.
* Fix the behavior of --needed option to not skip a target when the local
version is newer, and clarify its description.
* Add a new alpm_pkg_has_force function
This allows us to access the pkg->force field like any other package fields.
Signed-off-by: Xavier Chantry
* Change the return values to be more informative.
It was previously boolean, only indicating if a sync package was newer than
a local package.
Now it is a simple wrapper to vercmp, handling the force flag.
* Remove the verbose output from _alpm_pkg_compare_versions.
The "force" message is not so useful.
The "package : local (v1) is newer than repo (v2)" message can be moved to
-Su operation.
For the -S operation, it is better to have something like :
"downgrading package from v1 to v2"
* Don't display the "up to date -- skipping" and "up to date -- reinstalling"
messages, when the local version is newer than the sync one.
* Fix the behavior of --needed option to not skip a target when the local
version is newer, and clarify its description.
* Add a new alpm_pkg_has_force function
This allows us to access the pkg->force field like any other package fields.
Signed-off-by: Xavier Chantry
participants (3)
-
Nagy Gabor
-
Xavier
-
Xavier Chantry