Hello, Na Fri, Aug 17, 2007 at 12:52:56AM +0200, Xavier pisal(a):

There are several bugs fixed in 3.1 (mostly fix from Nagy), but also that bug with tr_TR locale that makes pacman unusable.

also upgraders should be notified about that you dropped support for ignorepkg and IGNOREPKG (which is probably used by many users) - VMiklos

Hello, Na Thu, Aug 23, 2007 at 05:08:33PM +0200, Xavier pisal(a):

On Thu, Aug 23, 2007 at 04:54:06PM +0200, VMiklos wrote:

...

Hello,

Na Fri, Aug 17, 2007 at 12:52:56AM +0200, Xavier pisal(a):

...

There are several bugs fixed in 3.1 (mostly fix from Nagy), but also that bug with tr_TR locale that makes pacman unusable.

also upgraders should be notified about that you dropped support for ignorepkg and IGNOREPKG (which is probably used by many users)

Please elaborate. As far as I can tell, it was never dropped, and it seems to work fine here.

hmm, IGNOREPKG still works, right. but ignorepkg will no longer work, right? - VMiklos

On Fri, Aug 24, 2007 at 11:50:35PM +0200, Xavier wrote:

If I understood correctly, this comp does not work correctly with tr_TR locale, so a case sensitive one had to be added as well.

On this matter, Dan added a comment in his working branch : /* For each directive, compare to the uppercase and camelcase string. * This prevents issues with certain locales where characters don't * follow the toupper() rules we may expect, e.g. tr_TR where i != I. */ Shouldn't it be rather : "where upper(i) != I" or something ?

On Sat, Aug 25, 2007 at 01:22:23AM +0200, VMiklos wrote:

if !strcasecmp(), then the comment is right. but still, the whole bugfix is about tr_TR and i think ignorepkg is still not handled for tr_TR..

Oh, you were talking about this particular weird locale.. I didn't get that. I think it's already nice that the default keyword, as seen in the manpage (so IgnorePkg) now works. These double comparisons are already ugly enough like that in my opinion. Not sure we want to add a third.

Hi! And please don't forget about the PS of http://www.archlinux.org/pipermail/pacman-dev/2007-August/009078.html data field of pmsyncpkg_t is used iff type == PM_SYNC_TYPE_REPLACE now, so we could use (and rename?) that field as replaces in all cases to avoid the problem above, and so pmsyncpkg_t would be a bit nicer (see alpm_sync_pkg_free);-). So PM_SYNC_TYPE_REPLACE is not needed IMHO (we record to the database explicit or depend reason only): (data != NULL) <=> (type == PM_SYNC_TYPE_REPLACE) if you keep my advice. Bye, ngaba ---------------------------------------------------- SZTE Egyetemi Könyvtár - http://www.bibl.u-szeged.hu This mail sent through IMP: http://horde.org/imp/

On Wed, Sep 05, 2007 at 12:12:54PM +0200, Nagy Gabor wrote:

Hi! And please don't forget about the PS of http://www.archlinux.org/pipermail/pacman-dev/2007-August/009078.html data field of pmsyncpkg_t is used iff type == PM_SYNC_TYPE_REPLACE now, so we could use (and rename?) that field as replaces in all cases to avoid the problem above, and so pmsyncpkg_t would be a bit nicer (see alpm_sync_pkg_free);-). So PM_SYNC_TYPE_REPLACE is not needed IMHO (we record to the database explicit or depend reason only): (data != NULL) <=> (type == PM_SYNC_TYPE_REPLACE) if you keep my advice.

As always, a pactest and a patch would make it much easier for us to understand. But about this syncpkg stuff, I just noticed recently that the universal transaction idea you're mentioning in the reply : http://www.archlinux.org/pipermail/pacman-dev/2007-August/009082.html is the first entry in aaron TODO list : * transaction object should contain two package list (install and remove) instead of a single list of syncpkgs - this should allow us to get rid of that type. This also requires seperate functionality to return a list of "replaces" packages to the front end, so the frontend can handle the QUESTION() stuff in that case But maybe you already read that before :) Anyway, I already liked this idea, and seeing it there made me more confident :)

