VMiklos wrote:

hi all

just wanted to suggest let's make a list of bugs that blocks us to release a public rc (and then the first stable) release of pacman3

we have several ideas, for example replacing the ugly chroot system() call, adding gettext support, etc, but these features were not in pacman2, and i think they're not "musthave"

so what are the points from TODO that must be solved before a release?

The ORE tags from the file lib/libalpm/sync.c are placeholders for missing chunks of code. The commentaries associated with the tags were copied/pasted from commentaries in the pacman.c file from pacman 2.9.7 to remind me to implement the missing sections. Implementing code for these parts may not be obvious and may need to be discussed. This is the most important task to achieve before pacman 3.0 can be released.

i think the only one is the lack of handling of package conflicts during packages replacement, and maybe the manpages of public funtions (via doxygen)

On the library side, I really would like to have error messages and log callbacks be reviewed. pm_errno usage is missing in some parts of the code. manpages are needed too. Upon release, the library must be usable (documented, and errors returned to the frontend must be really clean). On pacman side, outputs should be checked too. The ORE tag in src/pacman/log.c must be fixed. Last, but not least, more tests are needed, especially for backward compatibility between pacman 2.9.x and the library. For instance, I'm trying usually to install/removing the same set of packages inside two different subdirectories. For the first directory, I'm using pacman 2.9, and for the second, I'm using the library. Then I'm diffing both. But, my last test campaign is quite old... And I poorly tested the "sync" part, especially since the code is not complete. -- Aurelien

On Sun, Oct 30, 2005 at 10:39:41PM +0100, VMiklos wrote:

On Sun, Oct 30, 2005 at 06:46:14PM +0100, Aurelien Foret wrote:

...

On the library side, I really would like to have error messages and log callbacks be reviewed. pm_errno usage is missing in some parts of the code. manpages are needed too.

ok, as a first step, here is a patch to add a Doxyfile for libalpm

http://frugalware.org/~vmiklos/patches/libpacman-proposed/pacman-lib-doxyfil...

any comments? udv / greetings, VMiklos -- Developer of Frugalware Linux, to make things frugal - http://frugalware.org

On Mon, Nov 14, 2005 at 11:39:46AM +0100, VMiklos wrote:

On Sat, Nov 05, 2005 at 10:34:48AM +0100, VMiklos wrote:

...

...

ok, as a first step, here is a patch to add a Doxyfile for libalpm

http://frugalware.org/~vmiklos/patches/libpacman-proposed/pacman-lib-doxyfil...

any comments?

still not comments?

just for who does not co the cvs regulary: applied by judd udv / greetings, VMiklos -- Developer of Frugalware Linux, to make things frugal - http://frugalware.org

On Sun, Oct 30, 2005 at 06:46:14PM +0100, Aurelien Foret wrote:

The ORE tags from the file lib/libalpm/sync.c are placeholders for missing chunks of code. The commentaries associated with the tags were copied/pasted from commentaries in the pacman.c file from pacman 2.9.7 to remind me to implement the missing sections. Implementing code for these parts may not be obvious and may need to be discussed. This is the most important task to achieve before pacman 3.0 can be released.

