On Wed, Oct 03, 2007 at 07:52:44PM +0200, Nagy Gabor wrote:

-Sp simply seems to be broken. If I do pacman -Sp evince, I get the following error: 'error: evince': transaction not initialized error: failed to release transaction (transaction not initialized)

See http://bugs.archlinux.org/task/6812 git is currently broken because of this commit : http://projects.archlinux.org/git/?p=pacman.git;a=commit;h=b86479cf9b584c62d... However, after reverting this commit, this command still needs to be run as root for working. That wasn't the case with pacman 3.0 ..

On Thu, Oct 04, 2007 at 01:18:11PM +0200, Nagy Gabor wrote:

Well, I'm pretty sure that we should restructure pacman's option "handling". Sometimes "rethinking" a big codepart is easier than fixing ~100 flyspray bugs (as an example, I must mention the current libalpm/sync.c ...) See also (this method is the most pausible for me; the effects of this would restructure the source, too): http://www.archlinux.org/pipermail/pacman-dev/2007-September/009484.html

Unfortunately, no one is going to do it. I'm not sure I could do it (I suck, I have no experience in C, no experience in pacman, and understanding some part of the pacman code takes me ages). And in any cases, I'm afraid I don't have the motivation. I find the amount of TODO things really depressing :) And Aaron and Dan already have already plenty of things to do in pacman. For example, preparing for 3.1 release, handling all the bug reports (I don't think most of them are related to libalpm/sync,c code), and also plenty of others non-pacman arch-related things.

I don't really understand where the difficulty is. We can loop in the operation options, and do the requested operations. Permission handling: Do we really need this? libalpm will return with an error, if the user hasn't got enough privileges; and pacman can quit (any may ignore -y). libalpm could also inform the caller funtion about "Operation not permitted" with an error-type.

That's probably a good idea, but is it really that easy? What happens if you have enough privileges to do some stuff, but not enough for completing the transaction, and pacman fails in the middle of it? But apparently, ideas are not welcome here anyway. Since Aaron is familiar with the code base, he already knows about many of them, so this doesn't help him. Patches would help more. And Dan apparently finds that your descriptions / explanations of problems are hard to understand, and your patches hard to read.. So that most likely explains why some of them take so long to be merged.. In my opinion, git helps again here. It allows you to look back at your patch, and try improving their readability. And also, you have the time to work on the description of the commit, which explains the patch, and make it as clear as possible. However, I find that your ideas and patches are always right on spot, that you fixed many bugs, and improved some parts of the code. So I think you are a valuable contributor. I hope you could try to listen to Aaron and Dan advices. That is, don't submit so many ideas unless you are ready to work on them :) And when you do provide a patch, try to make it as clear as possible. Just place all chances on your side, so that your patch can be merged.

Unfortunately, no one is going to do it. I'm not sure I could do it (I suck, I have no experience in C, no experience in pacman, and understanding some part of the pacman code takes me ages). You are too humble again ;-)

...

I don't really understand where the difficulty is. We can loop in the operation options, and do the requested operations. Permission handling: Do we really need this? libalpm will return with an error, if the user hasn't got enough privileges; and pacman can quit (any may ignore -y). libalpm could also inform the caller funtion about "Operation not permitted" with an error-type.

That's probably a good idea, but is it really that easy? What happens if you have enough privileges to do some stuff, but not enough for completing the transaction, and pacman fails in the middle of it? First of all, pacman should be prepared to even a readdir() failure. Secondly, if the user doesn't have root rights, he cannot touch the local db, so he cannot do anything dangerous (in the worst case he may get a segfault...). You may say, that user can set a+w attributes for some parts of the db, and root-only attributes for some other parts, and this can lead to problems; but I say, that pacman needn't be idiot-proof ;-)

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

On Fri, Oct 05, 2007 at 04:37:54AM +0200, Miklos Vajna wrote:

On Thu, Oct 04, 2007 at 01:18:11PM +0200, Nagy Gabor <ngaba@bibl.u-szeged.hu> wrote:

