[pacman-dev] Frugalware changes merged.

[Aaron Griffin]

On 10/19/06, VMiklos <vmiklos at frugalware.org> wrote:
> 1) po4a detection is added 3 times to configure.ac

Fixed.  I've noticed one or two other instances of triple-insertions...

> 2) you forgot to cvs add the bindings directory, while you added the
> bindings part to configure.ac

Bindings are here:
http://cvs.archlinux.org/cgi-bin/viewcvs.cgi/bindings/?cvsroot=Pacman
What do you mean?

> 3) po4a support as been added to doc/Makefile.am, but the translations
> in /doc/po/ are missing

Not sure what you mean - the only translations I have from the patch
are hungarian.

> 4) you left several NoUpgrade files in pacman.conf while that directive
> should not be used by default (it is something for users)

According to CVS, NoUpgrade was never removed. This is the latest:
http://cvs.archlinux.org/cgi-bin/viewcvs.cgi/etc/Attic/pacman.conf?rev=HEAD&search=None&hideattic=1&cvsroot=Pacman&only_with_tag=HEAD&content-type=text/vnd.viewcvs-markup

> 5) libalpm/add.c: search for "Cleaning up", added 2 times

Fixed in CVS.

> 6) you've added a new callback parameter to alpm_db_register() which is
> totally useless imho. the callback is called with the treename (which is
> a parameter, too) and the database pointer, which is returned. so what's
> the point of it?

The point is that I changed this function to return the existing DB in
an attempt to reregister, in place of returning null.  It seems stupid
to let this fail:

[current]
Server = a
[current]
Server = b

Every config file parser I've seen for sectioned configs ([section
name]) parses this as if they were all one section.  As such, the
change requires the callback because only db_register knows when the
database is new or old.  It's a rather trivial change, and not
"totally useless", as it allows for more valid config file handling.

> 7) in alpm.c:
>
> - * @return a void* on success (the value), NULL on error
> + * @return a char* on success (the value), NULL on error
>   */
>  void *alpm_conflict_getinfo(pmconflict_t *conflict, unsigned char parm)
>
> is this a typo? :)

I didn't touch those comments... fixed in CVS though

> 8) i think you've reverted judd's following change:
> 2006-07-04 19:48 judd
>
> * lib/libalpm/deps.c: bugfix: when looking at provides, defer to
> the new, to-be-installed package's provisios instead of the the
> existing package's

Ah crap... didn't notice that one, I assumed your patches took this
into account.  I will fix this later, as it's a shade more complicated
than editing a few lines.

> 9) what is the benefit of this change in Makefile.am?
>
> -       cd pactest; python pactest.py --test=tests/*.py -p        ../src/pacman/pacman --debug=-1
> +       cd pactest; python pactest.py --test=tests/*.py -p        ../src/pacman/pacman --debug=1

That was a mistake on my part that shouldn't have been checked in.  I
was not getting debug output, so messed with the param.... later I
found the 2>&1 > logfile redirection and therefore added the nolog
parameter.
Fixed in CVS.

> 10) i think you forgot to cvs add the /pactest/tests/ directory
They show up for me:
http://cvs.archlinux.org/cgi-bin/viewcvs.cgi/pactest/tests/?search=None&hideattic=1&cvsroot=Pacman&only_with_tag=HEAD

> 11) i think you forgot to add the relevant copyright lines to
> scripts/makepkg

Ah, I didn't change mackepkg so didn't check that - fixed in CVS

>
> 12) you haven't removed src/pacman/db.[ch] while noone uses the
> functions provided by them

Done.


Thanks alot.