the question is if you plan to implement these or we should figure out the situation (probably it'll take some time for us) and try to do this? udv / greetings, VMiklos -- Developer of Frugalware Linux, to make things frugal - http://frugalware.org

VMiklos wrote:

On Sun, Oct 30, 2005 at 06:46:14PM +0100, Aurelien Foret wrote:

...

The ORE tags from the file lib/libalpm/sync.c are placeholders for missing chunks of code. The commentaries associated with the tags were copied/pasted from commentaries in the pacman.c file from pacman 2.9.7 to remind me to implement the missing sections. Implementing code for these parts may not be obvious and may need to be discussed. This is the most important task to achieve before pacman 3.0 can be released.

the question is if you plan to implement these or we should figure out the situation (probably it'll take some time for us) and try to do this?

udv / greetings, VMiklos

I've already implemented sporadic parts of it, but it is only a first try, and I'm not ready to commit or share it until I'm sure it is possible to get it working right. It is not really a simple task, and eventually, the transaction structure need to be extended to support sync operations: sync_commit() must be able to deal with both a list of packages to remove and to install in the same operation. To sum-up, there's nothing frozen for now, so I'm still interested to hear ideas or suggestions on this topic, or to get some patches too! -- Aurelien

On Mon, Nov 28, 2005 at 08:20:36PM +0100, Aurelien Foret wrote:

It is not really a simple task, and eventually, the transaction structure need to be extended to support sync operations: sync_commit() must be able to deal with both a list of packages to remove and to install in the same operation.

To sum-up, there's nothing frozen for now, so I'm still interested to hear ideas or suggestions on this topic, or to get some patches too!

http://frugalware.org/~vmiklos/patches/libpacman-proposed/conflict-remove.di... here is a possible implementation $ sudo pacman -S mtr-nc --noconfirm resolving dependencies... done. looking for inter-conflicts... done. Remove: mtr Targets: mtr-nc-0.69-2 Total Package Size: 0.1 MB Beginning upgrade process... checking package integrity... done. removing mtr... done. (1/1) installing mtr-nc [################] 100% udv / greetings, VMiklos -- Developer of Frugalware Linux, to make things frugal - http://frugalware.org

On Sun, Oct 30, 2005 at 06:46:14PM +0100, Aurelien Foret wrote:

The ORE tags from the file lib/libalpm/sync.c are placeholders for missing chunks of code. The commentaries associated with the tags were copied/pasted from commentaries in the pacman.c file from pacman 2.9.7 to remind me to implement the missing sections. Implementing code for these parts may not be obvious and may need to be discussed. This is the most important task to achieve before pacman 3.0 can be released.

imho this is now done

On the library side, I really would like to have error messages and log callbacks be reviewed. pm_errno usage is missing in some parts of the code. manpages are needed too.

cmiiw, but these are solved, too

Upon release, the library must be usable (documented, and errors returned to the frontend must be really clean).

same

On pacman side, outputs should be checked too. The ORE tag in src/pacman/log.c must be fixed.

a patch to fix that ore tag is just sent

Last, but not least, more tests are needed, especially for backward compatibility between pacman 2.9.x and the library.

this is something you can decide, i mean what tests fail so it would be nice if we could make a new todo about what must be done before the first 3.x release udv / greetings, VMiklos -- Developer of Frugalware Linux, to make things frugal - http://frugalware.org

On Sat, Feb 04, 2006 at 11:31:49AM +0100, Aurelien Foret wrote:

[1] bug in the conflict resolution in sync_prepare:

Example: :: Starting local database upgrade... ... :: avahi conflicts with nss-mdns. Remove nss-mdns? [Y/n] ... Remove: nss-mdns

Targets: avahi-0.6.6-2 nss-mdns-0.7-4

it seems that the "rmtargs" list was dropped from the implementation during the merge with pacman 2.9.x code...

interesting, i can't reproduce this: $ sudo pacman -S libnfsidmap resolving dependencies... done. looking for inter-conflicts... :: libnfsidmap conflicts with nfsidmap. Remove nfsidmap? [Y/n] done. Remove: nfsidmap Targets: libnfsidmap-0.12-1 i even tried the official cvs, but that works fine, too could you put out the buildscript of these packages to clarify the problem? (maybe it depends on if the packages is also provides the other, or something)

[3] cvs code synchronization with pacman 2.9.8:

is it already done? or is there something missing?

judd can confirm this, but what i saw the manpages were outdated, i sent a patch for this, but judd did it himself. so probably this is done

[5] database cleanup

- do not write empty entries in database files with db_write, (same thing for gensync? makepkg for .PKGINFO?) - do not add name and version entries in "desc" file (already available in the directory name), also for .PKGINFO file?

is this important for now? this would prevent people to downgrade to pacman2 if they want + my [9]: review file conflict code: what i posted already here, imho the state "dirs can't become files (symlinks) during an upgrade (it's a conflict)" is too strict, we should allow it udv / greetings, VMiklos -- Developer of Frugalware Linux, to make things frugal - http://frugalware.org

On Mon, Feb 06, 2006 at 11:18:08PM +0100, Aurelien Foret wrote:

[7] remove exit() calls from libalpm the library should never call exit() but always return an error code to the frontend (for instance, drop MALLOC macro)

hmm, is it really necessary to drop that macro? what about this patch? http://darcs.frugalware.org/darcsweb/darcsweb.cgi?r=pacman;a=plain_commitdif...; udv / greetings, VMiklos -- Developer of Frugalware Linux, to make things frugal - http://frugalware.org

VMiklos wrote:

...

[11] library symbols naming prepends private functions with "_alpm_"

=> it should be done as the last step before a first release to avoid introducing too much changes to the CVS code at once.

hmm. you mean _every_ function that is not declared in alpm.h?

Excepted static ones, yes. The library and the frontend are sharing the same namespace, so that symbol conflicts can arise. Pre-pending function names with a common prefix ("_alpm_" or just "_") helps to avoid such conflicts. This is all but mandatory, but it is usually a good practice. I've already done it to some extent in the past (see list.h or util.h) for the most common function names which are more likely to be defined in the frontend. It makes sense to me to extend it to all internal functions. -- Aurelien

On Tue, Feb 07, 2006 at 08:05:21PM +0100, Aurelien Foret wrote:

VMiklos wrote:

...

On Mon, Feb 06, 2006 at 11:18:08PM +0100, Aurelien Foret wrote:

...

[7] remove exit() calls from libalpm the library should never call exit() but always return an error code to the frontend (for instance, drop MALLOC macro)

hmm, is it really necessary to drop that macro? what about this patch? http://darcs.frugalware.org/darcsweb/darcsweb.cgi?r=pacman;a=plain_commitdif...;

I don't think it would fit: it drops all possibilities to take needed actions to cleanup the environment (freeing locally allocated memory, ...) before leaving a function.

ok, here is an updated patch: http://frugalware.org/~vmiklos/patches/libpacman-proposed/drop_malloc.diff i haven't added any extra FREE() for other allocated memory, if the patch is ok, then i can add udv / greetings, VMiklos -- Developer of Frugalware Linux, to make things frugal - http://frugalware.org

On Wed, Feb 08, 2006 at 02:07:59PM +0100, VMiklos wrote:

...

...

hmm. you mean _every_ function that is not declared in alpm.h?

Excepted static ones, yes.

ok, then i take this

http://frugalware.org/~vmiklos/patches/libpacman-proposed/_alpm.diff udv / greetings, VMiklos -- Developer of Frugalware Linux, to make things frugal - http://frugalware.org

On Wed, Feb 08, 2006 at 03:11:36PM +0100, VMiklos wrote:

On Wed, Feb 08, 2006 at 02:07:59PM +0100, VMiklos wrote:

...

...

...

hmm. you mean _every_ function that is not declared in alpm.h?

Excepted static ones, yes.

ok, then i take this

http://frugalware.org/~vmiklos/patches/libpacman-proposed/_alpm.diff

is there any reason why this was not commented? 1) as reviewing the patches takes sometimes weeks, we had to create our own tree 2) so every patch has to be backported manually 3) this patch is rather trivial 4) it implements Aurel's idea (not a Frugalware one) 5) you know that the patch was affected almost everything, and yes, it no longer applies sorry, but this is a bit annoying. i really don't want to port it again to your cvs possible sollutions i see: - speeding up reviewing the patches a bit (apply/reject them in a week) - giving cvs write access comments? udv / greetings, VMiklos -- Developer of Frugalware Linux, to make things frugal - http://frugalware.org

