[pacman-dev] Outdated comments in sync.c code ?
First this one: 515 /* hmmm, depend.name isn't installed, so it must be conflicting 516 * with another package in our final list. For example: 517 * 518 * pacman -S blackbox xfree86 519 * 520 * If no x-servers are installed and blackbox pulls in xorg, then 521 * xorg and xfree86 will conflict with each other. In this case, 522 * we should follow the user's preference and rip xorg out of final, 523 * opting for xfree86 instead. 524 */ 525 526 /* figure out which one was requested in targets. If they both 527 * were, then it's still an unresolvable conflict. */ I don't get it. Since xfree86 is already in the target, why would blackbox pull xorg ? That's not what the code does, but maybe it did before (I only tried git and cvs) ? If the following pactest is correct, then this comment doesn't apply anymore : self.description = "Choose provider in the targets first" sp1 = pmpkg("blackbox") sp1.depends = ["x-server"] self.addpkg2db("sync", sp1) sp2 = pmpkg("xfree86") sp2.provides = ["x-server"] self.addpkg2db("sync", sp2) sp3 = pmpkg("xorg") sp3.provides = ["x-server"] sp3.conflicts = ["xfree86"] self.addpkg2db("sync", sp3) self.args = "-S blackbox xfree86" self.addrule("PACMAN_RETCODE=0") self.addrule("PKG_EXIST=blackbox") self.addrule("PKG_EXIST=xfree86") self.addrule("!PKG_EXIST=xorg") This one is even worse, I don't understand it at all : 638 /* XXX: this fails for cases where a requested package wants 639 * a dependency that conflicts with an older version of 640 * the package. It will be removed from final, and the user 641 * has to re-request it to get it installed properly. 642 * 643 * Not gonna happen very often, but should be dealt with... 644 */ I still tried to do a pactest following the instructions, and this works fine as well (git and cvs) : self.description = "A dep conflicts with older version of package" sp1 = pmpkg("pkg1", "1.0-2") sp1.depends = ["pkg2=1.0-2"] self.addpkg2db("sync", sp1) sp2 = pmpkg("pkg2", "1.0-2") sp2.conflicts = [ "pkg1=1.0-1" ] self.addpkg2db("sync", sp2) lp1 = pmpkg("pkg1", "1.0-1") lp1.depends = ["pkg2=1.0-1"] self.addpkg2db("local", lp1) lp2 = pmpkg("pkg2", "1.0-1") lp2.requiredby = [ "pkg1" ] self.addpkg2db("local", lp2) self.args = "-S pkg1" self.addrule("PACMAN_RETCODE=0") self.addrule("PKG_EXIST=pkg1") self.addrule("PKG_VERSION=pkg1|1.0-2") self.addrule("PKG_EXIST=pkg2") self.addrule("PKG_VERSION=pkg2|1.0-2")
On 7/27/07, Xavier <shiningxc@gmail.com> wrote:
First this one:
515 /* hmmm, depend.name isn't installed, so it must be conflicting 516 * with another package in our final list. For example: 517 * 518 * pacman -S blackbox xfree86 519 * 520 * If no x-servers are installed and blackbox pulls in xorg, then 521 * xorg and xfree86 will conflict with each other. In this case, 522 * we should follow the user's preference and rip xorg out of final, 523 * opting for xfree86 instead. 524 */ 525 526 /* figure out which one was requested in targets. If they both 527 * were, then it's still an unresolvable conflict. */
I don't get it. Since xfree86 is already in the target, why would blackbox pull xorg ? That's not what the code does, but maybe it did before (I only tried git and cvs) ?
If the following pactest is correct, then this comment doesn't apply anymore :
self.description = "Choose provider in the targets first"
sp1 = pmpkg("blackbox") sp1.depends = ["x-server"] self.addpkg2db("sync", sp1)
sp2 = pmpkg("xfree86") sp2.provides = ["x-server"] self.addpkg2db("sync", sp2)
sp3 = pmpkg("xorg") sp3.provides = ["x-server"] sp3.conflicts = ["xfree86"] self.addpkg2db("sync", sp3)
self.args = "-S blackbox xfree86"
self.addrule("PACMAN_RETCODE=0") self.addrule("PKG_EXIST=blackbox") self.addrule("PKG_EXIST=xfree86") self.addrule("!PKG_EXIST=xorg")
This one is even worse, I don't understand it at all :
638 /* XXX: this fails for cases where a requested package wants 639 * a dependency that conflicts with an older version of 640 * the package. It will be removed from final, and the user 641 * has to re-request it to get it installed properly. 642 * 643 * Not gonna happen very often, but should be dealt with... 644 */
I still tried to do a pactest following the instructions, and this works fine as well (git and cvs) :
self.description = "A dep conflicts with older version of package"
sp1 = pmpkg("pkg1", "1.0-2") sp1.depends = ["pkg2=1.0-2"] self.addpkg2db("sync", sp1)
sp2 = pmpkg("pkg2", "1.0-2") sp2.conflicts = [ "pkg1=1.0-1" ] self.addpkg2db("sync", sp2)
lp1 = pmpkg("pkg1", "1.0-1") lp1.depends = ["pkg2=1.0-1"] self.addpkg2db("local", lp1)
lp2 = pmpkg("pkg2", "1.0-1") lp2.requiredby = [ "pkg1" ] self.addpkg2db("local", lp2)
self.args = "-S pkg1"
self.addrule("PACMAN_RETCODE=0") self.addrule("PKG_EXIST=pkg1") self.addrule("PKG_VERSION=pkg1|1.0-2") self.addrule("PKG_EXIST=pkg2") self.addrule("PKG_VERSION=pkg2|1.0-2")
If you turn this into a GIT patch (it looks legit to me that you followed the directions) that adds two pactests and removes the wrong comments, I'll pull it in. You might want to name the tests a little more descriptively: "Ensure we choose provider already in target list" "Install package with dep that conflicts with older version of package" -Dan
On Wed, Aug 22, 2007 at 12:18:10AM -0400, Dan McGee wrote:
If you turn this into a GIT patch (it looks legit to me that you followed the directions) that adds two pactests and removes the wrong comments, I'll pull it in.
You might want to name the tests a little more descriptively: "Ensure we choose provider already in target list" "Install package with dep that conflicts with older version of package"
By the way, Nagy already removed these two comments when reworking the code in sync.c (everything in my sync branch). Otherwise, these pactests aren't very interesting, but maybe they could still be added as a reference. I actually have more interesting pactests, but I didn't submit them, because they all pass (or nearly). The thing is, I find pactest very useful for debugging and understanding the code. I generally add a new pactest for triggering a part of the code I'm looking at. The big problem I have is to know if an existing package already triggered it. There are so many of them, it's not easy to tell. And the more we add, the harder it will be. When I get a failing pactest though, I know it's different in a way with all others, so I'm more confident for submitting it :)
participants (2)
-
Dan McGee
-
Xavier