[pacman-dev] [PATCH 00/13] global handle removal patch series

[Rémy Oudompheng]

On Tue 07 June 2011 at 16:36 -0500, Dan McGee wrote:
> This is a bitch of a diffstat, unfortunately, and the patches aren't all that
> fun to look through. I have it pushed to my repo as alpm-cleanups if you'd
> rather grab it there. The bright side is it should successfully build and test
> after each patch as long as they are applied in the order sent here (or in my
> repo).
> 
> Comments/suggestion/feedback welcome. Once these are applied, I think our API
> makes a lot more sense from a consumer standpoint as we have a single object,
> tracked by the "client", tracking all state in the library rather than the
> state being held in a global variable in the backend library.
> 
> This patch series (and the previous one sent and now applied) makes it *much*
> easier for the next few patches, which will ensure DBs are signature-checked
> and verified at the right time, and at a time all clients expect them to be
> checked so error codes can be checked.

It seems a bit odd to me that packages acquired a handle member in the
process: databases are intrinsically stateful, or at least related to a
state, since they have paths, locks, and are concerned by transactions.

But packages are twofold:
- sometimes they come from a database: they at least inherit a global
  state through alpm_pkg_get_db(pkg)->handle.
- sometimes they come from a file: then things are very different: the
  user must free the structure when finished with alpm_pkg_free(),
  and the package itself is much more pure: it doesn't have a related
  DBPath, no associated root, no options, just a file path.

I support the idea that if a package is loaded from a file, there is not
really a reason why it should be linked to any handle. The downside of
that is that catching error codes would need some modifications in
related functions, but it doesn't look like package functions throw many
error codes.

Another reason why it bothers me is that all transaction functions need a
handle: alpm_trans*, alpm_sync_sysupgrade, but not alpm_add_pkg nor
alpm_remove_pkg.

One may argue that having a handle argument would be redundant with the
pkg->handle thing, but that's a case where it seems really logical to
me, and more consistent with the rest of the API, to declare explicitly
the handle with holds the transaction where a package is added/removed.
The signature alpm_add_pkg(pmpkg_t *pkg) would require some introspection
to detect where the package will be installed.

And actually, it really seems like it is *impossible* to know where
alpm_add_pkg() will install the package, since the current API does not
provide the necessary alpm_pkg_get_handle() to do so (but we of course
could provide such a function: do we need it?)

-- 
Rémy.