[pacman-dev] [PATCH] Complete rework of package accessor logic

[Xavier]

On Mon, May 12, 2008 at 3:28 AM, Dan McGee <dan at archlinux.org> wrote:
>
>  So what does this commit change? It adds a pointer to a struct of function
>  pointers to every package for all of these 'package operations'  as I've
>  decided to call them (I know, sounds completely straightforward, right?). So
>  now when we call an alpm_pkg_get-* function, we don't do any of the cache
>  logic or anything else there- we let the actual backend handle it by
>  delegating all work to the method at pkg->ops->get_depends.
>
>  Now that be_package has achieved equal status with be_files, we can treat
>  packages from these completely different load points differently. We know a
>  package loaded from a pkg.tar.gz will have all of its fields populated, so
>  we can set up all its accessor functions to be direct accessors. On the
>  other hand, the packages loaded from the local and sync DBs are not always
>  fully-loaded, so their accessor functions are routed through the same logic
>  as before.
>


So functions that are tied down to a specific database shouldn't be in
package.c at all, right?
They should just be put in the corresponding be_* files?
But I will go on below with the checkmd5sum() example.


>  Net result? More code. However, this code now make it roughly 52 times
>  easier to open the door to something like a read-only tar.gz database
>  backend.
>


That's indeed quite a lot of code, but it's pretty straightforward,
and it indeed seems like it would be easier to implement another
backend.


>
>  +/* Default package accessor functions. These will get overridden by any
>  + * backend logic that needs lazy access, such as the local database through
>  + * a lazy-laod cache. However, the defaults will work just fine for fully-
>  + * populated package structures. */


you mean load? :)


>
>  +/** Package operations struct. This struct contains function pointers to
>  + * all methods used to access data in a package to allow for things such
>  + * as lazy package intialization (such as used by the file backend). Each
>  + * backend is free to define a stuct containing pointers to a specific
>  + * implementation of these methods. Some backends may find using the
>  + * defined default_pkg_ops struct to work just fine for their needs.
>  + */
>  +struct pkg_operations {
>  +       const char *(*get_filename) (pmpkg_t *);
>  +       const char *(*get_name) (pmpkg_t *);
>  +       const char *(*get_version) (pmpkg_t *);
>  +       const char *(*get_desc) (pmpkg_t *);
>  +       const char *(*get_url) (pmpkg_t *);
>  +       time_t (*get_builddate) (pmpkg_t *);
>  +       time_t (*get_installdate) (pmpkg_t *);
>  +       const char *(*get_packager) (pmpkg_t *);
>  +       const char *(*get_md5sum) (pmpkg_t *);
>  +       const char *(*get_arch) (pmpkg_t *);
>  +       unsigned long (*get_size) (pmpkg_t *);
>  +       unsigned long (*get_isize) (pmpkg_t *);
>  +       pmpkgreason_t (*get_reason) (pmpkg_t *);
>  +
>  +       alpm_list_t *(*get_licenses) (pmpkg_t *);
>  +       alpm_list_t *(*get_groups) (pmpkg_t *);
>  +       alpm_list_t *(*get_depends) (pmpkg_t *);
>  +       alpm_list_t *(*get_optdepends) (pmpkg_t *);
>  +       alpm_list_t *(*get_conflicts) (pmpkg_t *);
>  +       alpm_list_t *(*get_provides) (pmpkg_t *);
>  +       alpm_list_t *(*get_replaces) (pmpkg_t *);
>  +       alpm_list_t *(*get_deltas) (pmpkg_t *);
>  +       alpm_list_t *(*get_files) (pmpkg_t *);
>  +       alpm_list_t *(*get_backup) (pmpkg_t *);
>  +
>  +       void *(*changelog_open) (pmpkg_t *);
>  +       size_t (*changelog_read) (void *, size_t, const pmpkg_t *, const void *);
>  +       int (*changelog_close) (const pmpkg_t *, void *);
>  +
>  +       /* still to add:
>  +        * free()
>  +        * dup()
>  +        * checkmd5sum() ?
>  +        * has_scriptlet()
>  +        * compute_requiredby()
>  +        */
>  +};
>  +

Well checkmd5sum is tied down to cache db, so it belongs neither in
package.c, nor in pkg_operations, right?
Also let me remind you that this function isn't used anywhere :) But
you and Aaron wanted to keep it because it might be useful for
frontends.

About has_scriptlet, how do we implement that for sync db? Or well,
maybe it could just return false.. Because it seems like all these
package ops can't fail anymore.

And what is the problem with compute_requiredby, it seems to be
independent from the backends, it just uses normal accessors. Actually
that function might fit better in deps.c, with all others dep
computing things.