On Tue, Sep 04, 2007 at 11:41:50AM +0200, Nagy Gabor wrote:

...

...

There are several bugs fixed in 3.1 (mostly fix from Nagy), ...

Well, I suggest rewriting _alpm_sync_prepare as soon as possible. You

needn't

...

apply my patch, but the current version is buggy and messy. Without going into details: -sync1003.py: you need a more clever checkdeps. -alpm_checkdeps is called in the beginning of the function, THEN some hard-to-understand manipulations are done in the target list to resolve conflicts, and (upgrade) dependencies are not checked again!

But the manipulations done there mostly cause the removal of packages. There may be a particular case where it fails, but I don't really know.

...

-removing a package from target list may induce a new conflict (for example: the old version conflicted, but we assumed that it will be upgraded to a non-conflicting one; but after we removed the upgrade-package, we got the conflict again)

That case looks a bit odd, but well, I tried to write a pactest for it, but couldn't reproduce this exact situation? Semi-pactest file: local: foo-1.0-1 bar-1.0-1 sync: foo-2.0-1 conflicts with bar-2.0-1 bar-2.0-1 conflicts with foo-2.0-1

This is similar to sync1003. It's very dangerous to check deps in the beginning of the function. If you later remove a package (during conflict resolution for example) from the target list, you _mustn't_ trust on the first checkdeps result (the removed package may be needed for an other package in the target list; the first checkdeps assumed that a local package will be overwritten with the removed-from-target package...; checkdeps with TRANS_TYPE_REMOVE won't detect these!). I don't want to waste more word for this; the _concept_ is buggy. pkg-1.0-1 conflicts with foo-1.0-1 and bar-1.0-1 User does a pacman -S "foo bar pkg". I know, that this is rare, but if pacman will be able to resolve a pkglist<->local conflict with remove the package from the target list instead of removing the local one (why don't we let user decide?), this can happen more often.

...

You can create pactest files to test these. (IIRC, my patches solve all the problems above.)

But who can? Apparently not me, at least :)

_______________________________________________ pacman-dev mailing list pacman-dev@archlinux.org http://archlinux.org/mailman/listinfo/pacman-dev

---------------------------------------------------- SZTE Egyetemi Könyvtár - http://www.bibl.u-szeged.hu This mail sent through IMP: http://horde.org/imp/

Idézés Nagy Gabor :

...

On Tue, Sep 04, 2007 at 11:41:50AM +0200, Nagy Gabor wrote:

...

...

There are several bugs fixed in 3.1 (mostly fix from Nagy), ...

Well, I suggest rewriting _alpm_sync_prepare as soon as possible. You

needn't

...

apply my patch, but the current version is buggy and messy. Without going into details: -sync1003.py: you need a more clever checkdeps. -alpm_checkdeps is called in the beginning of the function, THEN some hard-to-understand manipulations are done in the target list to resolve conflicts, and (upgrade) dependencies are not checked again!

But the manipulations done there mostly cause the removal of packages. There may be a particular case where it fails, but I don't really know.

This is similar to sync1003. It's very dangerous to check deps in the beginning of the function. If you later remove a package (during conflict resolution for example) from the target list, you _mustn't_ trust on the first checkdeps result (the removed package may be needed for an other package in the target list; the first checkdeps assumed that a local package will be overwritten with the removed-from-target package...; checkdeps with TRANS_TYPE_REMOVE won't detect these!). I don't want to waste more word for this; the _concept_ is buggy.

...

...

-removing a package from target list may induce a new conflict (for example: the old version conflicted, but we assumed that it will be upgraded to a non-conflicting one; but after we removed the upgrade-package, we got the conflict again)

That case looks a bit odd, but well, I tried to write a pactest for it, but couldn't reproduce this exact situation? Semi-pactest file: local: foo-1.0-1 bar-1.0-1 sync: foo-2.0-1 conflicts with bar-2.0-1 bar-2.0-1 conflicts with foo-2.0-1 pkg-1.0-1 conflicts with foo-1.0-1 and bar-1.0-1 User does a pacman -S "foo bar pkg". I know, that this is rare, but if pacman will be able to resolve a pkglist<->local conflict with remove the package from the target list instead of removing the local one (why don't we let user decide?), this can happen more often.

...

...

You can create pactest files to test these. (IIRC, my patches solve all the problems above.)

But who can? Apparently not me, at least :)

