Re: [pacman-dev] [PATCH v3 2/2] Add -F --machinereable option

On 10/27/15 at 11:24pm, Florian Pritz wrote:

Signed-off-by: Florian Pritz ---

v3: - Change output format to use \0 as field separator. - Only output one repo for -Fl - Combine else if - Fix incorrect placement of FREELIST(match)

doc/pacman.8.txt | 5 +++++ src/pacman/conf.h | 2 ++ src/pacman/files.c | 46 ++++++++++++++++++++++++++++++++++++++++------ src/pacman/pacman.c | 6 ++++++ 4 files changed, 53 insertions(+), 6 deletions(-)

Aside from a few style nitpicks, the code looks fine. I am curious as to the motivation for this though. Most of our operations are made easier to parse by using --quiet to reduce the output to a single field. Do you have a use case in mind for this that wouldn't be taken care of by adding a --quiet option to -F or intend to add --machinereadable to other operations?

diff --git a/doc/pacman.8.txt b/doc/pacman.8.txt index 57929b4..1df7980 100644 --- a/doc/pacman.8.txt +++ b/doc/pacman.8.txt @@ -477,6 +477,11 @@ The '--list', '--search' and '--owns' options are exclusive and can not be combi *-o <file>, \--owns<file>*:: Search for packages that own a particular file.

+*--machinereadable*:: + Use a machine readable output format for '--list', '--search' and + '--owns'. The format is 'repository\0pkgname\0pkgver\0path\n' with '\0' + being the NULL character and '\n' a linefeed. +

Handling Config Files[[HCF]] ---------------------------- diff --git a/src/pacman/conf.h b/src/pacman/conf.h index cde6741..7a7e9cf 100644 --- a/src/pacman/conf.h +++ b/src/pacman/conf.h @@ -90,6 +90,7 @@ typedef struct __config_t { unsigned short op_s_upgrade;

unsigned short op_f_regex; + unsigned short op_f_machinereadable;

unsigned short group; unsigned short noask; @@ -192,6 +193,7 @@ enum { OP_RECURSIVE, OP_SEARCH, OP_REGEX, + OP_MACHINEREADABLE, OP_UNREQUIRED, OP_UPGRADES, OP_SYSUPGRADE, diff --git a/src/pacman/files.c b/src/pacman/files.c index 31ce9e0..18fd763 100644 --- a/src/pacman/files.c +++ b/src/pacman/files.c @@ -27,6 +27,25 @@ #include "conf.h" #include "package.h"

+static void print_line_machinereadable(alpm_db_t *db, alpm_pkg_t *pkg, char *filename) {

Opening brace on its own line.

+ // Fields are repo, pkgname, pkgver, filename separated with \0

C89-style comment.

+ fputs(alpm_db_get_name(db), stdout); + fputc(0, stdout); + fputs(alpm_pkg_get_name(pkg), stdout); + fputc(0, stdout); + fputs(alpm_pkg_get_version(pkg), stdout); + fputc(0, stdout); + fputs(filename, stdout); + fputs("\n", stdout); +} + +static void dump_pkg_machinereadable(alpm_db_t *db, alpm_pkg_t *pkg) {

Opening brace on its own line.

+ alpm_filelist_t *pkgfiles = alpm_pkg_get_files(pkg); + for(size_t filenum = 0; filenum < pkgfiles->count; filenum++) { + const alpm_file_t *file = pkgfiles->files + filenum; + print_line_machinereadable(db, pkg, file->name); + } +}

static int files_fileowner(alpm_list_t *syncs, alpm_list_t *targets) { int ret = 0; @@ -59,8 +78,9 @@ static int files_fileowner(alpm_list_t *syncs, alpm_list_t *targets) { alpm_filelist_t *files = alpm_pkg_get_files(pkg);

if(alpm_filelist_contains(files, f)) { - - if(!config->quiet) { + if(config->op_f_machinereadable) { + print_line_machinereadable(repo, pkg, f); + } else if(!config->quiet) { printf(_("%s is owned by %s/%s %s\n"), f, alpm_db_get_name(repo), alpm_pkg_get_name(pkg), alpm_pkg_get_version(pkg)); @@ -137,7 +157,13 @@ static int files_search(alpm_list_t *syncs, alpm_list_t *targets, int regex) { }

if(match != NULL) { - if(config->quiet) { + if(config->op_f_machinereadable) { + alpm_list_t *ml; + for(ml = match; ml; ml = alpm_list_next(ml)) { + char *filename = ml->data; + print_line_machinereadable(repo, pkg, filename); + } + } else if(config->quiet) { printf("%s/%s\n", alpm_db_get_name(repo), alpm_pkg_get_name(pkg)); } else { alpm_list_t *ml; @@ -149,8 +175,8 @@ static int files_search(alpm_list_t *syncs, alpm_list_t *targets, int regex) { c = ml->data; printf(" %s\n", c); } - FREELIST(match); } + FREELIST(match); } } } @@ -222,7 +248,11 @@ static int files_list(alpm_list_t *syncs, alpm_list_t *targets) {

if((pkg = alpm_db_get_pkg(db, targ)) != NULL) { found = 1; - dump_file_list(pkg); + if(config->op_f_machinereadable) { + dump_pkg_machinereadable(db, pkg); + } else { + dump_file_list(pkg); + } break; } } @@ -240,7 +270,11 @@ static int files_list(alpm_list_t *syncs, alpm_list_t *targets) {

for(j = alpm_db_get_pkgcache(db); j; j = alpm_list_next(j)) { alpm_pkg_t *pkg = j->data; - dump_file_list(pkg); + if(config->op_f_machinereadable) { + dump_pkg_machinereadable(db, pkg); + } else { + dump_file_list(pkg); + } } } } diff --git a/src/pacman/pacman.c b/src/pacman/pacman.c index d777663..ad90ee6 100644 --- a/src/pacman/pacman.c +++ b/src/pacman/pacman.c @@ -181,6 +181,8 @@ static void usage(int op, const char * const myname) addlist(_(" -x, --regex enable searching using regular expressions\n")); addlist(_(" -y, --refresh download fresh package databases from the server\n" " (-yy to force a refresh even if up to date)\n")); + addlist(_(" --machinereadable\n" + " produce machine-readable output\n")); } switch(op) { case PM_OP_SYNC: @@ -796,6 +798,9 @@ static int parsearg_files(int opt) case 'x': config->op_f_regex = 1; break; + case OP_MACHINEREADABLE: + config->op_f_machinereadable = 1; + break; case OP_QUIET: case 'q': config->quiet = 1; @@ -945,6 +950,7 @@ static int parseargs(int argc, char *argv[]) {"recursive", no_argument, 0, OP_RECURSIVE}, {"search", no_argument, 0, OP_SEARCH}, {"regex", no_argument, 0, OP_REGEX}, + {"machinereadable", no_argument, 0, OP_MACHINEREADABLE}, {"unrequired", no_argument, 0, OP_UNREQUIRED}, {"upgrades", no_argument, 0, OP_UPGRADES}, {"sysupgrade", no_argument, 0, OP_SYSUPGRADE}, -- 2.6.2