[pacman-dev] Outdated comments in sync.c code ?

[Dan McGee]

On 7/27/07, Xavier <shiningxc at 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