OMG, I noticed that current pacman won't resolve target<->target conflict (it stops with unresolveable conflict error <- so that ~150 line conflict-resolution monster is not so efficient in sync.c ;-). However pacman may still remove packages from the target list: sync.c: /* pacman -S blackbox xfree86 ... */. I don't understand that part at all: resolvedeps first search for (blackbox's) dependency satisfiers in the target list. So it will find xfree86, if that dep is OK [in this case we don't need to remove anything]; if xfree86 dep is not OK, resolvedeps must find and pull a real satisfier (xorg), [in this case we cannot remove xorg, because we will get a broken dep]. Any idea? ---------------------------------------------------- SZTE Egyetemi Könyvtár - http://www.bibl.u-szeged.hu This mail sent through IMP: http://horde.org/imp/

On Mon, Sep 17, 2007 at 01:26:55PM +0200, Nagy Gabor wrote:

...

OMG, I noticed that current pacman won't resolve target<->target conflict (it stops with unresolveable conflict error <- so that ~150 line conflict-resolution monster is not so efficient in sync.c ;-). However pacman may still remove packages from the target list: sync.c: /* pacman -S blackbox xfree86 ... */.

Actually, I think that's the only reason why your patch breaks a few pactest. In case of target <-> target conflicts, pacman used to fail. But after your patch, it tries to automatically resolve it by selecting one of the target. You said that yourself :) Oh, I remember now, but I thought that those pactests "decide" not to resolve that conflict [Note: I used the word 'autoresolves' here: http://www.archlinux.org/pipermail/pacman-dev/2007-August/009078.html];-) And I never could get myself to read that whole messy part in sync.c, however I saw that pacman _can_ remove packages from the target list.

So you broke the pactests that had a target <-> target conflict, and expected pacman to fail. I am still not sure what's the best default behavior (for example, the behavior using --noconfirm, which is how pactest are run). But in any cases, it's probably better to ask the user (you already said that as well).

...

I don't understand that part at all: resolvedeps first search for (blackbox's) dependency satisfiers in the target list. So it will find xfree86, if that dep is OK [in this case we don't need to remove anything]; if xfree86 dep is not OK, resolvedeps must find and pull a real satisfier (xorg), [in this case we cannot remove xorg, because we will get a broken dep]. Any idea?

See : http://www.archlinux.org/pipermail/pacman-dev/2007-July/009041.html http://www.archlinux.org/pipermail/pacman-dev/2007-August/009155.html

I would be happy if only the comment would be outdated ;-) Hopefully the whole part around that comment never runs (otherwise it breaks some depends), so it can be removed [line 504-554, 577-587(?)]. So the current sync.c is not so dangerous as I thought, however that is very ugly (IMHO): [asked variable is unneeded, sync1003.py still fails] Bye, ngaba ---------------------------------------------------- SZTE Egyetemi Könyvtár - http://www.bibl.u-szeged.hu This mail sent through IMP: http://horde.org/imp/

On 9/18/07, Nagy Gabor wrote:

3. We are sitting on this problem for ~1.5 months, we (mostly I) discussed it in 10+ mails, so it's time to do something in the code too ;-)