...

pacman can quit (any may ignore -y). libalpm could also inform the caller funtion about "Operation not permitted" with an error-type.

this is what PM_ERR_BADPERMS is about. this has been disabled in pacman.git's c22e381a (maybe earlier)

(in pacman-g2 if the frontend does not have any such checks, then the commit of the transaction fails with this error code)

Rather earlier : cdb46ef3 (funny enough, the realpath stuff I mentioned in another thread was introduced there too) I don't know what this ACCESS stuff was about, but that's not at all how I understood Nagy's comment. See his previous comment : http://www.archlinux.org/pipermail/pacman-dev/2007-October/009521.html What he's suggesting is that pacman returns a PM_ERR_BADPERMS on every read / write error. Not just a simple root check, like this ACCESS stuff appears to be. Besides, this basic root check is already done in the frontend, so what's the point of having it in the backend as well? Ideally, pacman should have a real transaction model. That is, it's not possible to do a half transaction. So either, it should be able to detect that everything will go fine before starting. For having an idea about how easy that is, see for example the freespace check Dan had to remove : ed13ac2c Or the remove check that was introduced because of bug 5887 with commit a94e22d9, and which is totally unreliable : * bug 7652 * http://www.archlinux.org/pipermail/pacman-dev/2007-August/009150.html The second possibility I see would be the ability to rollback, but that might be even harder. As Nagy mentioned, in the common case, that ability to roolback might not be needed. If pacman fails on the first read/write error, it'll probably be early enough in most cases.

On Sun, Nov 18, 2007 at 11:47:49PM -0600, Dan McGee wrote:

On Oct 3, 2007 1:03 PM, Xavier <shiningxc@gmail.com> wrote:

...

On Wed, Oct 03, 2007 at 07:52:44PM +0200, Nagy Gabor wrote:

...

-Sp simply seems to be broken. If I do pacman -Sp evince, I get the following error: 'error: evince': transaction not initialized error: failed to release transaction (transaction not initialized)

See http://bugs.archlinux.org/task/6812

git is currently broken because of this commit : http://projects.archlinux.org/git/?p=pacman.git;a=commit;h=b86479cf9b584c62d...

However, after reverting this commit, this command still needs to be run as root for working. That wasn't the case with pacman 3.0 ..

So who wants to tackle this one? Hopefully we can get a real solution to this crap in 3.2, but we may have to hack something out here for 3.1.

It took me a while to understand this problem again, though I already explained it in this old thread : "Oh... I just figured out why this happens. In pacman 3.0, the lock was in /tmp/pacman.lck , while it's now in /var/lib/pacman/db.lck. So previously, a normal user could lock. And not anymore.. " So, I don't know, should we explictly skip locking the db in alpm_trans_init in case of PRINTURIS ? That sounds really ugly. But if you do that, besides putting the PRINTURIS check back in needs_transaction , src/pacman/util.c , then it might work.

On Mon, Nov 19, 2007 at 06:45:52AM -0600, Dan McGee wrote:

The best solution would be to factor print URIs operation out of needing a transaction at all. This may be helped by me actually merging your upgrade patch first.

That patch alone doesn't help much. But if the idea is alright, maybe it could be extended to sync_prepare, after find_replacements and sync_sysupgrade : skip the QUESTIONs when the trans argument is NULL. The questions in sync_prepare are LOCAL_UPTODATE and CONFLICT_PKG. If I'm not mistaken, the first one will never be skipped in case of -Su anyway, and the second one shouldn't come often. So that would not be a problem. But I'm looking at it now, and sync_prepare looks much more dependent on the trans argument than the others. It also uses trans->packages, trans->targets, trans->flags, EVENT(trans,...). So that doesn't seem easy. Using a null trans argument looks quite hackish anyway.. If the current code really doesn't allow to implement features like -Qu and -Sup correctly, then I think these options should be removed, and the problematic code rewritten, so that they can be added properly later.