[pacman-dev] [PATCH 2/2] Add new --print operation for all operations
Dan McGee
dpmcgee at gmail.com
Sun Nov 15 22:02:34 EST 2009
On Mon, Sep 7, 2009 at 4:43 PM, Xavier Chantry <shiningxc at gmail.com> wrote:
> And a new --print-format option to configure the output.
>
> This implements FS#14208
>
> Signed-off-by: Xavier Chantry <shiningxc at gmail.com>
> ---
> contrib/bash_completion | 9 +++-
> contrib/zsh_completion | 1 +
> doc/pacman.8.txt | 11 ++++
> src/pacman/conf.c | 1 +
> src/pacman/conf.h | 3 +-
> src/pacman/pacman.c | 24 +++++++--
> src/pacman/remove.c | 6 ++
> src/pacman/sync.c | 24 ++-------
> src/pacman/upgrade.c | 8 +++-
> src/pacman/util.c | 120 +++++++++++++++++++++++++++++++++++++++++++----
> src/pacman/util.h | 1 +
> 11 files changed, 171 insertions(+), 37 deletions(-)
>
> diff --git a/contrib/bash_completion b/contrib/bash_completion
> index 65135cf..8fd62d6 100644
> --- a/contrib/bash_completion
> +++ b/contrib/bash_completion
> @@ -167,7 +167,7 @@ _pacman ()
> groups) mod="${mod}g" ;;
> info) mod="${mod}i" ;;
> list) mod="${mod}l" ;;
> - print-uris) mod="${mod}p" ;;
> + print) mod="${mod}p" ;;
> search) mod="${mod}s" ;;
> sysupgrade) mod="${mod}u" ;;
> upgrades) mod="${mod}u" ;;
> @@ -231,6 +231,8 @@ _pacman ()
> -v --verbose \
> -r --root \
> -b --dbpath \
> + -p --print \
> + --print-format \
> --cachedir \
> ' -- $cur ) )
> return 0
> @@ -252,6 +254,8 @@ _pacman ()
> -v --verbose \
> -r --root \
> -b --dbpath \
> + -p --print \
> + --print-format \
> --cachedir \
> ' -- $cur ) )
> return 0
> @@ -267,7 +271,6 @@ _pacman ()
> -h --help \
> -i --info \
> -l --list \
> - -p --print-uris \
> -s --search \
> -u --sysupgrade \
> -w --downloadonly \
> @@ -283,6 +286,8 @@ _pacman ()
> -v --verbose \
> -r --root \
> -b --dbpath \
> + -p --print \
> + --print-format \
> --cachedir \
> ' -- $cur ) )
> return 0
> diff --git a/contrib/zsh_completion b/contrib/zsh_completion
> index 2f43d9b..2ecef35 100644
> --- a/contrib/zsh_completion
> +++ b/contrib/zsh_completion
> @@ -26,6 +26,7 @@ _pacman_opts_common=(
> '--noconfirm[Do not ask for confirmation]'
> '--noprogressbar[Do not show a progress bar when downloading files]'
> '--noscriptlet[Do not execute the install scriptlet if one exists]'
> + '--print[Only print the targets instead of performing the operation]'
> )
>
> # options for passing to _arguments: options for --upgrade commands
> diff --git a/doc/pacman.8.txt b/doc/pacman.8.txt
> index a534e05..e38b909 100644
> --- a/doc/pacman.8.txt
> +++ b/doc/pacman.8.txt
> @@ -169,6 +169,17 @@ Options
> *\--arch* <'arch'>::
> Specify an alternate architecture.
>
> +*\--print*::
> + Only print the targets instead of performing the actual operation (sync,
> + remove or upgrade). Use '\--print-format' to specify how targets are
> + displayed.
> +
> +*\--print-format* <'format'>::
> + Specify a printf-like format to control the output of the '\--print'
> + operation. The possible are attributes are : %n for pkgname, %v for pkgver, %l
> + for location, %r for repo and %s for size. The default format string is "%l",
> + which displays url with '-S', filename with '-U' and pkgname-pkgver with '-R'.
> +
* Isn't --print actually -p/--print?
* Should the default format stuff actually be in the --print
documentation (either as well, or completely move it there?). I feel
like I would look there first.
* You didn't kill the print-uri documentation.
> Query Options[[QO]]
> -------------------
> *-c, \--changelog*::
> diff --git a/src/pacman/conf.c b/src/pacman/conf.c
> index 92c6f4e..a8da49c 100644
> --- a/src/pacman/conf.c
> +++ b/src/pacman/conf.c
> @@ -62,6 +62,7 @@ int config_free(config_t *oldconfig)
> free(oldconfig->dbpath);
> free(oldconfig->logfile);
> free(oldconfig->xfercommand);
> + free(oldconfig->print_format);
> free(oldconfig);
> oldconfig = NULL;
>
> diff --git a/src/pacman/conf.h b/src/pacman/conf.h
> index 2d3de98..192f80f 100644
> --- a/src/pacman/conf.h
> +++ b/src/pacman/conf.h
> @@ -31,6 +31,8 @@ typedef struct __config_t {
> unsigned short noconfirm;
> unsigned short noprogressbar;
> unsigned short logmask;
> + unsigned short print;
> + char *print_format;
> /* unfortunately, we have to keep track of paths both here and in the library
> * because they can come from both the command line or config file, and we
> * need to ensure we get the order of preference right. */
> @@ -59,7 +61,6 @@ typedef struct __config_t {
> unsigned short op_s_sync;
> unsigned short op_s_search;
> unsigned short op_s_upgrade;
> - unsigned short op_s_printuris;
>
> unsigned short group;
> pmtransflag_t flags;
> diff --git a/src/pacman/pacman.c b/src/pacman/pacman.c
> index 3747208..929f745 100644
> --- a/src/pacman/pacman.c
> +++ b/src/pacman/pacman.c
> @@ -128,7 +128,6 @@ static void usage(int op, const char * const myname)
> printf(_(" -g, --groups view all members of a package group\n"));
> printf(_(" -i, --info view package information\n"));
> printf(_(" -l, --list <repo> view a list of packages in a repo\n"));
> - printf(_(" -p, --print-uris print out URIs for given packages and their dependencies\n"));
> printf(_(" -s, --search <regex> search remote repositories for matching strings\n"));
> printf(_(" -u, --sysupgrade upgrade all outdated packages (-uu enables downgrade)\n"));
> printf(_(" -w, --downloadonly download packages but do not install/upgrade anything\n"));
> @@ -139,6 +138,9 @@ static void usage(int op, const char * const myname)
> " ignore a group upgrade (can be used more than once)\n"));
> printf(_(" -q, --quiet show less information for query and search\n"));
> }
> + printf(_(" --print only print the targets instead of performing the operation\n"));
> + printf(_(" --print-format <string>\n"
> + " specify how the targets should be printed\n"));
> printf(_(" --config <path> set an alternate configuration file\n"));
> printf(_(" --logfile <path> set an alternate log file\n"));
> printf(_(" --noconfirm do not ask for any confirmation\n"));
> @@ -366,7 +368,7 @@ static int parseargs(int argc, char *argv[])
> {"nosave", no_argument, 0, 'n'},
> {"owns", no_argument, 0, 'o'},
> {"file", no_argument, 0, 'p'},
> - {"print-uris", no_argument, 0, 'p'},
> + {"print", no_argument, 0, 'p'},
> {"quiet", no_argument, 0, 'q'},
> {"root", required_argument, 0, 'r'},
> {"recursive", no_argument, 0, 's'},
> @@ -391,6 +393,7 @@ static int parseargs(int argc, char *argv[])
> {"needed", no_argument, 0, 1011},
> {"asexplicit", no_argument, 0, 1012},
> {"arch", required_argument, 0, 1013},
> + {"print-format", required_argument, 0, 1014},
> {0, 0, 0, 0}
> };
>
> @@ -468,6 +471,9 @@ static int parseargs(int argc, char *argv[])
> case 1013:
> setarch(optarg);
> break;
> + case 1014:
> + config->print_format = strdup(optarg);
> + 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;
> @@ -503,9 +509,7 @@ static int parseargs(int argc, char *argv[])
> case 'o': config->op_q_owns = 1; break;
> case 'p':
> config->op_q_isfile = 1;
> - config->op_s_printuris = 1;
> - config->flags |= PM_TRANS_FLAG_NOCONFLICTS;
> - config->flags |= PM_TRANS_FLAG_NOLOCK;
> + config->print = 1;
> break;
> case 'q':
> config->quiet = 1;
> @@ -1075,6 +1079,16 @@ int main(int argc, char *argv[])
> alpm_option_set_totaldlcb(cb_dl_total);
> }
>
> + /* set up the print operations */
> + if(config->print) {
> + config->noconfirm = 1;
> + config->flags |= PM_TRANS_FLAG_NOCONFLICTS;
> + config->flags |= PM_TRANS_FLAG_NOLOCK;
> + /* Display only errors */
> + config->logmask &= ~PM_LOG_WARNING;
Man, I hate having to do this. We really should think about a
stderr/stdout jihad.
> + }
> +
> +
> #if defined(HAVE_GETEUID) && !defined(CYGWIN)
> /* check if we have sufficient permission for the requested operation */
> if(myuid > 0 && needs_root()) {
> diff --git a/src/pacman/remove.c b/src/pacman/remove.c
> index b5119fa..06503d3 100644
> --- a/src/pacman/remove.c
> +++ b/src/pacman/remove.c
> @@ -159,6 +159,12 @@ int pacman_remove(alpm_list_t *targets)
> }
>
> /* Step 3: actually perform the removal */
> + if(config->print) { /* --print */
> + print_packages(alpm_trans_get_pkgs());
> + /* we are done */
None of the comments here seem particularly necessary. (/*--print*/,
/*we are done*/). Can you kill them?
> + goto cleanup;
> + }
> +
> if(alpm_trans_commit(NULL) == -1) {
> pm_fprintf(stderr, PM_LOG_ERROR, _("failed to commit transaction (%s)\n"),
> alpm_strerrorlast());
> diff --git a/src/pacman/sync.c b/src/pacman/sync.c
> index 896d028..30638bb 100644
> --- a/src/pacman/sync.c
> +++ b/src/pacman/sync.c
> @@ -697,22 +697,8 @@ static int sync_trans(alpm_list_t *targets)
> }
>
> /* Step 3: actually perform the operation */
> - if(config->op_s_printuris) {
> - /* print uris */
> - alpm_list_t *i;
> - for(i = packages; i; i = alpm_list_next(i)) {
> - pmpkg_t *pkg = alpm_list_getdata(i);
> - pmdb_t *db = alpm_pkg_get_db(pkg);
> - const char *dburl = alpm_db_get_url(db);
> - if(dburl) {
> - printf("%s/%s\n", dburl, alpm_pkg_get_filename(pkg));
> - } else {
> - /* can't use WARNING here, we don't show warnings in -Sp... */
> - pm_fprintf(stderr, PM_LOG_ERROR, _("no database for package: %s\n"),
> - alpm_pkg_get_name(pkg));
> - }
> -
> - }
> + if(config->print) {
> + print_packages(packages);
> /* we are done */
> goto cleanup;
> }
> @@ -785,8 +771,8 @@ int pacman_sync(alpm_list_t *targets)
> {
> alpm_list_t *sync_dbs = NULL;
>
> - /* Display only errors with -Sp and -Sw operations */
> - if((config->flags & PM_TRANS_FLAG_DOWNLOADONLY) || config->op_s_printuris) {
> + /* Display only errors with -Sw operations */
> + if(config->flags & PM_TRANS_FLAG_DOWNLOADONLY) {
Why do we do this for -Sw?
> config->logmask &= ~PM_LOG_WARNING;
> }
>
> @@ -859,7 +845,7 @@ int pacman_sync(alpm_list_t *targets)
> }
>
> alpm_list_t *targs = alpm_list_strdup(targets);
> - if(!(config->flags & PM_TRANS_FLAG_DOWNLOADONLY) && !config->op_s_printuris) {
> + if(!(config->flags & PM_TRANS_FLAG_DOWNLOADONLY) && !config->print) {
> /* check for newer versions of packages to be upgraded first */
> alpm_list_t *packages = syncfirst();
> if(packages) {
> diff --git a/src/pacman/upgrade.c b/src/pacman/upgrade.c
> index e769118..8269b3d 100644
> --- a/src/pacman/upgrade.c
> +++ b/src/pacman/upgrade.c
> @@ -70,7 +70,6 @@ int pacman_upgrade(alpm_list_t *targets)
> }
>
> /* add targets to the created transaction */
> - printf(_("loading package data...\n"));
> for(i = targets; i; i = alpm_list_next(i)) {
> char *targ = alpm_list_getdata(i);
> if(alpm_trans_addtarget(targ) == -1) {
> @@ -143,6 +142,13 @@ int pacman_upgrade(alpm_list_t *targets)
> alpm_list_free(data);
>
> /* Step 3: perform the installation */
> + if(config->print) { /* --print */
> + print_packages(alpm_trans_get_pkgs());
> + /* we are done */
> + trans_release();
> + return(0);
Kill comments
> + }
> +
> if(alpm_trans_commit(NULL) == -1) {
> pm_fprintf(stderr, PM_LOG_ERROR, _("failed to commit transaction (%s)\n"),
> alpm_strerrorlast());
> diff --git a/src/pacman/util.c b/src/pacman/util.c
> index a02b43c..8558302 100644
> --- a/src/pacman/util.c
> +++ b/src/pacman/util.c
> @@ -47,8 +47,15 @@
>
> int trans_init(pmtranstype_t type, pmtransflag_t flags)
> {
> - if(alpm_trans_init(type, flags, cb_trans_evt,
> - cb_trans_conv, cb_trans_progress) == -1) {
> + int ret;
> + if(config->print) {
> + ret = alpm_trans_init(type, flags, NULL, NULL, NULL);
> + } else {
> + ret = alpm_trans_init(type, flags, cb_trans_evt, cb_trans_conv,
> + cb_trans_progress);
> + }
> +
> + if(ret == -1) {
> pm_fprintf(stderr, PM_LOG_ERROR, _("failed to init transaction (%s)\n"),
> alpm_strerrorlast());
> if(pm_errno == PM_ERR_HANDLE_LOCK) {
> @@ -72,13 +79,16 @@ int trans_release(void)
>
> int needs_root(void)
> {
> - if(config->op == PM_OP_UPGRADE || config->op == PM_OP_REMOVE || /* -U, -R */
> - (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 */
> - return(1);
> - } else {
> - return(0);
> + switch(config->op) {
> + case PM_OP_UPGRADE:
> + case PM_OP_REMOVE:
> + return(!config->print);
> + case PM_OP_SYNC:
> + return(config->op_s_clean || config->op_s_sync ||
> + (!config->group && !config->op_s_info && !config->op_q_list &&
> + !config->op_s_search && !config->print));
> + default:
> + return(0);
Aha, much cleaner. I'll assume your logic translation is right. :)
> }
> }
>
> @@ -607,6 +617,98 @@ void display_synctargets(const alpm_list_t *syncpkgs)
> alpm_list_free(rpkglist);
> }
>
> +off_t pkg_get_size(pmpkg_t *pkg)
> +{
> + switch(config->op) {
> + case PM_OP_SYNC:
> + return(alpm_pkg_download_size(pkg));
> + case PM_OP_UPGRADE:
> + return(alpm_pkg_get_size(pkg));
> + default:
> + return(alpm_pkg_get_isize(pkg));
> + }
> +}
> +
> +char *pkg_get_location(pmpkg_t *pkg)
> +{
> + pmdb_t *db;
> + const char *dburl;
> + char *string;
> + switch(config->op) {
> + case PM_OP_SYNC:
> + db = alpm_pkg_get_db(pkg);
> + dburl = alpm_db_get_url(db);
> + if(dburl) {
> + char *pkgurl = NULL;
> + asprintf(&pkgurl, "%s/%s", dburl, alpm_pkg_get_filename(pkg));
> + return(pkgurl);
> + }
> + case PM_OP_UPGRADE:
> + return(strdup(alpm_pkg_get_filename(pkg)));
> + default:
> + string = NULL;
> + asprintf(&string, "%s-%s", alpm_pkg_get_name(pkg), alpm_pkg_get_version(pkg));
> + return(string);
> + }
> +}
> +
> +void print_packages(const alpm_list_t *packages)
> +{
> + const alpm_list_t *i;
> + if(!config->print_format) {
> + config->print_format = strdup("%l");
> + }
> + for(i = packages; i; i = alpm_list_next(i)) {
> + pmpkg_t *pkg = alpm_list_getdata(i);
> + char *string = strdup(config->print_format);
> + char *temp = string;
> + /* %n : pkgname */
> + if(strstr(temp,"%n")) {
> + string = strreplace(temp, "%n", alpm_pkg_get_name(pkg));
> + free(temp);
> + temp = string;
> + }
> + /* %v : pkgver */
> + if(strstr(temp,"%v")) {
> + string = strreplace(temp, "%v", alpm_pkg_get_version(pkg));
> + free(temp);
> + temp = string;
> + }
> + /* %l : location */
> + if(strstr(temp,"%l")) {
> + char *pkgloc = pkg_get_location(pkg);
> + string = strreplace(temp, "%l", pkgloc);
> + free(pkgloc);
> + free(temp);
> + temp = string;
> + }
> + /* %r : repo */
> + if(strstr(temp,"%r")) {
> + const char *repo = "local";
> + pmdb_t *db = alpm_pkg_get_db(pkg);
> + if(db) {
> + repo = alpm_db_get_name(db);
> + }
> + string = strreplace(temp, "%r", repo);
> + free(temp);
> + temp = string;
> + }
> + /* %s : size */
> + if(strstr(temp,"%s")) {
> + char *size;
> + double mbsize = 0.0;
> + mbsize = pkg_get_size(pkg) / (1024.0 * 1024.0);
> + asprintf(&size, "%.2f", mbsize);
> + string = strreplace(temp, "%s", size);
> + free(size);
> + free(temp);
> + temp = string;
> + }
This strreplace() stuff seems a bit naive, considering we could do a
lot of replaces on the string. I'm not sure we need to worry about it
but could be a place for some improvement.
> + printf("%s\n",string);
> + free(string);
> + }
> +}
> +
> /* Helper function for comparing strings using the
> * alpm "compare func" signature */
> int str_cmp(const void *s1, const void *s2)
> diff --git a/src/pacman/util.h b/src/pacman/util.h
> index 1282422..25a8168 100644
> --- a/src/pacman/util.h
> +++ b/src/pacman/util.h
> @@ -57,6 +57,7 @@ void display_targets(const alpm_list_t *pkgs, int install);
> void display_synctargets(const alpm_list_t *syncpkgs);
> void display_new_optdepends(pmpkg_t *oldpkg, pmpkg_t *newpkg);
> void display_optdepends(pmpkg_t *pkg);
> +void print_packages(const alpm_list_t *packages);
> int yesno(char *fmt, ...);
> int noyes(char *fmt, ...);
> int pm_printf(pmloglevel_t level, const char *format, ...) __attribute__((format(printf,2,3)));
> --
> 1.6.4.2
OK, and now for some usability testing.
$ ./src/pacman/pacman -Q
<big list of packages>
...
zope-interface 3.5.1-1
zvbi 0.2.33-1
$ ./src/pacman/pacman -Q --print
error: no targets specified (use -h for help)
So two things here- I expected --print to work and it did not, and we
already have the standard "pkgname <space> pkgver" output that our
default print format uses. Do we want to invent a new one with the
default --print for remove?
Oh wait, does --print even work for -Q? Now I'm all confused. :)
$ ./src/pacman/pacman -Q --print pacman-git
error: package "pacman-git" not found
$ ./src/pacman/pacman -Q pacman-git
pacman-git 20091018-1
More unexpected behavior...
$ ./src/pacman/pacman -Ss --print pacman
core/pacman 3.3.3-1 (base)
A library-based package manager with dependency support
core/pacman-mirrorlist 20090616-1 (base) [installed]
Arch Linux mirror list for use by pacman
extra/namcap 2.4-1 [installed]
A Pacman package analyzer
I wanted a list of URLs for all packages that matched my pacman
search, I did not get that...
-Dan
More information about the pacman-dev
mailing list