[pacman-dev] [patch] testdb

[Nagy Gabor]

> On Mon, Oct 08, 2007 at 04:37:50PM +0200, Nagy Gabor wrote:
> > this is also done; but TODO: I did a dirty hack to make check_conflict
> visible
> > from testdb.c (I'm not familiar in these static/symexport... stuffs,
> > check_conflict probably should be renamed, too)
> 
> I don't know really how the interfaces for the conflict checks should be.
> Currently we have these 2 functions :
> * void check_conflict(alpm_list_t *list1, alpm_list_t *list2,
>     alpm_list_t **baddeps, int order)
> 
> * alpm_list_t *_alpm_checkconflicts(pmdb_t *db, alpm_list_t *packages)
>   built on top of check_conflict.
> 
> And _alpm_checkconflicts is used in sync.c and add.c .
> Though, after your resolve conflict patch (not merged), _alpm_checkconflicts
> is no longer used in sync.c . Instead, check_conflict is used directly.
> And now, for testdb too, check_conflict has to be used directly.
> 
> So it appears that the _alpm_checkconflicts function isn't really useful.
> However, the interface of check_conflict is too confusing for a public
> function imo.
> I don't know what should be done really, this requires some thinking.

I'm on the same opinion; this is an other reason for the dirty hack.
But you can see that direct using of check_conflict is reasonable in many cases,
so I suggest you to provide both a beginner public funtion (alpm_checkconflicts)
and an advanced one (check_conflict). Or you can also choose does_conflict
instead of check_conflict as an atomic conflict-compare function (analogue of
alpm_depcmp), but this may lead to many duplicated codes (like alpm_depcmp ;-)

> And anyway, you said these conflict checks sucked because of the
> assymetrical
> storing (I don't know how that should be handled either..).

IMHO the most cleaner would be a new pmconflict_t type, because using
pmdepmissing_t confuses me often; and we can treat the two conflicting packages
as sets (unordered pairs).
An other possible solution:
check_conflict (or add_conflict) should do the asymmetric check at least in a
special order == case [before adding a (p,q) conflict pair it should check if
(p,q) or (q,p) has been already added to the list.

> About static/symexport stuff, first a part of the README file :
> 
>  27 In a general manner, public library functions are named
> "alpm_<type>_<action>"
>  28 (examples: alpm_trans_commit(), alpm_release(), alpm_pkg_getinfo(),
> ...).
>  29 Internal (and thus private) functions should be named "_alpm_XXX" for
> instance
>  30 (examples: _alpm_needbackup(), _alpm_runscriplet(), ...). Functions
> defined and
>  31 used inside a single file should be defined as "static".
> 
> So, if I undestood correctly, you could have :
> 1) void SYMEXPORT alpm_conflict_check defined in conflict.c and declared in
> alpm.h (public)
> 2) void _alpm_conflict_check defined in conflict.c and declared in conflict.h
> (internal)
> 3) static void conflict_check defined in conflict.c (only used in
> conflict.c)
> 
> And the three could exist in the same time (1 and 2 in the same time is very
> common in libalpm).
Thx for this useful info.

Bye, ngaba

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