On Fri, Feb 17, 2006 at 09:10:28AM +0100, Aurelien Foret wrote:

6) if you had read my TODO completely, you would have notice that I suggested this change to be the last one to be done before a release. But you chose to do it and send it anyway...

okay, then just forgot that patch

Should I feel sorry because you're working on a modified version of pacman sources, and that you have to backport your changes to the official sources in order send us patches, or that you have to merge our cvs code with yours? To be honest, I don't think so. You're maintaining a parallel version of pacman, and I'm not responsible for this choice.

of course that's our choice, that's not a problem. my problem is that submitting the patches here requires some work. you know, a one-liner sometimes a result of a half day of headache. then i submit here a patch, and i even don't get a "rejected" answer after a week

By the way, the fact that a patch does not apply cleanly is absolutely not an issue.

good to know :)

For your information, I already applied it to my local code sources, although I didn't committed it yet. I'm still reviewing it (especially the need to rename symbols for db_XXX functions).

thanks for the info

I don't see the need for it. If it takes more than a week to review and commit, what's the deal?

nothing just i can't write down everything about the patch in the mail and then if you ask something, i won't know why i did something. look at my "chroot - change working dir" patch: probably you haven't answered because it takes time to prepare again the testsuite for verifying what i wrote the other problem with the slow review is that you reguralry find the same bug what one of the patches fix, then you fix that, and then a) just don't reply to my mail b) ask here why this fix is needed, it's already been fixed. yes. two hours ago. and the patch was sent two _weeks_ ago

So, there's no reason to put us under pressure.

ok, as you want. just i see less and less motivation to submit patches here, but that's your choice udv / greetings, VMiklos -- Developer of Frugalware Linux, to make things frugal - http://frugalware.org

VMiklos wrote:

On Sat, Feb 11, 2006 at 07:44:00PM +0100, Aurelien Foret wrote:

...

For instance, you chose to set pm_errno in db_open and pkg_new, but the pm_errno value can be overwritten. See code chunks where pkg_new is called: in case of a failure, pm_errno is set afterwards, meaning that the pm_errno value set in pkg_new is lost.

i think this is the right way, the "failed to prepare the transaction" error message is more infomative as the "memory error" one for a frontend. or don't you think so?

Agreed. Patch applied.

...

It may or may not be more interesting to set pm_errno in the offending function, or upon return of the function. It depends on what is more meaningful from the frontend point of view...

i would say set pm_errno to PM_ERR_MEMORY, and if it's overwritten, then it's not a problem, any error message other than PM_ERR_MEMORY is more informative to the fronend. that's what my patch does.

To go even further, we should maybe add a LOG_ERROR logs mentioning the issue? That would give some clues even it pm_errno got overwritten... -- Aurelien

VMiklos wrote:

On Mon, Feb 20, 2006 at 09:42:45PM +0100, Aurelien Foret wrote:

...

To go even further, we should maybe add a LOG_ERROR logs mentioning the issue? That would give some clues even it pm_errno got overwritten...

maybe adding debug messages also could do the task? that sounds a bit more simpler to me

That's why I meant, but with the ERROR mask rather than the DEBUG one. -- Aurelien

VMiklos wrote:

should i make a patch to implement this? (before we get to a situation like with the symbol renaming ;) )

If you're inclined to, then yes :) -- Aurelien

VMiklos wrote:

http://frugalware.org/~vmiklos/patches/libpacman-proposed/malloc_error.diff

Applied. -- Aurelien