[pacman-dev] [PATCH] Complete rework of package accessor logic
Hopefully we've finally arrived at package handling nirvana, or at least this commit will get us a hell of a lot closer. The former method of getting the depends list for a package was the following: 1. call alpm_pkg_get_depends() 2. this method would check if the package came from the cache 3. if so, ensure our cache level is correct, otherwise call db_load 4. finally return the depends list Why did this suck? Because getting the depends list from the package shouldn't care about whether the package was loaded from a file, from the 'package cache', or some other system which we can't even use because the damn thing is so complicated. It should just return the depends list. 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. 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. Are you still reading? I'm impressed. Looking at the patch will probably be clearer than this long-winded explanation. Signed-off-by: Dan McGee <dan@archlinux.org> --- lib/libalpm/be_files.c | 383 +++++++++++++++++++++++++++++++++++++++++++++- lib/libalpm/be_package.c | 2 + lib/libalpm/package.c | 327 +++++++++++----------------------------- lib/libalpm/package.h | 61 +++++++- 4 files changed, 527 insertions(+), 246 deletions(-) diff --git a/lib/libalpm/be_files.c b/lib/libalpm/be_files.c index b0f597d..c9e708d 100644 --- a/lib/libalpm/be_files.c +++ b/lib/libalpm/be_files.c @@ -1,8 +1,7 @@ /* * be_files.c * - * Copyright (c) 2006 by Christian Hamar <krics@linuxforum.hu> - * Copyright (c) 2006 by Miklos Vajna <vmiklos@frugalware.org> + * Copyright (c) 2002-2008 by Judd Vinet <jvinet@zeroflux.org> * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -46,6 +45,384 @@ #include "deps.h" #include "dload.h" +/* Cache-specific accessor functions. These implementations allow for lazy + * loading by the files backend when a data member is actually needed + * rather than loading all pieces of information when the package is first + * initialized. + */ +const char *_cache_get_filename(pmpkg_t *pkg) +{ + ALPM_LOG_FUNC; + + /* Sanity checks */ + ASSERT(handle != NULL, return(NULL)); + ASSERT(pkg != NULL, return(NULL)); + + if(!(pkg->infolevel & INFRQ_DESC)) { + _alpm_db_read(pkg->origin_data.db, pkg, INFRQ_DESC); + } + + return pkg->filename; +} + +const char *_cache_get_name(pmpkg_t *pkg) +{ + /* Sanity checks */ + ASSERT(pkg != NULL, return(NULL)); + + return pkg->name; +} + +static const char *_cache_get_version(pmpkg_t *pkg) +{ + /* Sanity checks */ + ASSERT(pkg != NULL, return(NULL)); + + return pkg->version; +} + +static const char *_cache_get_desc(pmpkg_t *pkg) +{ + ALPM_LOG_FUNC; + + /* Sanity checks */ + ASSERT(handle != NULL, return(NULL)); + ASSERT(pkg != NULL, return(NULL)); + + if(!(pkg->infolevel & INFRQ_DESC)) { + _alpm_db_read(pkg->origin_data.db, pkg, INFRQ_DESC); + } + return pkg->desc; +} + +const char *_cache_get_url(pmpkg_t *pkg) +{ + ALPM_LOG_FUNC; + + /* Sanity checks */ + ASSERT(handle != NULL, return(NULL)); + ASSERT(pkg != NULL, return(NULL)); + + if(!(pkg->infolevel & INFRQ_DESC)) { + _alpm_db_read(pkg->origin_data.db, pkg, INFRQ_DESC); + } + return pkg->url; +} + +time_t _cache_get_builddate(pmpkg_t *pkg) +{ + ALPM_LOG_FUNC; + + /* Sanity checks */ + ASSERT(handle != NULL, return(0)); + ASSERT(pkg != NULL, return(0)); + + if(!(pkg->infolevel & INFRQ_DESC)) { + _alpm_db_read(pkg->origin_data.db, pkg, INFRQ_DESC); + } + return pkg->builddate; +} + +time_t _cache_get_installdate(pmpkg_t *pkg) +{ + ALPM_LOG_FUNC; + + /* Sanity checks */ + ASSERT(handle != NULL, return(0)); + ASSERT(pkg != NULL, return(0)); + + if(!(pkg->infolevel & INFRQ_DESC)) { + _alpm_db_read(pkg->origin_data.db, pkg, INFRQ_DESC); + } + return pkg->installdate; +} + +const char *_cache_get_packager(pmpkg_t *pkg) +{ + ALPM_LOG_FUNC; + + /* Sanity checks */ + ASSERT(handle != NULL, return(NULL)); + ASSERT(pkg != NULL, return(NULL)); + + if(!(pkg->infolevel & INFRQ_DESC)) { + _alpm_db_read(pkg->origin_data.db, pkg, INFRQ_DESC); + } + return pkg->packager; +} + +const char *_cache_get_md5sum(pmpkg_t *pkg) +{ + ALPM_LOG_FUNC; + + /* Sanity checks */ + ASSERT(handle != NULL, return(NULL)); + ASSERT(pkg != NULL, return(NULL)); + + if(!(pkg->infolevel & INFRQ_DESC)) { + _alpm_db_read(pkg->origin_data.db, pkg, INFRQ_DESC); + } + return pkg->md5sum; +} + +const char *_cache_get_arch(pmpkg_t *pkg) +{ + ALPM_LOG_FUNC; + + /* Sanity checks */ + ASSERT(handle != NULL, return(NULL)); + ASSERT(pkg != NULL, return(NULL)); + + if(!(pkg->infolevel & INFRQ_DESC)) { + _alpm_db_read(pkg->origin_data.db, pkg, INFRQ_DESC); + } + return pkg->arch; +} + +unsigned long _cache_get_size(pmpkg_t *pkg) +{ + ALPM_LOG_FUNC; + + /* Sanity checks */ + ASSERT(handle != NULL, return(-1)); + ASSERT(pkg != NULL, return(-1)); + + if(!(pkg->infolevel & INFRQ_DESC)) { + _alpm_db_read(pkg->origin_data.db, pkg, INFRQ_DESC); + } + return pkg->size; +} + +unsigned long _cache_get_isize(pmpkg_t *pkg) +{ + ALPM_LOG_FUNC; + + /* Sanity checks */ + ASSERT(handle != NULL, return(-1)); + ASSERT(pkg != NULL, return(-1)); + + if(!(pkg->infolevel & INFRQ_DESC)) { + _alpm_db_read(pkg->origin_data.db, pkg, INFRQ_DESC); + } + return pkg->isize; +} + +pmpkgreason_t _cache_get_reason(pmpkg_t *pkg) +{ + ALPM_LOG_FUNC; + + /* Sanity checks */ + ASSERT(handle != NULL, return(-1)); + ASSERT(pkg != NULL, return(-1)); + + if(!(pkg->infolevel & INFRQ_DESC)) { + _alpm_db_read(pkg->origin_data.db, pkg, INFRQ_DESC); + } + return pkg->reason; +} + +alpm_list_t *_cache_get_licenses(pmpkg_t *pkg) +{ + ALPM_LOG_FUNC; + + /* Sanity checks */ + ASSERT(handle != NULL, return(NULL)); + ASSERT(pkg != NULL, return(NULL)); + + if(!(pkg->infolevel & INFRQ_DESC)) { + _alpm_db_read(pkg->origin_data.db, pkg, INFRQ_DESC); + } + return pkg->licenses; +} + +alpm_list_t *_cache_get_groups(pmpkg_t *pkg) +{ + ALPM_LOG_FUNC; + + /* Sanity checks */ + ASSERT(handle != NULL, return(NULL)); + ASSERT(pkg != NULL, return(NULL)); + + if(!(pkg->infolevel & INFRQ_DESC)) { + _alpm_db_read(pkg->origin_data.db, pkg, INFRQ_DESC); + } + return pkg->groups; +} + +alpm_list_t *_cache_get_depends(pmpkg_t *pkg) +{ + ALPM_LOG_FUNC; + + /* Sanity checks */ + ASSERT(handle != NULL, return(NULL)); + ASSERT(pkg != NULL, return(NULL)); + + if(!(pkg->infolevel & INFRQ_DEPENDS)) { + _alpm_db_read(pkg->origin_data.db, pkg, INFRQ_DEPENDS); + } + return pkg->depends; +} + +alpm_list_t *_cache_get_optdepends(pmpkg_t *pkg) +{ + ALPM_LOG_FUNC; + + /* Sanity checks */ + ASSERT(handle != NULL, return(NULL)); + ASSERT(pkg != NULL, return(NULL)); + + if(!(pkg->infolevel & INFRQ_DEPENDS)) { + _alpm_db_read(pkg->origin_data.db, pkg, INFRQ_DEPENDS); + } + return pkg->optdepends; +} + +alpm_list_t *_cache_get_conflicts(pmpkg_t *pkg) +{ + ALPM_LOG_FUNC; + + /* Sanity checks */ + ASSERT(handle != NULL, return(NULL)); + ASSERT(pkg != NULL, return(NULL)); + + if(!(pkg->infolevel & INFRQ_DEPENDS)) { + _alpm_db_read(pkg->origin_data.db, pkg, INFRQ_DEPENDS); + } + return pkg->conflicts; +} + +alpm_list_t *_cache_get_provides(pmpkg_t *pkg) +{ + ALPM_LOG_FUNC; + + /* Sanity checks */ + ASSERT(handle != NULL, return(NULL)); + ASSERT(pkg != NULL, return(NULL)); + + if(!(pkg->infolevel & INFRQ_DEPENDS)) { + _alpm_db_read(pkg->origin_data.db, pkg, INFRQ_DEPENDS); + } + return pkg->provides; +} + +alpm_list_t *_cache_get_replaces(pmpkg_t *pkg) +{ + ALPM_LOG_FUNC; + + /* Sanity checks */ + ASSERT(handle != NULL, return(NULL)); + ASSERT(pkg != NULL, return(NULL)); + + if(!(pkg->infolevel & INFRQ_DESC)) { + _alpm_db_read(pkg->origin_data.db, pkg, INFRQ_DESC); + } + return pkg->replaces; +} + +alpm_list_t *_cache_get_deltas(pmpkg_t *pkg) +{ + ALPM_LOG_FUNC; + + /* Sanity checks */ + ASSERT(handle != NULL, return(NULL)); + ASSERT(pkg != NULL, return(NULL)); + + if(!(pkg->infolevel & INFRQ_DELTAS)) { + _alpm_db_read(pkg->origin_data.db, pkg, INFRQ_DELTAS); + } + return pkg->deltas; +} + + +alpm_list_t *_cache_get_files(pmpkg_t *pkg) +{ + ALPM_LOG_FUNC; + + /* Sanity checks */ + ASSERT(handle != NULL, return(NULL)); + ASSERT(pkg != NULL, return(NULL)); + + if(pkg->origin == PKG_FROM_LOCALDB + && !(pkg->infolevel & INFRQ_FILES)) { + _alpm_db_read(pkg->origin_data.db, pkg, INFRQ_FILES); + } + return pkg->files; +} + +alpm_list_t *_cache_get_backup(pmpkg_t *pkg) +{ + ALPM_LOG_FUNC; + + /* Sanity checks */ + ASSERT(handle != NULL, return(NULL)); + ASSERT(pkg != NULL, return(NULL)); + + if(pkg->origin == PKG_FROM_LOCALDB + && !(pkg->infolevel & INFRQ_FILES)) { + _alpm_db_read(pkg->origin_data.db, pkg, INFRQ_FILES); + } + return pkg->backup; +} + +/** The sync database operations struct. Get package fields through + * lazy accessor methods that handle any backend loading and caching + * logic. + */ +static struct pkg_operations sync_pkg_ops = { + .get_filename = _cache_get_filename, + .get_name = _cache_get_name, + .get_version = _cache_get_version, + .get_desc = _cache_get_desc, + .get_url = _cache_get_url, + .get_builddate = _cache_get_builddate, + .get_installdate = _cache_get_installdate, + .get_packager = _cache_get_packager, + .get_md5sum = _cache_get_md5sum, + .get_arch = _cache_get_arch, + .get_size = _cache_get_size, + .get_isize = _cache_get_isize, + .get_reason = _cache_get_reason, + .get_licenses = _cache_get_licenses, + .get_groups = _cache_get_groups, + .get_depends = _cache_get_depends, + .get_optdepends = _cache_get_optdepends, + .get_conflicts = _cache_get_conflicts, + .get_provides = _cache_get_provides, + .get_replaces = _cache_get_replaces, + .get_deltas = _cache_get_deltas, + .get_files = _cache_get_files, + .get_backup = _cache_get_backup, +}; + +/** The local database operations struct. Get package fields through + * lazy accessor methods that handle any backend loading and caching + * logic. + */ +static struct pkg_operations local_pkg_ops = { + .get_filename = _cache_get_filename, + .get_name = _cache_get_name, + .get_version = _cache_get_version, + .get_desc = _cache_get_desc, + .get_url = _cache_get_url, + .get_builddate = _cache_get_builddate, + .get_installdate = _cache_get_installdate, + .get_packager = _cache_get_packager, + .get_md5sum = _cache_get_md5sum, + .get_arch = _cache_get_arch, + .get_size = _cache_get_size, + .get_isize = _cache_get_isize, + .get_reason = _cache_get_reason, + .get_licenses = _cache_get_licenses, + .get_groups = _cache_get_groups, + .get_depends = _cache_get_depends, + .get_optdepends = _cache_get_optdepends, + .get_conflicts = _cache_get_conflicts, + .get_provides = _cache_get_provides, + .get_replaces = _cache_get_replaces, + .get_deltas = _cache_get_deltas, + .get_files = _cache_get_files, + .get_backup = _cache_get_backup, +}; /* * Return the last update time as number of seconds from the epoch. @@ -368,8 +745,10 @@ pmpkg_t *_alpm_db_scan(pmdb_t *db, const char *target) /* TODO bad bad hack for now */ if(db == handle->db_local) { pkg->origin = PKG_FROM_LOCALDB; + pkg->ops = &local_pkg_ops; } else { pkg->origin = PKG_FROM_SYNCDB; + pkg->ops = &sync_pkg_ops; } pkg->origin_data.db = db; } diff --git a/lib/libalpm/be_package.c b/lib/libalpm/be_package.c index bee0635..93965db 100644 --- a/lib/libalpm/be_package.c +++ b/lib/libalpm/be_package.c @@ -229,7 +229,9 @@ static pmpkg_t *pkg_load(const char *pkgfile, unsigned short full) /* internal fields for package struct */ newpkg->origin = PKG_FROM_FILE; + /* TODO eventually kill/move this? */ newpkg->origin_data.file = strdup(pkgfile); + newpkg->ops = &default_pkg_ops; if(full) { /* "checking for conflicts" requires a sorted list, ensure that here */ diff --git a/lib/libalpm/package.c b/lib/libalpm/package.c index 2e84f3b..c226a64 100644 --- a/lib/libalpm/package.c +++ b/lib/libalpm/package.c @@ -1,10 +1,7 @@ /* * package.c * - * Copyright (c) 2002-2007 by Judd Vinet <jvinet@zeroflux.org> - * Copyright (c) 2005 by Aurelien Foret <orelien@chez.com> - * Copyright (c) 2005, 2006 by Christian Hamar <krics@linuxforum.hu> - * Copyright (c) 2005, 2006 by Miklos Vajna <vmiklos@frugalware.org> + * Copyright (c) 2002-2008 by Judd Vinet <jvinet@zeroflux.org> * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -112,329 +109,180 @@ int SYMEXPORT alpm_pkg_vercmp(const char *ver1, const char *ver2) return(_alpm_versioncmp(ver1, ver2)); } +/* 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. */ +const char *_pkg_get_filename(pmpkg_t *pkg) { return pkg->filename; } +const char *_pkg_get_name(pmpkg_t *pkg) { return pkg->name; } +const char *_pkg_get_version(pmpkg_t *pkg) { return pkg->version; } +const char *_pkg_get_desc(pmpkg_t *pkg) { return pkg->desc; } +const char *_pkg_get_url(pmpkg_t *pkg) { return pkg->url; } +time_t _pkg_get_builddate(pmpkg_t *pkg) { return pkg->builddate; } +time_t _pkg_get_installdate(pmpkg_t *pkg) { return pkg->installdate; } +const char *_pkg_get_packager(pmpkg_t *pkg) { return pkg->packager; } +const char *_pkg_get_md5sum(pmpkg_t *pkg) { return pkg->md5sum; } +const char *_pkg_get_arch(pmpkg_t *pkg) { return pkg->arch; } +unsigned long _pkg_get_size(pmpkg_t *pkg) { return pkg->size; } +unsigned long _pkg_get_isize(pmpkg_t *pkg) { return pkg->isize; } +pmpkgreason_t _pkg_get_reason(pmpkg_t *pkg) { return pkg->reason; } + +alpm_list_t *_pkg_get_licenses(pmpkg_t *pkg) { return pkg->licenses; } +alpm_list_t *_pkg_get_groups(pmpkg_t *pkg) { return pkg->groups; } +alpm_list_t *_pkg_get_depends(pmpkg_t *pkg) { return pkg->depends; } +alpm_list_t *_pkg_get_optdepends(pmpkg_t *pkg) { return pkg->optdepends; } +alpm_list_t *_pkg_get_conflicts(pmpkg_t *pkg) { return pkg->conflicts; } +alpm_list_t *_pkg_get_provides(pmpkg_t *pkg) { return pkg->provides; } +alpm_list_t *_pkg_get_replaces(pmpkg_t *pkg) { return pkg->replaces; } +alpm_list_t *_pkg_get_deltas(pmpkg_t *pkg) { return pkg->deltas; } +alpm_list_t *_pkg_get_files(pmpkg_t *pkg) { return pkg->files; } +alpm_list_t *_pkg_get_backup(pmpkg_t *pkg) { return pkg->backup; } + +/** The standard package operations struct. Get fields directly from the + * struct itself with no abstraction layer or any type of lazy loading. + */ +struct pkg_operations default_pkg_ops = { + .get_filename = _pkg_get_filename, + .get_name = _pkg_get_name, + .get_version = _pkg_get_version, + .get_desc = _pkg_get_desc, + .get_url = _pkg_get_url, + .get_builddate = _pkg_get_builddate, + .get_installdate = _pkg_get_installdate, + .get_packager = _pkg_get_packager, + .get_md5sum = _pkg_get_md5sum, + .get_arch = _pkg_get_arch, + .get_size = _pkg_get_size, + .get_isize = _pkg_get_isize, + .get_reason = _pkg_get_reason, + .get_licenses = _pkg_get_licenses, + .get_groups = _pkg_get_groups, + .get_depends = _pkg_get_depends, + .get_optdepends = _pkg_get_optdepends, + .get_conflicts = _pkg_get_conflicts, + .get_provides = _pkg_get_provides, + .get_replaces = _pkg_get_replaces, + .get_deltas = _pkg_get_deltas, + .get_files = _pkg_get_files, + .get_backup = _pkg_get_backup, +}; + +/* Public functions for getting package information. These functions + * delegate the hard work to the function callbacks attached to each + * package, which depend on where the package was loaded from. */ const char SYMEXPORT *alpm_pkg_get_filename(pmpkg_t *pkg) { - ALPM_LOG_FUNC; - - /* Sanity checks */ - ASSERT(handle != NULL, return(NULL)); - ASSERT(pkg != NULL, return(NULL)); - - if(pkg->origin != PKG_FROM_FILE && !(pkg->infolevel & INFRQ_DESC)) { - _alpm_db_read(pkg->origin_data.db, pkg, INFRQ_DESC); - } - - return pkg->filename; + return pkg->ops->get_filename(pkg); } const char SYMEXPORT *alpm_pkg_get_name(pmpkg_t *pkg) { - ALPM_LOG_FUNC; - - /* Sanity checks */ - ASSERT(handle != NULL, return(NULL)); - ASSERT(pkg != NULL, return(NULL)); - - if(pkg->origin != PKG_FROM_FILE && !(pkg->infolevel & INFRQ_BASE)) { - _alpm_db_read(pkg->origin_data.db, pkg, INFRQ_BASE); - } - return pkg->name; + return pkg->ops->get_name(pkg); } const char SYMEXPORT *alpm_pkg_get_version(pmpkg_t *pkg) { - ALPM_LOG_FUNC; - - /* Sanity checks */ - ASSERT(handle != NULL, return(NULL)); - ASSERT(pkg != NULL, return(NULL)); - - if(pkg->origin != PKG_FROM_FILE && !(pkg->infolevel & INFRQ_BASE)) { - _alpm_db_read(pkg->origin_data.db, pkg, INFRQ_BASE); - } - return pkg->version; + return pkg->ops->get_version(pkg); } const char SYMEXPORT *alpm_pkg_get_desc(pmpkg_t *pkg) { - ALPM_LOG_FUNC; - - /* Sanity checks */ - ASSERT(handle != NULL, return(NULL)); - ASSERT(pkg != NULL, return(NULL)); - - if(pkg->origin != PKG_FROM_FILE && !(pkg->infolevel & INFRQ_DESC)) { - _alpm_db_read(pkg->origin_data.db, pkg, INFRQ_DESC); - } - return pkg->desc; + return pkg->ops->get_desc(pkg); } const char SYMEXPORT *alpm_pkg_get_url(pmpkg_t *pkg) { - ALPM_LOG_FUNC; - - /* Sanity checks */ - ASSERT(handle != NULL, return(NULL)); - ASSERT(pkg != NULL, return(NULL)); - - if(pkg->origin != PKG_FROM_FILE && !(pkg->infolevel & INFRQ_DESC)) { - _alpm_db_read(pkg->origin_data.db, pkg, INFRQ_DESC); - } - return pkg->url; + return pkg->ops->get_url(pkg); } time_t SYMEXPORT alpm_pkg_get_builddate(pmpkg_t *pkg) { - ALPM_LOG_FUNC; - - /* Sanity checks */ - ASSERT(handle != NULL, return(0)); - ASSERT(pkg != NULL, return(0)); - - if(pkg->origin != PKG_FROM_FILE && !(pkg->infolevel & INFRQ_DESC)) { - _alpm_db_read(pkg->origin_data.db, pkg, INFRQ_DESC); - } - return pkg->builddate; + return pkg->ops->get_builddate(pkg); } time_t SYMEXPORT alpm_pkg_get_installdate(pmpkg_t *pkg) { - ALPM_LOG_FUNC; - - /* Sanity checks */ - ASSERT(handle != NULL, return(0)); - ASSERT(pkg != NULL, return(0)); - - if(pkg->origin != PKG_FROM_FILE && !(pkg->infolevel & INFRQ_DESC)) { - _alpm_db_read(pkg->origin_data.db, pkg, INFRQ_DESC); - } - return pkg->installdate; + return pkg->ops->get_installdate(pkg); } const char SYMEXPORT *alpm_pkg_get_packager(pmpkg_t *pkg) { - ALPM_LOG_FUNC; - - /* Sanity checks */ - ASSERT(handle != NULL, return(NULL)); - ASSERT(pkg != NULL, return(NULL)); - - if(pkg->origin != PKG_FROM_FILE && !(pkg->infolevel & INFRQ_DESC)) { - _alpm_db_read(pkg->origin_data.db, pkg, INFRQ_DESC); - } - return pkg->packager; + return pkg->ops->get_packager(pkg); } const char SYMEXPORT *alpm_pkg_get_md5sum(pmpkg_t *pkg) { - ALPM_LOG_FUNC; - - /* Sanity checks */ - ASSERT(handle != NULL, return(NULL)); - ASSERT(pkg != NULL, return(NULL)); - - if(pkg->origin != PKG_FROM_FILE && !(pkg->infolevel & INFRQ_DESC)) { - _alpm_db_read(pkg->origin_data.db, pkg, INFRQ_DESC); - } - return pkg->md5sum; + return pkg->ops->get_md5sum(pkg); } const char SYMEXPORT *alpm_pkg_get_arch(pmpkg_t *pkg) { - ALPM_LOG_FUNC; - - /* Sanity checks */ - ASSERT(handle != NULL, return(NULL)); - ASSERT(pkg != NULL, return(NULL)); - - if(pkg->origin != PKG_FROM_FILE && !(pkg->infolevel & INFRQ_DESC)) { - _alpm_db_read(pkg->origin_data.db, pkg, INFRQ_DESC); - } - return pkg->arch; + return pkg->ops->get_arch(pkg); } unsigned long SYMEXPORT alpm_pkg_get_size(pmpkg_t *pkg) { - ALPM_LOG_FUNC; - - /* Sanity checks */ - ASSERT(handle != NULL, return(-1)); - ASSERT(pkg != NULL, return(-1)); - - if(pkg->origin != PKG_FROM_FILE && !(pkg->infolevel & INFRQ_DESC)) { - _alpm_db_read(pkg->origin_data.db, pkg, INFRQ_DESC); - } - return pkg->size; + return pkg->ops->get_size(pkg); } unsigned long SYMEXPORT alpm_pkg_get_isize(pmpkg_t *pkg) { - ALPM_LOG_FUNC; - - /* Sanity checks */ - ASSERT(handle != NULL, return(-1)); - ASSERT(pkg != NULL, return(-1)); - - if(pkg->origin != PKG_FROM_FILE && !(pkg->infolevel & INFRQ_DESC)) { - _alpm_db_read(pkg->origin_data.db, pkg, INFRQ_DESC); - } - return pkg->isize; + return pkg->ops->get_isize(pkg); } pmpkgreason_t SYMEXPORT alpm_pkg_get_reason(pmpkg_t *pkg) { - ALPM_LOG_FUNC; - - /* Sanity checks */ - ASSERT(handle != NULL, return(-1)); - ASSERT(pkg != NULL, return(-1)); - - if(pkg->origin != PKG_FROM_FILE && !(pkg->infolevel & INFRQ_DESC)) { - _alpm_db_read(pkg->origin_data.db, pkg, INFRQ_DESC); - } - return pkg->reason; + return pkg->ops->get_reason(pkg); } alpm_list_t SYMEXPORT *alpm_pkg_get_licenses(pmpkg_t *pkg) { - ALPM_LOG_FUNC; - - /* Sanity checks */ - ASSERT(handle != NULL, return(NULL)); - ASSERT(pkg != NULL, return(NULL)); - - if(pkg->origin != PKG_FROM_FILE && !(pkg->infolevel & INFRQ_DESC)) { - _alpm_db_read(pkg->origin_data.db, pkg, INFRQ_DESC); - } - return pkg->licenses; + return pkg->ops->get_licenses(pkg); } alpm_list_t SYMEXPORT *alpm_pkg_get_groups(pmpkg_t *pkg) { - ALPM_LOG_FUNC; - - /* Sanity checks */ - ASSERT(handle != NULL, return(NULL)); - ASSERT(pkg != NULL, return(NULL)); - - if(pkg->origin != PKG_FROM_FILE && !(pkg->infolevel & INFRQ_DESC)) { - _alpm_db_read(pkg->origin_data.db, pkg, INFRQ_DESC); - } - return pkg->groups; + return pkg->ops->get_groups(pkg); } alpm_list_t SYMEXPORT *alpm_pkg_get_depends(pmpkg_t *pkg) { - ALPM_LOG_FUNC; - - /* Sanity checks */ - ASSERT(handle != NULL, return(NULL)); - ASSERT(pkg != NULL, return(NULL)); - - if(pkg->origin != PKG_FROM_FILE && !(pkg->infolevel & INFRQ_DEPENDS)) { - _alpm_db_read(pkg->origin_data.db, pkg, INFRQ_DEPENDS); - } - return pkg->depends; + return pkg->ops->get_depends(pkg); } alpm_list_t SYMEXPORT *alpm_pkg_get_optdepends(pmpkg_t *pkg) { - ALPM_LOG_FUNC; - - /* Sanity checks */ - ASSERT(handle != NULL, return(NULL)); - ASSERT(pkg != NULL, return(NULL)); - - if(pkg->origin != PKG_FROM_FILE && !(pkg->infolevel & INFRQ_DEPENDS)) { - _alpm_db_read(pkg->origin_data.db, pkg, INFRQ_DEPENDS); - } - return pkg->optdepends; + return pkg->ops->get_optdepends(pkg); } alpm_list_t SYMEXPORT *alpm_pkg_get_conflicts(pmpkg_t *pkg) { - ALPM_LOG_FUNC; - - /* Sanity checks */ - ASSERT(handle != NULL, return(NULL)); - ASSERT(pkg != NULL, return(NULL)); - - if(pkg->origin != PKG_FROM_FILE && !(pkg->infolevel & INFRQ_DEPENDS)) { - _alpm_db_read(pkg->origin_data.db, pkg, INFRQ_DEPENDS); - } - return pkg->conflicts; + return pkg->ops->get_conflicts(pkg); } alpm_list_t SYMEXPORT *alpm_pkg_get_provides(pmpkg_t *pkg) { - ALPM_LOG_FUNC; - - /* Sanity checks */ - ASSERT(handle != NULL, return(NULL)); - ASSERT(pkg != NULL, return(NULL)); - - if(pkg->origin != PKG_FROM_FILE && !(pkg->infolevel & INFRQ_DEPENDS)) { - _alpm_db_read(pkg->origin_data.db, pkg, INFRQ_DEPENDS); - } - return pkg->provides; + return pkg->ops->get_provides(pkg); } -alpm_list_t SYMEXPORT *alpm_pkg_get_deltas(pmpkg_t *pkg) +alpm_list_t SYMEXPORT *alpm_pkg_get_replaces(pmpkg_t *pkg) { - ALPM_LOG_FUNC; - - /* Sanity checks */ - ASSERT(handle != NULL, return(NULL)); - ASSERT(pkg != NULL, return(NULL)); - - if(pkg->origin != PKG_FROM_FILE && !(pkg->infolevel & INFRQ_DELTAS)) { - _alpm_db_read(pkg->origin_data.db, pkg, INFRQ_DELTAS); - } - return pkg->deltas; + return pkg->ops->get_replaces(pkg); } -alpm_list_t SYMEXPORT *alpm_pkg_get_replaces(pmpkg_t *pkg) +alpm_list_t SYMEXPORT *alpm_pkg_get_deltas(pmpkg_t *pkg) { - ALPM_LOG_FUNC; - - /* Sanity checks */ - ASSERT(handle != NULL, return(NULL)); - ASSERT(pkg != NULL, return(NULL)); - - if(pkg->origin != PKG_FROM_FILE && !(pkg->infolevel & INFRQ_DESC)) { - _alpm_db_read(pkg->origin_data.db, pkg, INFRQ_DESC); - } - return pkg->replaces; + return pkg->ops->get_deltas(pkg); } alpm_list_t SYMEXPORT *alpm_pkg_get_files(pmpkg_t *pkg) { - ALPM_LOG_FUNC; - - /* Sanity checks */ - ASSERT(handle != NULL, return(NULL)); - ASSERT(pkg != NULL, return(NULL)); - - if(pkg->origin == PKG_FROM_LOCALDB - && !(pkg->infolevel & INFRQ_FILES)) { - _alpm_db_read(pkg->origin_data.db, pkg, INFRQ_FILES); - } - return pkg->files; + return pkg->ops->get_files(pkg); } alpm_list_t SYMEXPORT *alpm_pkg_get_backup(pmpkg_t *pkg) { - ALPM_LOG_FUNC; - - /* Sanity checks */ - ASSERT(handle != NULL, return(NULL)); - ASSERT(pkg != NULL, return(NULL)); - - if(pkg->origin == PKG_FROM_LOCALDB - && !(pkg->infolevel & INFRQ_FILES)) { - _alpm_db_read(pkg->origin_data.db, pkg, INFRQ_FILES); - } - return pkg->backup; + return pkg->ops->get_backup(pkg); } /** @@ -756,6 +604,7 @@ pmpkg_t *_alpm_pkg_dup(pmpkg_t *pkg) /* internal */ newpkg->origin = pkg->origin; + newpkg->ops = pkg->ops; if(newpkg->origin == PKG_FROM_FILE) { newpkg->origin_data.file = strdup(pkg->origin_data.file); } else { diff --git a/lib/libalpm/package.h b/lib/libalpm/package.h index 6027021..a40c2a3 100644 --- a/lib/libalpm/package.h +++ b/lib/libalpm/package.h @@ -1,11 +1,7 @@ /* * package.h * - * Copyright (c) 2002-2007 by Judd Vinet <jvinet@zeroflux.org> - * Copyright (c) 2005 by Aurelien Foret <orelien@chez.com> - * Copyright (c) 2006 by David Kimpe <dnaku@frugalware.org> - * Copyright (c) 2005, 2006 by Christian Hamar <krics@linuxforum.hu> - * Copyright (c) 2005, 2006 by Miklos Vajna <vmiklos@frugalware.org> + * Copyright (c) 2002-2008 by Judd Vinet <jvinet@zeroflux.org> * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -34,6 +30,59 @@ typedef enum _pmpkgfrom_t { PKG_FROM_SYNCDB } pmpkgfrom_t; +/** 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() + */ +}; + +/** The standard package operations struct. get fields directly from the + * struct itself with no abstraction layer or any type of lazy loading. + * The actual definition is in package.c so it can have access to the + * default accessor functions which are defined there. + */ +extern struct pkg_operations default_pkg_ops; + struct __pmpkg_t { char *filename; char *name; @@ -73,6 +122,8 @@ struct __pmpkg_t { pmdbinfrq_t infolevel; unsigned long download_size; alpm_list_t *delta_path; + + struct pkg_operations *ops; }; int _alpm_versioncmp(const char *a, const char *b); -- 1.5.5.1
Signed-off-by: Dan McGee <dan@archlinux.org> --- lib/libalpm/be_files.c | 61 +++++++++++++++++++++++++++ lib/libalpm/be_package.c | 104 +++++++++++++++++++++++++++++++++++++++++++++- lib/libalpm/package.c | 71 ++----------------------------- 3 files changed, 168 insertions(+), 68 deletions(-) diff --git a/lib/libalpm/be_files.c b/lib/libalpm/be_files.c index c9e708d..485d960 100644 --- a/lib/libalpm/be_files.c +++ b/lib/libalpm/be_files.c @@ -364,6 +364,63 @@ alpm_list_t *_cache_get_backup(pmpkg_t *pkg) return pkg->backup; } +/** + * Open a package changelog for reading. Similar to fopen in functionality, + * except that the returned 'file stream' is from the database. + * @param pkg the package (from db) to read the changelog + * @return a 'file stream' to the package changelog + */ +void *_cache_changelog_open(pmpkg_t *pkg) +{ + ALPM_LOG_FUNC; + + /* Sanity checks */ + ASSERT(handle != NULL, return(NULL)); + ASSERT(pkg != NULL, return(NULL)); + + char clfile[PATH_MAX]; + snprintf(clfile, PATH_MAX, "%s/%s/%s-%s/changelog", + alpm_option_get_dbpath(), + alpm_db_get_name(handle->db_local), + alpm_pkg_get_name(pkg), + alpm_pkg_get_version(pkg)); + return fopen(clfile, "r"); +} + +/** + * Read data from an open changelog 'file stream'. Similar to fread in + * functionality, this function takes a buffer and amount of data to read. + * @param ptr a buffer to fill with raw changelog data + * @param size the size of the buffer + * @param pkg the package that the changelog is being read from + * @param fp a 'file stream' to the package changelog + * @return the number of characters read, or 0 if there is no more data + */ +size_t _cache_changelog_read(void *ptr, size_t size, + const pmpkg_t *pkg, const void *fp) +{ + return ( fread(ptr, 1, size, (FILE*)fp) ); +} + +/* +int _cache_changelog_feof(const pmpkg_t *pkg, void *fp) +{ + return( feof((FILE*)fp) ); +} +*/ + +/** + * Close a package changelog for reading. Similar to fclose in functionality, + * except that the 'file stream' is from the database. + * @param pkg the package that the changelog was read from + * @param fp a 'file stream' to the package changelog + * @return whether closing the package changelog stream was successful + */ +int _cache_changelog_close(const pmpkg_t *pkg, void *fp) +{ + return( fclose((FILE*)fp) ); +} + /** The sync database operations struct. Get package fields through * lazy accessor methods that handle any backend loading and caching * logic. @@ -422,6 +479,10 @@ static struct pkg_operations local_pkg_ops = { .get_deltas = _cache_get_deltas, .get_files = _cache_get_files, .get_backup = _cache_get_backup, + + .changelog_open = _cache_changelog_open, + .changelog_read = _cache_changelog_read, + .changelog_close = _cache_changelog_close, }; /* diff --git a/lib/libalpm/be_package.c b/lib/libalpm/be_package.c index 93965db..0c8291a 100644 --- a/lib/libalpm/be_package.c +++ b/lib/libalpm/be_package.c @@ -25,6 +25,7 @@ #include <limits.h> #include <ctype.h> #include <locale.h> /* setlocale */ +#include <errno.h> /* libarchive */ #include <archive.h> @@ -39,6 +40,107 @@ #include "deps.h" /* _alpm_splitdep */ /** + * Open a package changelog for reading. Similar to fopen in functionality, + * except that the returned 'file stream' is from an archive. + * @param pkg the package (file) to read the changelog + * @return a 'file stream' to the package changelog + */ +void *_package_changelog_open(pmpkg_t *pkg) +{ + ALPM_LOG_FUNC; + + ASSERT(pkg != NULL, return(NULL)); + + struct archive *archive = NULL; + struct archive_entry *entry; + const char *pkgfile = pkg->origin_data.file; + int ret = ARCHIVE_OK; + + if((archive = archive_read_new()) == NULL) { + RET_ERR(PM_ERR_LIBARCHIVE, NULL); + } + + archive_read_support_compression_all(archive); + archive_read_support_format_all(archive); + + if (archive_read_open_filename(archive, pkgfile, + ARCHIVE_DEFAULT_BYTES_PER_BLOCK) != ARCHIVE_OK) { + RET_ERR(PM_ERR_PKG_OPEN, NULL); + } + + while((ret = archive_read_next_header(archive, &entry)) == ARCHIVE_OK) { + const char *entry_name = archive_entry_pathname(entry); + + if(strcmp(entry_name, ".CHANGELOG") == 0) { + return(archive); + } + } + /* we didn't find a changelog */ + archive_read_finish(archive); + errno = ENOENT; + + return(NULL); +} + +/** + * Read data from an open changelog 'file stream'. Similar to fread in + * functionality, this function takes a buffer and amount of data to read. + * @param ptr a buffer to fill with raw changelog data + * @param size the size of the buffer + * @param pkg the package that the changelog is being read from + * @param fp a 'file stream' to the package changelog + * @return the number of characters read, or 0 if there is no more data + */ +size_t _package_changelog_read(void *ptr, size_t size, + const pmpkg_t *pkg, const void *fp) +{ + return( archive_read_data((struct archive*)fp, ptr, size) ); +} + +/* +int _package_changelog_feof(const pmpkg_t *pkg, void *fp) +{ + // note: this doesn't quite work, no feof in libarchive + return( archive_read_data((struct archive*)fp, NULL, 0) ); +} +*/ + +/** + * Close a package changelog for reading. Similar to fclose in functionality, + * except that the 'file stream' is from an archive. + * @param pkg the package (file) that the changelog was read from + * @param fp a 'file stream' to the package changelog + * @return whether closing the package changelog stream was successful + */ +int _package_changelog_close(const pmpkg_t *pkg, void *fp) +{ + return( archive_read_finish((struct archive *)fp) ); +} + + +/** Package file operations struct accessor. We implement this as a method + * rather than a static struct as in be_files because we want to reuse the + * majority of the default_pkg_ops struct and add only a few operations of + * our own on top. The static file_pkg_ops variable inside this function + * lets us only initialize an operations struct once which can always be + * accessed by this method. + */ +static struct pkg_operations *get_file_pkg_ops() +{ + static struct pkg_operations *file_pkg_ops = NULL; + /* determine whether our static file_pkg_ops struct has been initialized */ + if(!file_pkg_ops) { + MALLOC(file_pkg_ops, sizeof(struct pkg_operations), + RET_ERR(PM_ERR_MEMORY, NULL)); + memcpy(file_pkg_ops, &default_pkg_ops, sizeof(struct pkg_operations)); + file_pkg_ops->changelog_open = _package_changelog_open; + file_pkg_ops->changelog_read = _package_changelog_read; + file_pkg_ops->changelog_close = _package_changelog_close; + } + return(file_pkg_ops); +} + +/** * Parses the package description file for a package into a pmpkg_t struct. * @param archive the archive to read from, pointed at the .PKGINFO entry * @param newpkg an empty pmpkg_t struct to fill with package info @@ -231,7 +333,7 @@ static pmpkg_t *pkg_load(const char *pkgfile, unsigned short full) newpkg->origin = PKG_FROM_FILE; /* TODO eventually kill/move this? */ newpkg->origin_data.file = strdup(pkgfile); - newpkg->ops = &default_pkg_ops; + newpkg->ops = get_file_pkg_ops(); if(full) { /* "checking for conflicts" requires a sorted list, ensure that here */ diff --git a/lib/libalpm/package.c b/lib/libalpm/package.c index c226a64..1dec422 100644 --- a/lib/libalpm/package.c +++ b/lib/libalpm/package.c @@ -294,51 +294,7 @@ alpm_list_t SYMEXPORT *alpm_pkg_get_backup(pmpkg_t *pkg) */ void SYMEXPORT *alpm_pkg_changelog_open(pmpkg_t *pkg) { - ALPM_LOG_FUNC; - - /* Sanity checks */ - ASSERT(handle != NULL, return(NULL)); - ASSERT(pkg != NULL, return(NULL)); - - if(pkg->origin == PKG_FROM_FILE) { - struct archive *archive = NULL; - struct archive_entry *entry; - const char *pkgfile = pkg->origin_data.file; - int ret = ARCHIVE_OK; - - if((archive = archive_read_new()) == NULL) { - RET_ERR(PM_ERR_LIBARCHIVE, NULL); - } - - archive_read_support_compression_all(archive); - archive_read_support_format_all(archive); - - if (archive_read_open_filename(archive, pkgfile, - ARCHIVE_DEFAULT_BYTES_PER_BLOCK) != ARCHIVE_OK) { - RET_ERR(PM_ERR_PKG_OPEN, NULL); - } - - while((ret = archive_read_next_header(archive, &entry)) == ARCHIVE_OK) { - const char *entry_name = archive_entry_pathname(entry); - - if(strcmp(entry_name, ".CHANGELOG") == 0) { - return(archive); - } - } - /* we didn't find a changelog */ - archive_read_finish(archive); - errno = ENOENT; - } else { - char clfile[PATH_MAX]; - snprintf(clfile, PATH_MAX, "%s/%s/%s-%s/changelog", - alpm_option_get_dbpath(), - alpm_db_get_name(handle->db_local), - alpm_pkg_get_name(pkg), - alpm_pkg_get_version(pkg)); - return fopen(clfile, "r"); - } - - return(NULL); + return pkg->ops->changelog_open(pkg); } /** @@ -353,26 +309,13 @@ void SYMEXPORT *alpm_pkg_changelog_open(pmpkg_t *pkg) size_t SYMEXPORT alpm_pkg_changelog_read(void *ptr, size_t size, const pmpkg_t *pkg, const void *fp) { - size_t ret = 0; - if(pkg->origin == PKG_FROM_FILE) { - ret = archive_read_data((struct archive*)fp, ptr, size); - } else { - ret = fread(ptr, 1, size, (FILE*)fp); - } - return(ret); + return pkg->ops->changelog_read(ptr, size, pkg, fp); } /* int SYMEXPORT alpm_pkg_changelog_feof(const pmpkg_t *pkg, void *fp) { - int ret = 0; - if(pkg->origin == PKG_FROM_FILE) { - // note: this doesn't quite work, no feof in libarchive - ret = archive_read_data((struct archive*)fp, NULL, 0); - } else { - ret = feof((FILE*)fp); - } - return(ret); + return pkg->ops->changelog_feof(pkg, fp); } */ @@ -386,13 +329,7 @@ int SYMEXPORT alpm_pkg_changelog_feof(const pmpkg_t *pkg, void *fp) */ int SYMEXPORT alpm_pkg_changelog_close(const pmpkg_t *pkg, void *fp) { - int ret = 0; - if(pkg->origin == PKG_FROM_FILE) { - ret = archive_read_finish((struct archive *)fp); - } else { - ret = fclose((FILE*)fp); - } - return(ret); + return pkg->ops->changelog_close(pkg, fp); } unsigned short SYMEXPORT alpm_pkg_has_scriptlet(pmpkg_t *pkg) -- 1.5.5.1
On Mon, May 12, 2008 at 3:28 AM, Dan McGee <dan@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.
On Sun, May 11, 2008 at 08:28:59PM -0500, Dan McGee <dan@archlinux.org> wrote:
@@ -1,8 +1,7 @@ /* * be_files.c * - * Copyright (c) 2006 by Christian Hamar <krics@linuxforum.hu> - * Copyright (c) 2006 by Miklos Vajna <vmiklos@frugalware.org> + * Copyright (c) 2002-2008 by Judd Vinet <jvinet@zeroflux.org> * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by
feel free to do this, but remember that i was even called a GPL violator (by Aaron) just because of i did a similar change in pacman-g2 ;) (or an other example is Git where for example something is rewritten in C from bash the names are not removed.)
On Mon, May 12, 2008 at 5:20 PM, Miklos Vajna <vmiklos@frugalware.org> wrote:
On Sun, May 11, 2008 at 08:28:59PM -0500, Dan McGee <dan@archlinux.org> wrote:
@@ -1,8 +1,7 @@ /* * be_files.c * - * Copyright (c) 2006 by Christian Hamar <krics@linuxforum.hu> - * Copyright (c) 2006 by Miklos Vajna <vmiklos@frugalware.org> + * Copyright (c) 2002-2008 by Judd Vinet <jvinet@zeroflux.org> * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by
feel free to do this, but remember that i was even called a GPL violator (by Aaron) just because of i did a similar change in pacman-g2 ;)
(or an other example is Git where for example something is rewritten in C from bash the names are not removed.)
Oh, so you do still read the list? I was just checking. :) These patches aren't final at all, I actually think I copied things around too much as I have quite a bit of stuff locally that I'm working on. I'm not even close to committing these directly, and I can pretty much see that I'm going to completely rewrite the trio of cache/be_files/db (along with package) and rework just about everything, so that is what I was anticipating. Looks like I jumped the gun. But before you go howling at the copyright police (which is completely different than a GPL violation, so I apologize for that mixup in the past), it sure would be nice if you gave us some credit too when you do port over our commits and work. And our project name is not pacman-g1. http://git.frugalware.org/gitweb/gitweb.cgi?p=pacman-g2.git;a=commitdiff;h=f... http://git.frugalware.org/gitweb/gitweb.cgi?p=pacman-g2.git;a=commitdiff;h=4... (ok, this one is a joke) http://git.frugalware.org/gitweb/gitweb.cgi?p=pacman-g2.git;a=commitdiff;h=0... (you did give credit here, thanks, but still couldn't set the author right?) http://git.frugalware.org/gitweb/gitweb.cgi?p=pacman-g2.git;a=commitdiff;h=9... (nice jab there, pretty mature) -Dan
On Mon, May 12, 2008 at 09:09:00PM -0500, Dan McGee <dpmcgee@gmail.com> wrote:
Oh, so you do still read the list? I was just checking. :)
heh, barely.
But before you go howling at the copyright police (which is completely different than a GPL violation, so I apologize for that mixup in the past), it sure would be nice if you gave us some credit too when you do port over our commits and work. And our project name is not pacman-g1.
ok, i'll try to stop using it, just mplayer was always called as mplayer-g1 on the mplayer-g2 mailing list and i got that bad habit, my bad. (probably "old pacman" is even more arrogant :P)
http://git.frugalware.org/gitweb/gitweb.cgi?p=pacman-g2.git;a=commitdiff;h=f... http://git.frugalware.org/gitweb/gitweb.cgi?p=pacman-g2.git;a=commitdiff;h=4... (ok, this one is a joke) http://git.frugalware.org/gitweb/gitweb.cgi?p=pacman-g2.git;a=commitdiff;h=0... (you did give credit here, thanks, but still couldn't set the author right?)
yup, i think you discovered the --author switch of git-commit way before me! (nowadays i usually use it where appreciate)
http://git.frugalware.org/gitweb/gitweb.cgi?p=pacman-g2.git;a=commitdiff;h=9... (nice jab there, pretty mature)
oh well, different targets. this is just a decision. we try to keep a stable api, you don't. both has benefits. and at the end users can choose. that's the best :)
http://git.frugalware.org/gitweb/gitweb.cgi?p=pacman-g2.git;a=commitdiff;h=9...
(nice jab there, pretty mature)
oh well, different targets. this is just a decision. we try to keep a stable api, you don't. both has benefits. and at the end users can choose. that's the best :)
IIRC, this was reverted, but that is just one more argument to your 'not stable API' "campaign". In my opinion the parameters of callback functions should be reworked (warning, API change ;-). My preferred solution would be putting one param, a pointer to a complicated union or whatever, which can be accessed via alpm_info_get_xfered(ptr) etc. thus making it much more flexible. Thoughts? Bye ---------------------------------------------------- SZTE Egyetemi Könyvtár - http://www.bibl.u-szeged.hu This mail sent through IMP: http://horde.org/imp/
2008/5/14 Nagy Gabor <ngaba@bibl.u-szeged.hu>:
http://git.frugalware.org/gitweb/gitweb.cgi?p=pacman-g2.git;a=commitdiff;h=9...
(nice jab there, pretty mature)
oh well, different targets. this is just a decision. we try to keep a stable api, you don't. both has benefits. and at the end users can choose. that's the best :)
IIRC, this was reverted, but that is just one more argument to your 'not stable API' "campaign".
It doesn't make any sense to stabilize such a crappy API. But just as reminder, the API remains stable between minor releases (3.x.y), but not between major releases (3.x) About the dltotal thing, Dan indeed reverted it, but we need to add that feature back with another way. We currently have this callback : void cb_dl_progress(const char *filename, int xfered, int total) Dan suggested to add another one 2 days ago, I believe it was something like : void cb_dl_total_progress(int xfered, int total) But I already forgot. Dan, can you confirm this? :)
In my opinion the parameters of callback functions should be reworked (warning, API change ;-). My preferred solution would be putting one param, a pointer to a complicated union or whatever, which can be accessed via alpm_info_get_xfered(ptr) etc. thus making it much more flexible. Thoughts?
This might not be a bad idea, any work that would allow us to get rid of these TODO in callback.c is welcome : /* TODO this is one of the worst ever functions written. void *data ? wtf */ /* TODO we take this route based on data2 being not null? WTF */
On Wed, May 14, 2008 at 4:46 AM, Xavier <shiningxc@gmail.com> wrote:
2008/5/14 Nagy Gabor <ngaba@bibl.u-szeged.hu>: About the dltotal thing, Dan indeed reverted it, but we need to add that feature back with another way. We currently have this callback : void cb_dl_progress(const char *filename, int xfered, int total) Dan suggested to add another one 2 days ago, I believe it was something like : void cb_dl_total_progress(int xfered, int total) But I already forgot. Dan, can you confirm this? :)
Yes, this is what I was thinking. The frontend(s) could then use these two functions accordingly. Both would be called numerous times during the download as they are now.
In my opinion the parameters of callback functions should be reworked (warning, API change ;-). My preferred solution would be putting one param, a pointer to a complicated union or whatever, which can be accessed via alpm_info_get_xfered(ptr) etc. thus making it much more flexible. Thoughts?
I don't think this complexity is quite needed for the download functions. Maybe the other functions later on though. Download we are dealing with 4 numbers and a filename. -Dan
On Wed, May 14, 2008 at 7:46 PM, Dan McGee <dpmcgee@gmail.com> wrote:
On Wed, May 14, 2008 at 4:46 AM, Xavier <shiningxc@gmail.com> wrote:
2008/5/14 Nagy Gabor <ngaba@bibl.u-szeged.hu>:
About the dltotal thing, Dan indeed reverted it, but we need to add that feature back with another way. We currently have this callback : void cb_dl_progress(const char *filename, int xfered, int total) Dan suggested to add another one 2 days ago, I believe it was something like : void cb_dl_total_progress(int xfered, int total) But I already forgot. Dan, can you confirm this? :)
Yes, this is what I was thinking. The frontend(s) could then use these two functions accordingly. Both would be called numerous times during the download as they are now.
In my opinion the parameters of callback functions should be reworked (warning, API change ;-). My preferred solution would be putting one param, a pointer to a complicated union or whatever, which can be accessed via alpm_info_get_xfered(ptr) etc. thus making it much more flexible. Thoughts?
I don't think this complexity is quite needed for the download functions. Maybe the other functions later on though. Download we are dealing with 4 numbers and a filename.
Using a struct for this would be a shade more clear. But it's really just smoke-and-mirrors. Whether all that data is a struct or raw params means little.
participants (6)
-
Aaron Griffin
-
Dan McGee
-
Dan McGee
-
Miklos Vajna
-
Nagy Gabor
-
Xavier