[pacman-dev] [PATCH] Introduce -D, --database
Dan McGee
dan at archlinux.org
Wed Sep 9 00:03:33 EDT 2009
On Tue, Sep 8, 2009 at 10:30 PM, Dan McGee<dan at archlinux.org> wrote:
> From: Nagy Gabor <ngaba at bibl.u-szeged.hu>
>
> The request of FS#12950 is implemented.
>
> On the backend side, I introduced a new function, alpm_db_set_pkgreason(),
> to modify the install reason of a package in the local database. On the
> front-end side, I introduced a new main operation, -D/--database, which has
> two options, --asdeps and --asexplicit. I documented this in pacman manual.
> I've created two pactests to test -D: database001.py and database002.py.
>
> Signed-off-by: Nagy Gabor <ngaba at bibl.u-szeged.hu>
I sent this here because I have a few comments, and I'm guessing I
dropped the original submission on the floor a long time ago.
> ---
> doc/pacman.8.txt | 12 +++++-
> lib/libalpm/alpm.h | 16 +++++---
> lib/libalpm/db.c | 38 ++++++++++++++++++
> pactest/tests/database001.py | 11 +++++
> pactest/tests/database002.py | 11 +++++
> src/pacman/Makefile.am | 1 +
> src/pacman/conf.h | 3 +-
> src/pacman/database.c | 90 ++++++++++++++++++++++++++++++++++++++++++
> src/pacman/pacman.c | 22 ++++++++--
> src/pacman/pacman.h | 2 +
> src/pacman/util.c | 2 +-
> 11 files changed, 193 insertions(+), 15 deletions(-)
> create mode 100644 pactest/tests/database001.py
> create mode 100644 pactest/tests/database002.py
> create mode 100644 src/pacman/database.c
>
> diff --git a/doc/pacman.8.txt b/doc/pacman.8.txt
> index 7f92ec7..36b027c 100644
> --- a/doc/pacman.8.txt
> +++ b/doc/pacman.8.txt
> @@ -28,6 +28,12 @@ front ends to be written (for instance, a GUI front end).
>
> Operations
> ----------
> +*-D, \--database*::
> + Modify the package database. This options allows you to modify certain
> + attributes of the installed packages in pacman's database. At the
> + moment, you can only change the install reason. See the '\--asdeps'
> + and '\--asexplicit' options below.
> +
> *-Q, \--query*::
> Query the package database. This operation allows you to view installed
> packages and their files, as well as meta-information about individual
> @@ -101,13 +107,15 @@ Options
> Install packages non-explicitly; in other words, fake their install reason
> to be installed as a dependency. This is useful for makepkg and other
> build from source tools that need to install dependencies before building
> - the package.
> + the package. This option can be also used in combination with '-D' to
> + change the install reason of local packages without reinstalling.
>
> *\--asexplicit*::
> Install packages explicitly; in other words, fake their install reason to
> be explicitly installed. This is useful if you want to mark a dependency
> as explicitly installed so it will not be removed by the '\--recursive'
> - remove operation.
> + remove operation. This option can be also used in combination with '-D'
> + to change the install reason of local packages without reinstalling.
Not so fond of this way of organizing (or disorganizing?) the manpage.
We don't do this anywhere else.
>
> *-b, \--dbpath* <'path'>::
> Specify an alternative database location (a typical default is
> diff --git a/lib/libalpm/alpm.h b/lib/libalpm/alpm.h
> index 661df45..caea28c 100644
> --- a/lib/libalpm/alpm.h
> +++ b/lib/libalpm/alpm.h
> @@ -160,6 +160,15 @@ pmdb_t *alpm_option_get_localdb();
> alpm_list_t *alpm_option_get_syncdbs();
>
> /*
> + * Install reasons -- ie, why the package was installed
> + */
> +
> +typedef enum _pmpkgreason_t {
> + PM_PKG_REASON_EXPLICIT = 0, /* explicitly requested by the user */
> + PM_PKG_REASON_DEPEND = 1 /* installed as a dependency for another package */
> +} pmpkgreason_t;
> +
> +/*
> * Databases
> */
>
> @@ -182,6 +191,7 @@ alpm_list_t *alpm_db_get_pkgcache(pmdb_t *db);
> pmgrp_t *alpm_db_readgrp(pmdb_t *db, const char *name);
> alpm_list_t *alpm_db_get_grpcache(pmdb_t *db);
> alpm_list_t *alpm_db_search(pmdb_t *db, const alpm_list_t* needles);
> +int alpm_db_set_pkgreason(pmdb_t *db, const char *name, pmpkgreason_t reason);
This seems silly to me. Why can't the signature be the standard
int alpm_db_set_pkgreason(pmpkg_t *pkg, pmpkgreason_t reason);
? Just ensure there is a check that it isn't a package file, and it
saves all the lookup junk since that is a public API anyway.
>
> /*
> * Packages
> @@ -189,12 +199,6 @@ alpm_list_t *alpm_db_search(pmdb_t *db, const alpm_list_t* needles);
>
> /* Info parameters */
>
> -/* reasons -- ie, why the package was installed */
> -typedef enum _pmpkgreason_t {
> - PM_PKG_REASON_EXPLICIT = 0, /* explicitly requested by the user */
> - PM_PKG_REASON_DEPEND = 1 /* installed as a dependency for another package */
> -} pmpkgreason_t;
> -
> int alpm_pkg_load(const char *filename, unsigned short full, pmpkg_t **pkg);
> int alpm_pkg_free(pmpkg_t *pkg);
> int alpm_pkg_checkmd5sum(pmpkg_t *pkg);
> diff --git a/lib/libalpm/db.c b/lib/libalpm/db.c
> index 62c2e0a..75c505e 100644
> --- a/lib/libalpm/db.c
> +++ b/lib/libalpm/db.c
> @@ -321,6 +321,44 @@ alpm_list_t SYMEXPORT *alpm_db_search(pmdb_t *db, const alpm_list_t* needles)
> return(_alpm_db_search(db, needles));
> }
>
> +/* Set install reason for a package in db
> + * @param db pointer to the package database
> + * @param name the name of the package
> + * @param reason the new install reason
> + * @return 0 on success, -1 on error (pm_errno is set accordingly)
> + */
> +int SYMEXPORT alpm_db_set_pkgreason(pmdb_t *db, const char *name, pmpkgreason_t reason)
> +{
> + ALPM_LOG_FUNC;
> +
> + /* Sanity checks */
> + ASSERT(handle != NULL, RET_ERR(PM_ERR_HANDLE_NULL, -1));
> + ASSERT(db != NULL && name != NULL, RET_ERR(PM_ERR_WRONG_ARGS, -1));
> +
> + pmpkg_t *pkg = _alpm_db_get_pkgfromcache(db, name);
> + if(pkg == NULL) {
> + RET_ERR(PM_ERR_PKG_NOT_FOUND, -1);
> + }
> +
> + _alpm_log(PM_LOG_DEBUG, "setting install reason %u for %s/%s\n", reason, db->treename, name);
> + /* read DESC */
> + if(_alpm_db_read(db, pkg, INFRQ_DESC)) {
> + return(-1);
> + }
> + if(pkg->reason == reason) {
> + /* we are done */
> + return(0);
> + }
> + /* set reason (in pkgcache) */
> + pkg->reason = reason;
This is scary- manual DESC level reading, direct ->reason accessors-
don't we abstract this all away in package.c?
> + /* write DESC */
> + if(_alpm_db_write(db, pkg, INFRQ_DESC)) {
> + return(-1);
> + }
> +
> + return(0);
> +}
> +
> /** @} */
>
> pmdb_t *_alpm_db_new(const char *dbpath, const char *treename)
> diff --git a/pactest/tests/database001.py b/pactest/tests/database001.py
> new file mode 100644
> index 0000000..de4c040
> --- /dev/null
> +++ b/pactest/tests/database001.py
> @@ -0,0 +1,11 @@
> +self.description = "-D --asdeps"
> +
> +lp = pmpkg("pkg")
> +lp.reason = 0
> +self.addpkg2db("local", lp)
> +
> +self.args = "-D pkg --asdeps"
> +
> +self.addrule("PACMAN_RETCODE=0")
> +self.addrule("PKG_EXIST=pkg")
> +self.addrule("PKG_REASON=pkg|1")
> diff --git a/pactest/tests/database002.py b/pactest/tests/database002.py
> new file mode 100644
> index 0000000..05fa7f4
> --- /dev/null
> +++ b/pactest/tests/database002.py
> @@ -0,0 +1,11 @@
> +self.description = "-D --asexplicit"
> +
> +lp = pmpkg("pkg")
> +lp.reason = 1
> +self.addpkg2db("local", lp)
> +
> +self.args = "-D pkg --asexplicit"
> +
> +self.addrule("PACMAN_RETCODE=0")
> +self.addrule("PKG_EXIST=pkg")
> +self.addrule("PKG_REASON=pkg|0")
> diff --git a/src/pacman/Makefile.am b/src/pacman/Makefile.am
> index 220ee9c..8c14562 100644
> --- a/src/pacman/Makefile.am
> +++ b/src/pacman/Makefile.am
> @@ -24,6 +24,7 @@ endif
>
> pacman_SOURCES = \
> conf.h conf.c \
> + database.c \
> deptest.c \
> package.h package.c \
> pacman.h pacman.c \
> diff --git a/src/pacman/conf.h b/src/pacman/conf.h
> index 3c588a7..0dcda8c 100644
> --- a/src/pacman/conf.h
> +++ b/src/pacman/conf.h
> @@ -85,7 +85,8 @@ enum {
> PM_OP_UPGRADE,
> PM_OP_QUERY,
> PM_OP_SYNC,
> - PM_OP_DEPTEST
> + PM_OP_DEPTEST,
> + PM_OP_DATABASE
> };
>
> /* clean method */
> diff --git a/src/pacman/database.c b/src/pacman/database.c
> new file mode 100644
> index 0000000..0e8e57d
> --- /dev/null
> +++ b/src/pacman/database.c
> @@ -0,0 +1,90 @@
> +/*
> + * database.c
> + *
> + * Copyright (c) 2002-2007 by Judd Vinet <jvinet at 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
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program. If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include "config.h"
> +
> +#include <stdlib.h>
> +#include <stdio.h>
> +#include <string.h>
> +
> +#include <alpm.h>
> +#include <alpm_list.h>
> +
> +/* pacman */
> +#include "pacman.h"
> +#include "conf.h"
> +#include "util.h"
> +
> +extern pmdb_t *db_local;
> +
> +/**
> + * @brief Modify the 'local' package database.
> + *
> + * @param targets a list of packages (as strings) to modify
> + *
> + * @return 0 on success, 1 on failure
> + */
> +int pacman_database(alpm_list_t *targets)
> +{
> + alpm_list_t *i;
> + int retval = 0;
> + pmpkgreason_t reason;
> +
> + if(targets == NULL) {
> + pm_printf(PM_LOG_ERROR, _("no targets specified (use -h for help)\n"));
> + return(1);
> + }
> +
> + if(config->flags & PM_TRANS_FLAG_ALLDEPS) { /* --asdeps */
> + reason = PM_PKG_REASON_DEPEND;
> + } else if(config->flags & PM_TRANS_FLAG_ALLEXPLICIT) { /* --asexplicit */
> + reason = PM_PKG_REASON_EXPLICIT;
> + } else {
> + pm_printf(PM_LOG_ERROR, _("no install reason specified (use -h for help)\n"));
> + return(1);
> + }
> +
> + /* Lock database */
> + if(trans_init(PM_TRANS_TYPE_SYNC, 0) == -1) {
> + return(1);
> + }
> +
> + for(i = targets; i; i = alpm_list_next(i)) {
> + char *pkgname = i->data;
> + if(alpm_db_set_pkgreason(db_local, pkgname, reason) == -1) {
> + pm_printf(PM_LOG_ERROR, _("could not set install reason for package %s (%s)\n"),
> + pkgname, alpm_strerrorlast());
> + retval = 1;
> + } else {
> + if(reason == PM_PKG_REASON_DEPEND) {
> + printf(_("%s: install reason has been set to 'installed as dependency'\n"), pkgname);
> + } else {
> + printf(_("%s: install reason has been set to 'explicitly installed'\n"), pkgname);
> + }
> + }
> + }
> +
> + /* Unlock database */
> + if(trans_release() == -1) {
> + return(1);
> + }
> + return(retval);
> +}
> +
> +/* vim: set ts=2 sw=2 noet: */
> diff --git a/src/pacman/pacman.c b/src/pacman/pacman.c
> index 64598b0..a1051fd 100644
> --- a/src/pacman/pacman.c
> +++ b/src/pacman/pacman.c
> @@ -76,10 +76,11 @@ static void usage(int op, const char * const myname)
> printf(_("operations:\n"));
> printf(" %s {-h --help}\n", myname);
> printf(" %s {-V --version}\n", myname);
> - printf(" %s {-Q --query} [%s] [%s]\n", myname, str_opt, str_pkg);
> - printf(" %s {-R --remove} [%s] <%s>\n", myname, str_opt, str_pkg);
> - printf(" %s {-S --sync} [%s] [%s]\n", myname, str_opt, str_pkg);
> - printf(" %s {-U --upgrade} [%s] <%s>\n", myname, str_opt, str_file);
> + printf(" %s {-D --database} <%s> <%s>\n", myname, str_opt, str_pkg);
> + printf(" %s {-Q --query} [%s] [%s]\n", myname, str_opt, str_pkg);
> + printf(" %s {-R --remove} [%s] <%s>\n", myname, str_opt, str_pkg);
> + printf(" %s {-S --sync} [%s] [%s]\n", myname, str_opt, str_pkg);
> + printf(" %s {-U --upgrade} [%s] <%s>\n", myname, str_opt, str_file);
> printf(_("\nuse '%s {-h --help}' with an operation for available options\n"),
> myname);
> } else {
> @@ -138,6 +139,11 @@ static void usage(int op, const char * const myname)
> printf(_(" --ignoregroup <grp>\n"
> " ignore a group upgrade (can be used more than once)\n"));
> printf(_(" -q, --quiet show less information for query and search\n"));
> + } else if (op == PM_OP_DATABASE) {
> + printf("%s: %s {-D --database} <%s> <%s>\n", str_usg, myname, str_opt, str_pkg);
> + printf("%s:\n", str_opt);
> + printf(_(" --asdeps mark packages as non-explicitly installed\n"));
> + printf(_(" --asexplicit mark packages as explicitly installed\n"));
> }
> printf(_(" --config <path> set an alternate configuration file\n"));
> printf(_(" --logfile <path> set an alternate log file\n"));
> @@ -342,6 +348,8 @@ static int parseargs(int argc, char *argv[])
> int option_index = 0;
> static struct option opts[] =
> {
> +
> + {"database", no_argument, 0, 'D'},
> {"query", no_argument, 0, 'Q'},
> {"remove", no_argument, 0, 'R'},
> {"sync", no_argument, 0, 'S'},
> @@ -395,7 +403,7 @@ static int parseargs(int argc, char *argv[])
> {0, 0, 0, 0}
> };
>
> - while((opt = getopt_long(argc, argv, "RUFQSTr:b:vkhscVfmnoldepqituwygz", opts, &option_index))) {
> + while((opt = getopt_long(argc, argv, "RUDQSTr:b:vkhscVfmnoldepqituwygz", opts, &option_index))) {
> alpm_list_t *list = NULL, *item = NULL; /* lists for splitting strings */
>
> if(opt < 0) {
> @@ -470,6 +478,7 @@ static int parseargs(int argc, char *argv[])
> case 1013:
> setarch(optarg);
> break;
> + case 'D': config->op = (config->op != PM_OP_MAIN ? 0 : PM_OP_DATABASE); break;
> case 'Q': config->op = (config->op != PM_OP_MAIN ? 0 : PM_OP_QUERY); break;
> case 'R': config->op = (config->op != PM_OP_MAIN ? 0 : PM_OP_REMOVE); break;
> case 'S': config->op = (config->op != PM_OP_MAIN ? 0 : PM_OP_SYNC); break;
> @@ -1110,6 +1119,9 @@ int main(int argc, char *argv[])
>
> /* start the requested operation */
> switch(config->op) {
> + case PM_OP_DATABASE:
> + ret = pacman_database(pm_targets);
> + break;
> case PM_OP_REMOVE:
> ret = pacman_remove(pm_targets);
> break;
> diff --git a/src/pacman/pacman.h b/src/pacman/pacman.h
> index d7cb50f..72b0362 100644
> --- a/src/pacman/pacman.h
> +++ b/src/pacman/pacman.h
> @@ -22,6 +22,8 @@
>
> #include <alpm_list.h>
>
> +/* database.c */
> +int pacman_database(alpm_list_t *targets);
> /* query.c */
> int pacman_query(alpm_list_t *targets);
> /* remove.c */
> diff --git a/src/pacman/util.c b/src/pacman/util.c
> index 353aae3..c3b54ff 100644
> --- a/src/pacman/util.c
> +++ b/src/pacman/util.c
> @@ -72,7 +72,7 @@ int trans_release(void)
>
> int needs_root(void)
> {
> - if(config->op == PM_OP_UPGRADE || config->op == PM_OP_REMOVE || /* -U, -R */
> + if(config->op == PM_OP_UPGRADE || config->op == PM_OP_REMOVE || config->op == PM_OP_DATABASE || /* -U, -R, -D */
> (config->op == PM_OP_SYNC && (config->op_s_clean || config->op_s_sync || /* -Sc, -Sy */
> (!config->group && !config->op_s_info && !config->op_q_list /* all other -S combinations, where */
> && !config->op_s_search && !config->op_s_printuris)))) { /* -g, -i, -l, -s, -p is not set */
> --
> 1.6.4.2
I'm going to end up merging this in if the above things get addressed,
but I'm still not feeling great about this top-level flag for some
reason. It seems like it doesn't have much going for it (yet), and
maybe we need to look at some of the flags on the other top-level
options to see if we should move them here. -Rd comes to mind, and
maybe some corresponding "fake that I have a package installed" or
something, I don't know.
-Dan
More information about the pacman-dev
mailing list