[pacman-dev] [PATCH] Quick and dirty change to move sync repos.
Here's the quick-and-dirty version of moving repos to $DBPATH/sync/$REPO. I don't much like it, to be honest - I'd prefer to separate the concept of the local repo from the sync repos entirely, but that would break the current interface into libalpm, as alpm_db_register("local") would, ideally, register a sync repo named "local", and we'd have to add another method like alpm_localdb_register() or something of that sort to register the local database. I could do the less-dirty fix, but I quite imagine the current API is supposed to remain decently stable, so here's one that works with the current setup. From 552b39f5dafdf4e1104ee7f4876f6a1a7115a736 Mon Sep 17 00:00:00 2001 From: Travis Willard <travis@archlinux.org> Date: Fri, 24 Aug 2007 22:28:15 -0400 Subject: [PATCH] Quick and dirty change to move sync repos. This is a quick-n-dirty fix to place all sync repos into the $DBPATH/sync/$REPO path instead of the previous $DBPATH/$REPO - this differentiates sync repos from local repos structurally. Signed-off-by: Travis Willard <travis@archlinux.org> --- lib/libalpm/db.c | 11 ++++++++--- 1 files changed, 8 insertions(+), 3 deletions(-) diff --git a/lib/libalpm/db.c b/lib/libalpm/db.c index 0cb375d..6bc85b7 100644 --- a/lib/libalpm/db.c +++ b/lib/libalpm/db.c @@ -695,7 +695,9 @@ pmdb_t *_alpm_db_register(const char *treename) struct stat buf; pmdb_t *db; const char *dbpath; + char real_dbpath[PATH_MAX]; char path[PATH_MAX]; + char local; ALPM_LOG_FUNC; @@ -704,6 +706,7 @@ pmdb_t *_alpm_db_register(const char *treename) _alpm_log(PM_LOG_WARNING, _("attempt to re-register the 'local' DB")); RET_ERR(PM_ERR_DB_NOT_NULL, NULL); } + local = 1; } else { alpm_list_t *i; for(i = handle->dbs_sync; i; i = i->next) { @@ -713,6 +716,7 @@ pmdb_t *_alpm_db_register(const char *treename) return sdb; } } + local = 0; } _alpm_log(PM_LOG_DEBUG, "registering database '%s'", treename); @@ -723,7 +727,8 @@ pmdb_t *_alpm_db_register(const char *treename) _alpm_log(PM_LOG_WARNING, _("database path is undefined")); RET_ERR(PM_ERR_DB_OPEN, NULL); } - snprintf(path, PATH_MAX, "%s%s", dbpath, treename); + snprintf(real_dbpath, PATH_MAX, "%s%s", dbpath, local? "":"sync/"); + snprintf(path, PATH_MAX, "%s%s", real_dbpath, treename); /* TODO this is rediculous, we try to do this even if we can't */ if(stat(path, &buf) != 0 || !S_ISDIR(buf.st_mode)) { _alpm_log(PM_LOG_DEBUG, "database dir '%s' does not exist, creating it", @@ -733,7 +738,7 @@ pmdb_t *_alpm_db_register(const char *treename) } } - db = _alpm_db_new(dbpath, treename); + db = _alpm_db_new(real_dbpath, treename); if(db == NULL) { RET_ERR(PM_ERR_DB_CREATE, NULL); } @@ -744,7 +749,7 @@ pmdb_t *_alpm_db_register(const char *treename) RET_ERR(PM_ERR_DB_OPEN, NULL); } - if(strcmp(treename, "local") == 0) { + if(local) { handle->db_local = db; } else { handle->dbs_sync = alpm_list_add(handle->dbs_sync, db); -- 1.5.2.5
On Fri, Aug 24, 2007 at 10:40:41PM -0400, Travis Willard wrote:
I don't much like it, to be honest - I'd prefer to separate the concept of the local repo from the sync repos entirely, but that would break the current interface into libalpm, as alpm_db_register("local") would, ideally, register a sync repo named "local", and we'd have to add another method like alpm_localdb_register() or something of that sort to register the local database.
I could do the less-dirty fix, but I quite imagine the current API is supposed to remain decently stable, so here's one that works with the current setup.
I'm not aware of any projects using it. Which ones are there ? Imo it's better to try improving the interface now than waiting :) Since both registering and unregistering the local databases require a special handling already in these functions : if (strcmp(name, "local") == 0) { blabla } I think it might be a good idea to split them, if it doesn't duplicate code too much.
On Fri, 24 Aug 2007 22:40:41 -0400 Travis Willard <travis@archlinux.org> wrote:
Here's the quick-and-dirty version of moving repos to $DBPATH/sync/$REPO.
I don't much like it, to be honest - I'd prefer to separate the concept of the local repo from the sync repos entirely
And here's my patch to do this. Read the comment for details - I think I was specific enough there. From 7d9c73eda546195a0a207be8b264219184929712 Mon Sep 17 00:00:00 2001 From: Travis Willard <travis@archlinux.org> Date: Sun, 26 Aug 2007 12:00:03 -0400 Subject: [PATCH] Clean way to separate local from sync dbs. Introduce two new methods into the API - alpm_syncdb_register and alpm_localdb_register, which replace the functionality of alpm_db_register. which has now been flagged as deprecated. localdb_register always returns the local DB, and syncdb_register will always try to register a sync DB. This removes the silly limitation that a sync DB couldn't be named 'local', along with structurally separating sync DBs and the local DB in the filesystem. Also updated the pacman frontend to use the new functions. Signed-off-by: Travis Willard <travis@archlinux.org> --- lib/libalpm/alpm.h | 8 ++++++- lib/libalpm/db.c | 56 +++++++++++++++++++++++++++++++++++++++++++------- lib/libalpm/db.h | 9 +++++++- src/pacman/pacman.c | 10 +------- 4 files changed, 65 insertions(+), 18 deletions(-) diff --git a/lib/libalpm/alpm.h b/lib/libalpm/alpm.h index 844d9bf..74d6102 100644 --- a/lib/libalpm/alpm.h +++ b/lib/libalpm/alpm.h @@ -31,6 +31,8 @@ extern "C" { #include <time.h> /* for time_t */ #include <stdarg.h> /* for va_list */ +#define DEPRECATED __attribute__((deprecated)) + /* * Arch Linux Package Management library */ @@ -143,7 +145,11 @@ alpm_list_t *alpm_option_get_syncdbs(); * Databases */ -pmdb_t *alpm_db_register(const char *treename); +/* Deprecated interface alpm_db_register */ +pmdb_t DEPRECATED *alpm_db_register(const char *treename); +/* Preferred interfaces localdb_register and syncdb_register */ +pmdb_t *alpm_localdb_register(); +pmdb_t *alpm_syncdb_register(const char *treename); int alpm_db_unregister(pmdb_t *db); int alpm_db_unregister_all(void); diff --git a/lib/libalpm/db.c b/lib/libalpm/db.c index 6bc85b7..dac1959 100644 --- a/lib/libalpm/db.c +++ b/lib/libalpm/db.c @@ -53,8 +53,8 @@ * @{ */ -/** Register a package database - * @param treename the name of the repository +/** Register a package database (deprecated) + * @param treename the name of the sync repository or "local" for the local DB * @return a pmdb_t* on success (the value), NULL on error */ pmdb_t SYMEXPORT *alpm_db_register(const char *treename) @@ -67,9 +67,42 @@ pmdb_t SYMEXPORT *alpm_db_register(const char *treename) /* Do not register a database if a transaction is on-going */ ASSERT(handle->trans == NULL, RET_ERR(PM_ERR_TRANS_NOT_NULL, NULL)); - return(_alpm_db_register(treename)); + return(_alpm_db_register(treename, DB_REGISTER_OLD_BEHAVIOUR)); } +/** Register a sync-database of packages. + * @param treename the name of the sync repository + * @return a pmdb_t* on success (the value), NULL on error + */ +pmdb_t SYMEXPORT *alpm_syncdb_register(const char *treename) +{ + ALPM_LOG_FUNC; + + /* Sanity checks */ + ASSERT(handle != NULL, RET_ERR(PM_ERR_HANDLE_NULL, NULL)); + ASSERT(treename != NULL && strlen(treename) != 0, RET_ERR(PM_ERR_WRONG_ARGS, NULL)); + /* Do not register a database if a transaction is on-going */ + ASSERT(handle->trans == NULL, RET_ERR(PM_ERR_TRANS_NOT_NULL, NULL)); + + return(_alpm_db_register(treename, DB_REGISTER_SYNC_DB)); +} + +/** Register the local package database + * @return a pmdb_t* representing the local database, or NULL on error + */ +pmdb_t SYMEXPORT *alpm_localdb_register() +{ + ALPM_LOG_FUNC; + + /* Sanity checks */ + ASSERT(handle != NULL, RET_ERR(PM_ERR_HANDLE_NULL, NULL)); + /* Do not register a database if a transaction is on-going */ + ASSERT(handle->trans == NULL, RET_ERR(PM_ERR_TRANS_NOT_NULL, NULL)); + + return(_alpm_db_register("local", DB_REGISTER_LOCAL_DB)); +} + + /* Helper function for alpm_db_unregister{_all} */ static void _alpm_db_unregister(pmdb_t *db) { @@ -690,7 +723,7 @@ alpm_list_t *_alpm_db_search(pmdb_t *db, const alpm_list_t *needles) return(ret); } -pmdb_t *_alpm_db_register(const char *treename) +pmdb_t *_alpm_db_register(const char *treename, pmdbregtype_t type) { struct stat buf; pmdb_t *db; @@ -701,10 +734,16 @@ pmdb_t *_alpm_db_register(const char *treename) ALPM_LOG_FUNC; - if(strcmp(treename, "local") == 0) { + if(type == DB_REGISTER_LOCAL_DB || + (type == DB_REGISTER_OLD_BEHAVIOUR && strcmp(treename, "local") == 0)) { + if(handle->db_local != NULL) { - _alpm_log(PM_LOG_WARNING, _("attempt to re-register the 'local' DB")); - RET_ERR(PM_ERR_DB_NOT_NULL, NULL); + if (type == DB_REGISTER_OLD_BEHAVIOUR) { + _alpm_log(PM_LOG_WARNING, _("attempt to re-register the 'local' DB")); + RET_ERR(PM_ERR_DB_NOT_NULL, NULL); + } else { + return handle->db_local; + } } local = 1; } else { @@ -717,9 +756,10 @@ pmdb_t *_alpm_db_register(const char *treename) } } local = 0; + } - _alpm_log(PM_LOG_DEBUG, "registering database '%s'", treename); + _alpm_log(PM_LOG_DEBUG, "registering %s database '%s'", local?"local":"sync", treename); /* make sure the database directory exists */ dbpath = alpm_option_get_dbpath(); diff --git a/lib/libalpm/db.h b/lib/libalpm/db.h index 2597c8c..be895f2 100644 --- a/lib/libalpm/db.h +++ b/lib/libalpm/db.h @@ -37,6 +37,13 @@ typedef enum _pmdbinfrq_t { INFRQ_ALL = 0x1F } pmdbinfrq_t; +/* Register functionality. */ +typedef enum _pmdbregtype_t { + DB_REGISTER_OLD_BEHAVIOUR = 0x1, + DB_REGISTER_SYNC_DB = 0x2, + DB_REGISTER_LOCAL_DB = 0x3 +} pmdbregtype_t; + /* Database */ struct __pmdb_t { char *path; @@ -52,7 +59,7 @@ pmdb_t *_alpm_db_new(const char *dbpath, const char *treename); void _alpm_db_free(pmdb_t *db); int _alpm_db_cmp(const void *db1, const void *db2); alpm_list_t *_alpm_db_search(pmdb_t *db, const alpm_list_t *needles); -pmdb_t *_alpm_db_register(const char *treename); +pmdb_t *_alpm_db_register(const char *treename, pmdbregtype_t type); /* be.c, backend specific calls */ int _alpm_db_install(pmdb_t *db, const char *dbfile); diff --git a/src/pacman/pacman.c b/src/pacman/pacman.c index 31302ab..ac4f6c5 100644 --- a/src/pacman/pacman.c +++ b/src/pacman/pacman.c @@ -495,15 +495,9 @@ static int _parseconfig(const char *file, const char *givensection, file, linenum); return(1); } - /* a section/database named local is not allowed */ - if(!strcmp(section, "local")) { - pm_printf(PM_LOG_ERROR, _("config file %s, line %d: 'local' cannot be used as section name.\n"), - file, linenum); - return(1); - } /* if we are not looking at the options section, register a db */ if(strcmp(section, "options") != 0) { - db = alpm_db_register(section); + db = alpm_syncdb_register(section); } } else { /* directive */ @@ -811,7 +805,7 @@ int main(int argc, char *argv[]) } /* Opening local database */ - db_local = alpm_db_register("local"); + db_local = alpm_localdb_register(); if(db_local == NULL) { pm_printf(PM_LOG_ERROR, _("could not register 'local' database (%s)\n"), alpm_strerror(pm_errno)); -- 1.5.2.5
On 8/26/07, Travis Willard <travis@archlinux.org> wrote:
On Fri, 24 Aug 2007 22:40:41 -0400 Travis Willard <travis@archlinux.org> wrote:
Here's the quick-and-dirty version of moving repos to $DBPATH/sync/$REPO.
I don't much like it, to be honest - I'd prefer to separate the concept of the local repo from the sync repos entirely
And here's my patch to do this. Read the comment for details - I think I was specific enough there.
From 7d9c73eda546195a0a207be8b264219184929712 Mon Sep 17 00:00:00 2001 From: Travis Willard <travis@archlinux.org> Date: Sun, 26 Aug 2007 12:00:03 -0400 Subject: [PATCH] Clean way to separate local from sync dbs.
Introduce two new methods into the API - alpm_syncdb_register and alpm_localdb_register, which replace the functionality of alpm_db_register. which has now been flagged as deprecated. localdb_register always returns the local DB, and syncdb_register will always try to register a sync DB. This removes the silly limitation that a sync DB couldn't be named 'local', along with structurally separating sync DBs and the local DB in the filesystem. Also updated the pacman frontend to use the new functions.
Signed-off-by: Travis Willard <travis@archlinux.org> ---
I like most of this here, but the patch got nice and malformed so I can't really take a look at it on my local code tree. Can you resend it as an attachment perhaps? And taking a quick look at it, there are a few small things that you might want to change. The naming prefix of alpm_db should probably still remain instead of changing it to alpm_localdb (maybe alpm_db_register_local and alpm_db_register_sync?). -Dan
Introduce two new methods into the API - alpm_db_register_sync and alpm_db_register_local, which replace the functionality of alpm_db_register. which has now been flagged as deprecated. db_register_local always returns the local DB, and db_register_sync will always try to register a sync DB. This conceptually separates the local DB from sync DBs in the code. Also updated the pacman frontend to use the new functions. In addition, this changes the location of all sync DBs in the filesystem from $DBPATH/$REPO to $DBPATH/sync/$REPO, This removes the silly limitation that a sync DB couldn't be named 'local', along with structurally separating sync DBs and the local DB in the filesystem. Signed-off-by: Travis Willard <travis@archlinux.org> --- lib/libalpm/alpm.h | 8 +++++- lib/libalpm/db.c | 67 ++++++++++++++++++++++++++++++++++++++++++-------- lib/libalpm/db.h | 9 ++++++- src/pacman/pacman.c | 10 +------ 4 files changed, 73 insertions(+), 21 deletions(-)
participants (3)
-
Dan McGee
-
Travis Willard
-
Xavier