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.