Um. See, this is why we always say "patches welcome". If *you* find it important, write a patch please. Apparently this isn't as big of a deal to other people. This is how open source is supposed to work. Ideas are free, code is not. Please send us a patch if you want this done quickly.

TODO: -update bash_completion I never hacked this, so I trust on our bash_completion "devel" instead of a dirty hack ;-) Bye, ngaba ---------------------------------------------------- SZTE Egyetemi Könyvtár - http://www.bibl.u-szeged.hu This mail sent through IMP: http://horde.org/imp/

On Wed, Sep 19, 2007 at 04:10:35PM +0200, Nagy Gabor wrote:

Well, I don't want to hurt you, but my list would be: 1. review/apply/write bugfixes 2. add new features/code clean-up/improvements...

I'm sure that you are busy, but you may add git access to other "trusted" devels (1 vote for Xavier) to help you. I'm not familiar with pacman developing philosophy, but as I see, only you and Aaron are the active devels who have git access.

There is no need for that, and I wouldn't want it anyway. I push my own changes to my git tree, and when they are good/simple enough, they are merged very fast. Maybe even too fast sometimes, I have messed up several things recently : 1) http://projects.archlinux.org/git/?p=pacman.git;a=commit;h=d34b2c4ed84bc40f4... one of the newline fix there is actually a newline breakage 2) http://projects.archlinux.org/git/?p=pacman.git;a=commit;h=26441cf65ca10d4bf... I noticed there was an issue there, but didn't notice it was done twice.. It's a bit stupid to require 2 commits for this, but well :) 3) http://projects.archlinux.org/git/?p=pacman.git&a=search&h=HEAD&st=commit&s=transaction it took me at least 4 commits to get this right (hopefully it is now, I'm not even sure), the last one making all the previous ones obsolete. So you shouldn't trust me, I can't even get my own stuff right, even for simple things :) Your patch are much more complicated, especially the last two big ones : fix for sync 1003 and conflicts resolving cleanup. I've spent hours trying to understand the existing pacman code edited by your patches, and it seems I'm still far from understand it perfectly. I have a basic understanding of it, and how you changed them. It does look good to me, so I pushed them to my tree, with the associated comments : http://chantry.homelinux.org/~xav/gitweb/gitweb.cgi?p=pacman.git;a=shortlog;... About the resolveconflict stuff, I've been willing to rework it a bit, with my suggestions there : http://www.archlinux.org/pipermail/pacman-dev/2007-August/009087.html But I only fixed the two little gcc warnings. I think some refactoring would be nice, but it isn't critical. I still have another question about it, but I'll reply to your previous mail instead.

Well, I'm all for going the easy way 1. :) I think the old behavior is acceptable, and that we can keep it for now, and avoid breaking the few pactests that depend on it. Your patch just needs to be slightly edited for doing that, right? Right. You can simply replace the autoresolve stuff (around TODO) with an error message.

But I must mention that (hopefully) my patch cannot break anything in the local db, even if you let it remove one of the conflicting packages from target list (my TODO says: let user choose); because the final dependency check is done just before the commit. So if it could resolve the conflict, then everything is OK (no dep/conflict error; apart from some orphan dependency install :-(), and if the resolving breaks a dependency, pacman will stop with an error message (which is the default behaviour now in case of target<->target conflict; however, user may get some strange error message) So 2. is already done by patch: It is difficult only if you want to solve some really exotic cases (what I mentioned in 2.); pacman now stops with an error message if it is not smart enough to do that (however, the problem may solvable). Summary: with my patch pacman tries to resolve the target<->target conflicts and it may not be clever enough to do that correctly, then it stops. Bye, ngaba PS: As I wrote this e-mail, the "assymmetric" conflict storing came into my mind again: http://www.archlinux.org/pipermail/pacman-dev/2007-August/009091.html ---------------------------------------------------- SZTE Egyetemi Könyvtár - http://www.bibl.u-szeged.hu This mail sent through IMP: http://horde.org/imp/