[pacman-dev] Fix reason handling before 3.1 (was: Final steps before 3.1 release)
On Dec 3, 2007 4:47 AM, Nagy Gabor <ngaba@bibl.u-szeged.hu> wrote:
Well, the changelog between 3.0.x and 3.1 will be huge and impressive;-) There are some issues which annoy me a bit: -sync044.py (but this is only one thing: see http://www.archlinux.org/pipermail/pacman-dev/2007-September/009271.html for details) <- %REASON% is one of my favourite libalpm features, and now this is quite buggy :-( -FS#8350
OK, I don't want to delay this release (imho we should release pacman more frequently <- but then we should do a message freeze between two major versions to help to our translators), but I think we should release 3.1.1 soon to fix these. Or to be precise, these are libalpm (libdownload?) issues, but in AL we have no separate libalpm package.
I think we can get this into 3.1 without a problem, IF we can exactly lay out the problem. From your other email, it looks like we should have a sequence of patches that do the following: 1. Add an --asexplicit option 2. First, determine a REASON policy for things such as installs, upgrades, and syncs including pulling in deps. Second, make a patch that implements this (and has comments in the code and commit msg!) 3. Implement HoldPkg reason (value of 2)? If they aren't already, these should become constants/enums in the code since we now have more than two possibilities. So, in short Nagy- pick what you want to do above and help us get this done. Also, let me know what Aaron or I need to do to help you. -Dan
Idézés Dan McGee <dpmcgee@gmail.com>:
On Dec 3, 2007 4:47 AM, Nagy Gabor <ngaba@bibl.u-szeged.hu> wrote:
Well, the changelog between 3.0.x and 3.1 will be huge and impressive;-) There are some issues which annoy me a bit: -sync044.py (but this is only one thing: see http://www.archlinux.org/pipermail/pacman-dev/2007-September/009271.html for details) <- %REASON% is one of my favourite libalpm features, and now this is quite buggy :-( -FS#8350
OK, I don't want to delay this release (imho we should release pacman more frequently <- but then we should do a message freeze between two major versions to help to our translators), but I think we should release 3.1.1 soon to fix these. Or to be precise, these are libalpm (libdownload?) issues, but in AL we have no separate libalpm package.
I think we can get this into 3.1 without a problem, IF we can exactly lay out the problem. From your other email, it looks like we should have a sequence of patches that do the following: 1. Add an --asexplicit option 2. First, determine a REASON policy for things such as installs, upgrades, and syncs including pulling in deps. Second, make a patch that implements this (and has comments in the code and commit msg!) 3. Implement HoldPkg reason (value of 2)? If they aren't already, these should become constants/enums in the code since we now have more than two possibilities.
So, in short Nagy- pick what you want to do above and help us get this done. Also, let me know what Aaron or I need to do to help you.
-Dan
3. is not important (I misinterpreted Holdpkg) Well, I overlooked something: "it owerwrites the old reason iff the new reason is depend" <- now this is not true imho (pactests?). %REASON% communication between upgrade and sync transactions is chaotic (via pkgcache), but looks OK: -sync.c presets reason: dependency iff this is a dependency (bit buggy, sync044.py) -add.c keep this result if no old version was found (package born), and copy the old reason if old version was found 1. is easy to implement, but we are in message freeze (or not?) 2. this needs sync.c (pmsyncpkg_t) rework, but currently sync.c is so hard-to-understand that I don't want to do this. Bye ---------------------------------------------------- SZTE Egyetemi Könyvtár - http://www.bibl.u-szeged.hu This mail sent through IMP: http://horde.org/imp/
On Mon, Dec 03, 2007 at 06:08:08PM +0100, Nagy Gabor wrote:
2. this needs sync.c (pmsyncpkg_t) rework, but currently sync.c is so hard-to-understand that I don't want to do this.
If you are referring to your pending sync.c patch, here is the situation : * your original patch breaks the sync040, sync041 and sync990 patch -> that might not be acceptable (at least Dan doesn't accept it :)) * my modified version of it breaks sync893, sync897, sync898, sync992 and sync999. Dan agreed that sync893 and sync897 pactests were weird and could be changed, but not the last 3 (btw, if we change these two pactests now, then applying my modified version of Nagy's patch won't make a big difference : 3 broken pactests instead of 2). But my opinion is that pacman shouldn't even try to guess which package the user wants when it can't know for sure, and that it should be left to the user to decide. Two possibilities for that : 1) interactively ask the user (not a real problem, but it's less easy to test this with pactests) 2) pacman fails and displays the package conflicts (the user can fix it using -S / -R) So let's have a look at the 3 problematic pactests. 1) sync898 --------------------------------------------------------------------- self.description = "System upgrade with conflicts and provides" sp1 = pmpkg("pkg1", "1.0-2") sp1.conflicts = ["pkg2"] sp1.provides = ["pkg2"] self.addpkg2db("sync", sp1); sp2 = pmpkg("pkg2", "1.0-2") self.addpkg2db("sync", sp2) lp1 = pmpkg("pkg1") self.addpkg2db("local", lp1) self.args = "-S %s" % " ".join([p.name for p in sp1, sp2]) self.addrule("PACMAN_RETCODE=0") self.addrule("PKG_MODIFIED=pkg1") self.addrule("!PKG_EXIST=pkg2") --------------------------------------------------------------------- The user tries to install sp1 and sp2, but these two packages conflict together. How does pacman know the user wouldn't prefer pkg2 over pkg1? If pacman just failed, then the user would have to make the decision, and either run "pacman -S pkg1" or "pacman -S pkg2". 2) sync 992 --------------------------------------------------------------------- self.description = "Sync a package pulling a conflicting dependency" sp1 = pmpkg("pkg1") sp1.depends = ["pkg3"] sp2 = pmpkg("pkg2") sp3 = pmpkg("pkg3") sp3.conflicts = ["pkg2"] sp3.provides = ["pkg2"] for p in sp1, sp2, sp3: self.addpkg2db("sync", p) lp1 = pmpkg("pkg2", "0.1-1") self.addpkg2db("local", lp1) self.args = "-S %s" % " ".join([p.name for p in sp1, sp2]) self.addrule("PACMAN_RETCODE=0") self.addrule("PKG_EXIST=pkg1") self.addrule("!PKG_EXIST=pkg2") self.addrule("PKG_EXIST=pkg3") --------------------------------------------------------------------- Well same problem here. The user asked for pkg1 and pkg2, yet pacman installed pkg1 and pkg3, and removed pkg2. What if pkg2 was more important? (and superior the the pkg3 provider) Other than that, I think this a strange situation. If pkg3 provides pkg2, it's usually because the other packages depend on pkg2 (yet pkg1 depends on pkg3 here, instead of pkg2). 3) sync 999 --------------------------------------------------------------------- self.description = "System upgrade" sp1 = pmpkg("pkg1", "1.0-2") sp1.conflicts = ["pkg2"] sp1.provides = ["pkg2"] self.addpkg2db("sync", sp1); sp2 = pmpkg("pkg2", "1.0-2") self.addpkg2db("sync", sp2) lp1 = pmpkg("pkg1") self.addpkg2db("local", lp1) lp2 = pmpkg("pkg2") self.addpkg2db("local", lp2) self.args = "-Su" self.addrule("PACMAN_RETCODE=0") self.addrule("PKG_EXIST=pkg1") self.addrule("!PKG_EXIST=pkg2") --------------------------------------------------------------------- And same problem here again, I don't understand why pacman has to choose pkg1 over pkg2.. Seems like pacman always prefer the package conflicting / providing the other. I can't understand why, because there is a (common?) situation where this doesn't make any sense : the mplayer-svn / mplayer one. If pacman really had to choose between both, it should rather pick the stable package, instead of the development one.
On Mon, Dec 03, 2007 at 06:08:08PM +0100, Nagy Gabor wrote:
2. this needs sync.c (pmsyncpkg_t) rework, but currently sync.c is so hard-to-understand that I don't want to do this.
If you are referring to your pending sync.c patch, here is the situation : ...
No, I didn't refer to that. But that is still interesting ;-) I just said, that I won't create patch for something which I don't really (want to) understand [that's why sync044.py-fix depends on sync.c-fix in my TODO list] 1. I just said that currently you can use PM_SYNC_TYPE_DEPEND xor PM_SYNC_TYPE_REPLACE (xor PM_SYNC_TYPE_UPGRADE) as sync-type, which prevent us from indicating replace _and_ depend. 2. PM_SYNC_TYPE_REPLACE is also "strange" <- it would be nice if we could copy reason between the _real_ to-be-replaced (util-linux) and replacer packages (util-linux-ng), but replace also indicates conflict resolving, which is a different(?) thing imho. [Replace is simply odd imho, but this is an other story: imho 'pacman -S util-linux-ng' should also offer remove util-linux] 3. I have some transaction problems again, sync creates an upgrade transaction, upgrade transaction creates a remove transaction and it is difficult to follow what and where happens -- 2. is a good example, where the current 'partly done by add.c, partly by sync.c' method doesn't work <- my preferred reason handling would be fully done in sync.c in case of sync transaction [*] 4. There are some other unclear thoughts in my mind: In some cases user want to _replace_ packageB with packageA, however packageA doesn't replace (I mean %REPLACES% here) packageB <- for example packageA and packageB have the same provision, and user want to change... Something similar(?) happens with most conflict resolving, too... And we reached universal transaction again ;-) (see also [*]), maybe one day we can rename-and-merge sync.c to trans.c... These goals (2.) cannot be reached within 2 weeks, and currently %REASON% handling works "somehow" (I overlooked something and I thought that this is totally broken, sorry), so my complaint is reverted. [To be honest, 1. I never like quick-hack bugfixes; and we need a concept first 2. I want to completely rework sync.c where this PM_SYNC_TYPE_DEPEND<->PM_SYNC_TYPE_REPLACE conflict won't be a problem.] Bye PS: If you still want to fix sync044.py only [to be clear: I won't], some pmsyncpkg_t suggestions (again, I don't fully understand sync.c, so be careful ;-): -TYPE_UPGRADE is needless -void ".data" can be renamed to alpm_list_t .replaces, and non-empty replaces simply means the old TYPE_REPLACES (<-so we don't need this synctype neither imho) ---------------------------------------------------- SZTE Egyetemi Könyvtár - http://www.bibl.u-szeged.hu This mail sent through IMP: http://horde.org/imp/
On Mon, Dec 03, 2007 at 08:26:05PM +0100, Nagy Gabor wrote:
On Mon, Dec 03, 2007 at 06:08:08PM +0100, Nagy Gabor wrote:
2. this needs sync.c (pmsyncpkg_t) rework, but currently sync.c is so hard-to-understand that I don't want to do this.
If you are referring to your pending sync.c patch, here is the situation : ...
No, I didn't refer to that. But that is still interesting ;-) I just said, that I won't create patch for something which I don't really (want to) understand [that's why sync044.py-fix depends on sync.c-fix in my TODO list]
Ok, so I just think this is another argument for considering the merge of your sync.c fix as soon as possible, and this could motivate you to work on a fix for sync044 and/or other related things after 3.1 release.
Idézés Xavier <shiningxc@gmail.com>:
On Mon, Dec 03, 2007 at 06:08:08PM +0100, Nagy Gabor wrote:
2. this needs sync.c (pmsyncpkg_t) rework, but currently sync.c is so hard-to-understand that I don't want to do this.
If you are referring to your pending sync.c patch, here is the situation : ...
No, I didn't refer to that. But that is still interesting ;-) I just said,
On Mon, Dec 03, 2007 at 08:26:05PM +0100, Nagy Gabor wrote: that
I won't create patch for something which I don't really (want to) understand [that's why sync044.py-fix depends on sync.c-fix in my TODO list]
Ok, so I just think this is another argument for considering the merge of your sync.c fix as soon as possible, and this could motivate you to work on a fix for sync044 and/or other related things after 3.1 release.
Thx, and of course I totally agree with you ;-) I add an other small issue here: when the current code removes a package from the target list it also removes the locally installed package (if any), which can be needless (versioned conflicts, provision-conflicts, etc.) <- but at least by doing this it avoids appearing new conflicts Bye ---------------------------------------------------- SZTE Egyetemi Könyvtár - http://www.bibl.u-szeged.hu This mail sent through IMP: http://horde.org/imp/
participants (3)
-
Dan McGee
-
Nagy Gabor
-
Xavier