[pacman-dev] [PATCH 2/2] New feature: files verification
From: Charly COSTE <changaco@laposte.net> A new option "-Qk" which checks if all packages files are really on the system (i.e. not accidentally deleted). This implements FS#13877 Signed-off-by: Charly COSTE <changaco@laposte.net> [Xav : don't ignore tmp, don't repeat package names in quiet output, add errno message in verbose output] Signed-off-by: Xavier Chantry <shiningxc@gmail.com> --- doc/pacman.8.txt | 4 ++ src/pacman/conf.h | 1 + src/pacman/pacman.c | 7 +++- src/pacman/query.c | 114 +++++++++++++++++++++++++++++++++++++++++++++++--- 4 files changed, 118 insertions(+), 8 deletions(-) diff --git a/doc/pacman.8.txt b/doc/pacman.8.txt index af85a15..ccff167 100644 --- a/doc/pacman.8.txt +++ b/doc/pacman.8.txt @@ -196,6 +196,10 @@ Query Options[[QO]] '\--info' or '-i' flags will also display the list of backup files and their modification states. +*-k \--check*:: + Check that all files owned by the given package(s) are present on the + system. If packages are not specified, check all installed packages. + *-l, \--list*:: List all files owned by a given package. Multiple packages can be specified on the command line. diff --git a/src/pacman/conf.h b/src/pacman/conf.h index 39802ca..6523d49 100644 --- a/src/pacman/conf.h +++ b/src/pacman/conf.h @@ -51,6 +51,7 @@ typedef struct __config_t { unsigned short op_q_search; unsigned short op_q_changelog; unsigned short op_q_upgrade; + unsigned short op_q_check; unsigned short op_s_clean; unsigned short op_s_downloadonly; diff --git a/src/pacman/pacman.c b/src/pacman/pacman.c index 7f86489..48d45ad 100644 --- a/src/pacman/pacman.c +++ b/src/pacman/pacman.c @@ -108,6 +108,7 @@ static void usage(int op, const char * const myname) printf(_(" -e, --explicit list packages explicitly installed [filter]\n")); printf(_(" -g, --groups view all members of a package group\n")); printf(_(" -i, --info view package information (-ii for backup files)\n")); + printf(_(" -k, --check check that the files owned by the package(s) are present\n")); printf(_(" -l, --list list the contents of the queried package\n")); printf(_(" -m, --foreign list installed packages not found in sync db(s) [filter]\n")); printf(_(" -o, --owns <file> query the package that owns <file>\n")); @@ -345,6 +346,7 @@ static int parseargs(int argc, char *argv[]) {"help", no_argument, 0, 'h'}, {"info", no_argument, 0, 'i'}, {"dbonly", no_argument, 0, 'k'}, + {"check", no_argument, 0, 'k'}, {"list", no_argument, 0, 'l'}, {"foreign", no_argument, 0, 'm'}, {"nosave", no_argument, 0, 'n'}, @@ -473,7 +475,10 @@ static int parseargs(int argc, char *argv[]) case 'g': (config->group)++; break; case 'h': config->help = 1; break; case 'i': (config->op_q_info)++; (config->op_s_info)++; break; - case 'k': config->flags |= PM_TRANS_FLAG_DBONLY; break; + case 'k': + config->flags |= PM_TRANS_FLAG_DBONLY; + config->op_q_check = 1; + break; case 'l': config->op_q_list = 1; break; case 'm': config->op_q_foreign = 1; break; case 'n': config->flags |= PM_TRANS_FLAG_NOSAVE; break; diff --git a/src/pacman/query.c b/src/pacman/query.c index 4997202..62eca53 100644 --- a/src/pacman/query.c +++ b/src/pacman/query.c @@ -334,10 +334,94 @@ static void display(pmpkg_t *pkg) } } +static void check_display_progress(int j, int pkgs_n, const char *pkgname) +{ + char message[getcols()]; + int len = snprintf(message, getcols(), "(%i/%i) %s %s ...", (int)j, (int)pkgs_n, _("checking"), pkgname); + fprintf(stderr, "\r%s %*s", message, getcols()-len-1, " "); + fflush(stderr); +} + +/* Loop through the packages. For each package, + * loop through files to check if they exist. */ +static void check(alpm_list_t *pkgs) +{ + alpm_list_t *i, *files, *file, *damaged = NULL; + pmpkg_t *pkg; + const char *root; + int pkgs_n, j = 0, allfiles = 0; + size_t rootlen; + char f[PATH_MAX]; + + pkgs_n = alpm_list_count(pkgs); + root = alpm_option_get_root(); + rootlen = strlen(root); + if(rootlen + 1 > PATH_MAX) { + /* we are in trouble here */ + pm_printf(PM_LOG_ERROR, _("root path too long\n")); + return; + } + strcpy(f, root); + + for(i = pkgs; i; i = alpm_list_next(i)) { + j++; + pkg = alpm_list_getdata(i); + const char *pkgname = alpm_pkg_get_name(pkg); + files = alpm_pkg_get_files(pkg); + if(!config->quiet) { + check_display_progress(j, pkgs_n, pkgname); + } + for(file = files; file; file = alpm_list_next(file)){ + struct stat st; + char *x = alpm_list_getdata(file); + if(rootlen + 1 + strlen(x) > PATH_MAX) { + pm_printf(PM_LOG_WARNING, _("file path too long\n")); + continue; + } + strcpy(f + rootlen, x); + allfiles++; + /* use lstat to prevent errors from symlinks */ + if(lstat(f,&st) != 0) { + if(config->quiet) { + fprintf(stderr, "%s %s\n", pkgname, f); + fflush(stderr); + } else { + fprintf(stderr, "\r%s: %s (%s)\n", pkgname, f, strerror(errno)); + check_display_progress(j, pkgs_n, pkgname); + fflush(stderr); + } + if(alpm_list_find_ptr(damaged, pkgname) == NULL) { + damaged = alpm_list_add(damaged, (char*)pkgname); + } + } + } + } + if(!config->quiet) { + char message[getcols()]; + int len = snprintf(message, getcols(), "%s: %i %s (%i %s)", + _("Check complete"), allfiles, _("files"), pkgs_n, _("packages")); + fprintf(stderr, "\r%s %*s\n", message, getcols()-len-1, " "); + fflush(stderr); + + if(alpm_list_count(damaged) > 0) { + fprintf(stderr, "\r%s ", _("Damaged packages:")); + fflush(stderr); + for(i = damaged; i; i = alpm_list_next(i)) { + fprintf(stdout, "%s ", (char*)alpm_list_getdata(i)); + } + fprintf(stdout, "\n"); + fflush(stdout); + } + } + + alpm_list_free(damaged); +} + int pacman_query(alpm_list_t *targets) { int ret = 0; - alpm_list_t *i; + alpm_list_t *i, *pkgs = NULL; + pmpkg_t *pkg = NULL; /* First: operations that do not require targets */ @@ -363,7 +447,7 @@ int pacman_query(alpm_list_t *targets) } /* operations on all packages in the local DB - * valid: no-op (plain -Q), list, info + * valid: no-op (plain -Q), list, info, check * invalid: isfile, owns */ if(targets == NULL) { if(config->op_q_isfile || config->op_q_owns) { @@ -372,11 +456,19 @@ int pacman_query(alpm_list_t *targets) } for(i = alpm_db_get_pkgcache(db_local); i; i = alpm_list_next(i)) { - pmpkg_t *pkg = alpm_list_getdata(i); + pkg = alpm_list_getdata(i); if(filter(pkg)) { - display(pkg); + if(config->op_q_check) { + pkgs = alpm_list_add(pkgs, pkg); + } else { + display(pkg); + } } } + if(config->op_q_check){ + check(pkgs); + alpm_list_free(pkgs); + } return(0); } @@ -389,10 +481,9 @@ int pacman_query(alpm_list_t *targets) } /* operations on named packages in the local DB - * valid: no-op (plain -Q), list, info */ + * valid: no-op (plain -Q), list, info, check */ for(i = targets; i; i = alpm_list_next(i)) { char *strname = alpm_list_getdata(i); - pmpkg_t *pkg = NULL; if(config->op_q_isfile) { alpm_pkg_load(strname, 1, &pkg); @@ -406,8 +497,13 @@ int pacman_query(alpm_list_t *targets) continue; } + if(filter(pkg)) { - display(pkg); + if(config->op_q_check) { + pkgs = alpm_list_add(pkgs, pkg); + } else { + display(pkg); + } } if(config->op_q_isfile) { @@ -415,6 +511,10 @@ int pacman_query(alpm_list_t *targets) pkg = NULL; } } + if(config->op_q_check){ + check(pkgs); + alpm_list_free(pkgs); + } return(ret); } -- 1.6.3.3
On Sat, Jul 18, 2009 at 8:09 PM, Xavier Chantry<shiningxc@gmail.com> wrote:
From: Charly COSTE <changaco@laposte.net>
A new option "-Qk" which checks if all packages files are really on the system (i.e. not accidentally deleted).
This implements FS#13877
Signed-off-by: Charly COSTE <changaco@laposte.net> [Xav : don't ignore tmp, don't repeat package names in quiet output, add errno message in verbose output] Signed-off-by: Xavier Chantry <shiningxc@gmail.com> ---
$ ./src/pacman/pacman -Qk at: /var/spool/atd/.SEQ (Permission denied) ttf-ms-fonts: /tmp/ttf-ms-fonts/ (No such file or directory) ttf-ms-fonts: /tmp/ttf-ms-fonts/andale32.exe (No such file or directory) ttf-ms-fonts: /tmp/ttf-ms-fonts/arial32.exe (No such file or directory) ttf-ms-fonts: /tmp/ttf-ms-fonts/arialb32.exe (No such file or directory) ttf-ms-fonts: /tmp/ttf-ms-fonts/comic32.exe (No such file or directory) ttf-ms-fonts: /tmp/ttf-ms-fonts/courie32.exe (No such file or directory) ttf-ms-fonts: /tmp/ttf-ms-fonts/georgi32.exe (No such file or directory) ttf-ms-fonts: /tmp/ttf-ms-fonts/impact32.exe (No such file or directory) ttf-ms-fonts: /tmp/ttf-ms-fonts/times32.exe (No such file or directory) ttf-ms-fonts: /tmp/ttf-ms-fonts/trebuc32.exe (No such file or directory) ttf-ms-fonts: /tmp/ttf-ms-fonts/verdan32.exe (No such file or directory) ttf-ms-fonts: /tmp/ttf-ms-fonts/webdin32.exe (No such file or directory) Check complete: 151945 files (609 packages) Damaged packages: at ttf-ms-fonts should we just skip the file when the permission is denied? when errno == EACCES "at" is wrongly reported as damaged.
On Sat, Jul 18, 2009 at 21:12, Xavier<shiningxc@gmail.com> wrote:
On Sat, Jul 18, 2009 at 8:09 PM, Xavier Chantry<shiningxc@gmail.com> wrote:
From: Charly COSTE <changaco@laposte.net>
A new option "-Qk" which checks if all packages files are really on the system (i.e. not accidentally deleted).
This implements FS#13877
Signed-off-by: Charly COSTE <changaco@laposte.net> [Xav : don't ignore tmp, don't repeat package names in quiet output, add errno message in verbose output] Signed-off-by: Xavier Chantry <shiningxc@gmail.com> ---
$ ./src/pacman/pacman -Qk at: /var/spool/atd/.SEQ (Permission denied) ttf-ms-fonts: /tmp/ttf-ms-fonts/ (No such file or directory) ttf-ms-fonts: /tmp/ttf-ms-fonts/andale32.exe (No such file or directory) ttf-ms-fonts: /tmp/ttf-ms-fonts/arial32.exe (No such file or directory) ttf-ms-fonts: /tmp/ttf-ms-fonts/arialb32.exe (No such file or directory) ttf-ms-fonts: /tmp/ttf-ms-fonts/comic32.exe (No such file or directory) ttf-ms-fonts: /tmp/ttf-ms-fonts/courie32.exe (No such file or directory) ttf-ms-fonts: /tmp/ttf-ms-fonts/georgi32.exe (No such file or directory) ttf-ms-fonts: /tmp/ttf-ms-fonts/impact32.exe (No such file or directory) ttf-ms-fonts: /tmp/ttf-ms-fonts/times32.exe (No such file or directory) ttf-ms-fonts: /tmp/ttf-ms-fonts/trebuc32.exe (No such file or directory) ttf-ms-fonts: /tmp/ttf-ms-fonts/verdan32.exe (No such file or directory) ttf-ms-fonts: /tmp/ttf-ms-fonts/webdin32.exe (No such file or directory) Check complete: 151945 files (609 packages) Damaged packages: at ttf-ms-fonts
should we just skip the file when the permission is denied? when errno == EACCES "at" is wrongly reported as damaged.
Wouldn't it be nice if it also checked if file permissios are the same as in package? -- Roman Kyrylych (Роман Кирилич)
On Sat, Jul 18, 2009 at 9:05 PM, Roman Kyrylych<roman.kyrylych@gmail.com> wrote:
should we just skip the file when the permission is denied? when errno == EACCES "at" is wrongly reported as damaged.
Wouldn't it be nice if it also checked if file permissios are the same as in package?
We don't have this information, it is not in the database.
changaco wrote:
Xavier wrote :
$ ./src/pacman/pacman -Qk at: /var/spool/atd/.SEQ (Permission denied) ttf-ms-fonts: /tmp/ttf-ms-fonts/ (No such file or directory) ttf-ms-fonts: /tmp/ttf-ms-fonts/andale32.exe (No such file or directory) ttf-ms-fonts: /tmp/ttf-ms-fonts/arial32.exe (No such file or directory) ttf-ms-fonts: /tmp/ttf-ms-fonts/arialb32.exe (No such file or directory) ttf-ms-fonts: /tmp/ttf-ms-fonts/comic32.exe (No such file or directory) ttf-ms-fonts: /tmp/ttf-ms-fonts/courie32.exe (No such file or directory) ttf-ms-fonts: /tmp/ttf-ms-fonts/georgi32.exe (No such file or directory) ttf-ms-fonts: /tmp/ttf-ms-fonts/impact32.exe (No such file or directory) ttf-ms-fonts: /tmp/ttf-ms-fonts/times32.exe (No such file or directory) ttf-ms-fonts: /tmp/ttf-ms-fonts/trebuc32.exe (No such file or directory) ttf-ms-fonts: /tmp/ttf-ms-fonts/verdan32.exe (No such file or directory) ttf-ms-fonts: /tmp/ttf-ms-fonts/webdin32.exe (No such file or directory) Check complete: 151945 files (609 packages) Damaged packages: at ttf-ms-fonts
should we just skip the file when the permission is denied? when errno == EACCES "at" is wrongly reported as damaged.
If you ignore the permission denied you will ignore missing files that are in protected directories. -Qk should be run as root.
And I still don't get why you absolutely want to search in /tmp, it makes the return value of -Qk totally irrelevant in scripts.
Not really... It just highligths that the ttf-ms-fonts package is crap. I think it should just store those files in /usr/share/ms-fonts. Allan
Xavier wrote :
$ ./src/pacman/pacman -Qk at: /var/spool/atd/.SEQ (Permission denied) ttf-ms-fonts: /tmp/ttf-ms-fonts/ (No such file or directory) ttf-ms-fonts: /tmp/ttf-ms-fonts/andale32.exe (No such file or directory) ttf-ms-fonts: /tmp/ttf-ms-fonts/arial32.exe (No such file or directory) ttf-ms-fonts: /tmp/ttf-ms-fonts/arialb32.exe (No such file or directory) ttf-ms-fonts: /tmp/ttf-ms-fonts/comic32.exe (No such file or directory) ttf-ms-fonts: /tmp/ttf-ms-fonts/courie32.exe (No such file or directory) ttf-ms-fonts: /tmp/ttf-ms-fonts/georgi32.exe (No such file or directory) ttf-ms-fonts: /tmp/ttf-ms-fonts/impact32.exe (No such file or directory) ttf-ms-fonts: /tmp/ttf-ms-fonts/times32.exe (No such file or directory) ttf-ms-fonts: /tmp/ttf-ms-fonts/trebuc32.exe (No such file or directory) ttf-ms-fonts: /tmp/ttf-ms-fonts/verdan32.exe (No such file or directory) ttf-ms-fonts: /tmp/ttf-ms-fonts/webdin32.exe (No such file or directory) Check complete: 151945 files (609 packages) Damaged packages: at ttf-ms-fonts
should we just skip the file when the permission is denied? when errno == EACCES "at" is wrongly reported as damaged.
If you ignore the permission denied you will ignore missing files that are in protected directories. -Qk should be run as root. And I still don't get why you absolutely want to search in /tmp, it makes the return value of -Qk totally irrelevant in scripts.
On Sun, Jul 19, 2009 at 1:13 PM, changaco<changaco@gmail.com> wrote:
If you ignore the permission denied you will ignore missing files that are in protected directories. -Qk should be run as root.
When you run it as root, you don't get permission denied, do you? So ignoring it doesn't change anything. However, when you run as user, not ignoring permission denied will report wrongly missing files.
Xavier wrote :
On Sun, Jul 19, 2009 at 1:13 PM, changaco<changaco@gmail.com> wrote:
If you ignore the permission denied you will ignore missing files that are in protected directories. -Qk should be run as root.
When you run it as root, you don't get permission denied, do you? So ignoring it doesn't change anything.
However, when you run as user, not ignoring permission denied will report wrongly missing files.
What I meant is that silent errors are bad, we can either force -Qk to be run as root or leave it like it is but silently ignoring potential problems is not a good idea IMHO.
On Sat, Jul 18, 2009 at 1:09 PM, Xavier Chantry<shiningxc@gmail.com> wrote:
From: Charly COSTE <changaco@laposte.net>
A new option "-Qk" which checks if all packages files are really on the system (i.e. not accidentally deleted).
This implements FS#13877
Signed-off-by: Charly COSTE <changaco@laposte.net> [Xav : don't ignore tmp, don't repeat package names in quiet output, add errno message in verbose output] Signed-off-by: Xavier Chantry <shiningxc@gmail.com>
OK, I'm going to finally review this patch enough that we can get it in. Hopefully we can make a team effort to get it fixed up.
--- doc/pacman.8.txt | 4 ++ src/pacman/conf.h | 1 + src/pacman/pacman.c | 7 +++- src/pacman/query.c | 114 +++++++++++++++++++++++++++++++++++++++++++++++--- 4 files changed, 118 insertions(+), 8 deletions(-)
diff --git a/doc/pacman.8.txt b/doc/pacman.8.txt index af85a15..ccff167 100644 --- a/doc/pacman.8.txt +++ b/doc/pacman.8.txt @@ -196,6 +196,10 @@ Query Options[[QO]] '\--info' or '-i' flags will also display the list of backup files and their modification states.
+*-k \--check*:: + Check that all files owned by the given package(s) are present on the + system. If packages are not specified, check all installed packages. + Good.
*-l, \--list*:: List all files owned by a given package. Multiple packages can be specified on the command line. diff --git a/src/pacman/conf.h b/src/pacman/conf.h index 39802ca..6523d49 100644 --- a/src/pacman/conf.h +++ b/src/pacman/conf.h @@ -51,6 +51,7 @@ typedef struct __config_t { unsigned short op_q_search; unsigned short op_q_changelog; unsigned short op_q_upgrade; + unsigned short op_q_check;
unsigned short op_s_clean; unsigned short op_s_downloadonly; diff --git a/src/pacman/pacman.c b/src/pacman/pacman.c index 7f86489..48d45ad 100644 --- a/src/pacman/pacman.c +++ b/src/pacman/pacman.c @@ -108,6 +108,7 @@ static void usage(int op, const char * const myname) printf(_(" -e, --explicit list packages explicitly installed [filter]\n")); printf(_(" -g, --groups view all members of a package group\n")); printf(_(" -i, --info view package information (-ii for backup files)\n")); + printf(_(" -k, --check check that the files owned by the package(s) are present\n")); printf(_(" -l, --list list the contents of the queried package\n")); printf(_(" -m, --foreign list installed packages not found in sync db(s) [filter]\n")); printf(_(" -o, --owns <file> query the package that owns <file>\n")); @@ -345,6 +346,7 @@ static int parseargs(int argc, char *argv[]) {"help", no_argument, 0, 'h'}, {"info", no_argument, 0, 'i'}, {"dbonly", no_argument, 0, 'k'}, + {"check", no_argument, 0, 'k'}, {"list", no_argument, 0, 'l'}, {"foreign", no_argument, 0, 'm'}, {"nosave", no_argument, 0, 'n'}, @@ -473,7 +475,10 @@ static int parseargs(int argc, char *argv[]) case 'g': (config->group)++; break; case 'h': config->help = 1; break; case 'i': (config->op_q_info)++; (config->op_s_info)++; break; - case 'k': config->flags |= PM_TRANS_FLAG_DBONLY; break; + case 'k': + config->flags |= PM_TRANS_FLAG_DBONLY; + config->op_q_check = 1; + break; case 'l': config->op_q_list = 1; break; case 'm': config->op_q_foreign = 1; break; case 'n': config->flags |= PM_TRANS_FLAG_NOSAVE; break; diff --git a/src/pacman/query.c b/src/pacman/query.c index 4997202..62eca53 100644 --- a/src/pacman/query.c +++ b/src/pacman/query.c @@ -334,10 +334,94 @@ static void display(pmpkg_t *pkg) } }
+static void check_display_progress(int j, int pkgs_n, const char *pkgname) +{ + char message[getcols()]; + int len = snprintf(message, getcols(), "(%i/%i) %s %s ...", (int)j, (int)pkgs_n, _("checking"), pkgname); + fprintf(stderr, "\r%s %*s", message, getcols()-len-1, " "); + fflush(stderr); +} + +/* Loop through the packages. For each package, + * loop through files to check if they exist. */ +static void check(alpm_list_t *pkgs) +{ + alpm_list_t *i, *files, *file, *damaged = NULL; + pmpkg_t *pkg; + const char *root; + int pkgs_n, j = 0, allfiles = 0; + size_t rootlen; + char f[PATH_MAX]; + + pkgs_n = alpm_list_count(pkgs); + root = alpm_option_get_root(); + rootlen = strlen(root); + if(rootlen + 1 > PATH_MAX) { + /* we are in trouble here */ + pm_printf(PM_LOG_ERROR, _("root path too long\n")); + return; + } + strcpy(f, root); + + for(i = pkgs; i; i = alpm_list_next(i)) { + j++; + pkg = alpm_list_getdata(i); + const char *pkgname = alpm_pkg_get_name(pkg); + files = alpm_pkg_get_files(pkg); + if(!config->quiet) { + check_display_progress(j, pkgs_n, pkgname); + } + for(file = files; file; file = alpm_list_next(file)){ + struct stat st; + char *x = alpm_list_getdata(file); + if(rootlen + 1 + strlen(x) > PATH_MAX) { + pm_printf(PM_LOG_WARNING, _("file path too long\n")); + continue; + } + strcpy(f + rootlen, x); + allfiles++; + /* use lstat to prevent errors from symlinks */ + if(lstat(f,&st) != 0) { + if(config->quiet) { + fprintf(stderr, "%s %s\n", pkgname, f); + fflush(stderr); + } else { + fprintf(stderr, "\r%s: %s (%s)\n", pkgname, f, strerror(errno)); + check_display_progress(j, pkgs_n, pkgname); + fflush(stderr); + } + if(alpm_list_find_ptr(damaged, pkgname) == NULL) { + damaged = alpm_list_add(damaged, (char*)pkgname); + } + } + } + } + if(!config->quiet) { + char message[getcols()]; + int len = snprintf(message, getcols(), "%s: %i %s (%i %s)", + _("Check complete"), allfiles, _("files"), pkgs_n, _("packages")); + fprintf(stderr, "\r%s %*s\n", message, getcols()-len-1, " "); + fflush(stderr); + + if(alpm_list_count(damaged) > 0) { + fprintf(stderr, "\r%s ", _("Damaged packages:")); + fflush(stderr); + for(i = damaged; i; i = alpm_list_next(i)) { + fprintf(stdout, "%s ", (char*)alpm_list_getdata(i)); + } + fprintf(stdout, "\n"); + fflush(stdout); + } + } + + alpm_list_free(damaged); +} + int pacman_query(alpm_list_t *targets) { int ret = 0; - alpm_list_t *i; + alpm_list_t *i, *pkgs = NULL; + pmpkg_t *pkg = NULL;
/* First: operations that do not require targets */
@@ -363,7 +447,7 @@ int pacman_query(alpm_list_t *targets) }
/* operations on all packages in the local DB - * valid: no-op (plain -Q), list, info + * valid: no-op (plain -Q), list, info, check * invalid: isfile, owns */ if(targets == NULL) { if(config->op_q_isfile || config->op_q_owns) { @@ -372,11 +456,19 @@ int pacman_query(alpm_list_t *targets) }
for(i = alpm_db_get_pkgcache(db_local); i; i = alpm_list_next(i)) { - pmpkg_t *pkg = alpm_list_getdata(i); + pkg = alpm_list_getdata(i); if(filter(pkg)) { - display(pkg); + if(config->op_q_check) { + pkgs = alpm_list_add(pkgs, pkg); + } else { + display(pkg); + } } } + if(config->op_q_check){ + check(pkgs); + alpm_list_free(pkgs); + } return(0); }
@@ -389,10 +481,9 @@ int pacman_query(alpm_list_t *targets) }
/* operations on named packages in the local DB - * valid: no-op (plain -Q), list, info */ + * valid: no-op (plain -Q), list, info, check */ for(i = targets; i; i = alpm_list_next(i)) { char *strname = alpm_list_getdata(i); - pmpkg_t *pkg = NULL;
if(config->op_q_isfile) { alpm_pkg_load(strname, 1, &pkg); @@ -406,8 +497,13 @@ int pacman_query(alpm_list_t *targets) continue; }
+ ^^^ necessary? if(filter(pkg)) { - display(pkg); + if(config->op_q_check) { + pkgs = alpm_list_add(pkgs, pkg); + } else { + display(pkg); + } }
if(config->op_q_isfile) { @@ -415,6 +511,10 @@ int pacman_query(alpm_list_t *targets) pkg = NULL; } } + if(config->op_q_check){ Please match brace style. You're missing space between the ) and the { closing the line. You did this in more than one place.
+ check(pkgs); + alpm_list_free(pkgs); + }
Hmm, the last few diff hunks are quite interesting. Really wish we didn't have to special case this so much. Why not call check() from display() so it fits in with the rest of the logic? Ahh, because we used a homegrown progress function...I said it way back when I was never a fan of this, unfortunately, and no one got the hint.
return(ret); } -- 1.6.3.3
So I didn't even type up all of my thoughts, but I did go and hack at the patch a bit instead. At first I was just going to submit a supplemental patch, but instead I've reworked it enough that I'll submit in one piece as a reply to this email. -Dan
This implements FS#13877. Add a new option "-Qk" which checks if all of the files for a given package (or packages) are really on the system (i.e. not accidentally deleted). This can be combined with filters and other display options. It also respects both the --quiet and --verbose flags to give varying levels of output. Based on the original patch by Charly Coste <changaco@laposte.net>, thanks for your work! Signed-off-by: Dan McGee <dan@archlinux.org> --- Some additional notes and example output: dmcgee@galway ~/projects/pacman (master) $ ./src/pacman/pacman -Qkq wpa_supplicant /etc/wpa_supplicant.conf dmcgee@galway ~/projects/pacman (master) $ ./src/pacman/pacman -Qk wpa_supplicant: missing /etc/wpa_supplicant.conf (No such file or directory) wpa_supplicant: 22 total, 1 missing file(s) $ ./src/pacman/pacman -Qkmv Root : / Conf File : /etc/pacman.conf DB Path : /var/lib/pacman/ Cache Dirs: /var/cache/pacman/pkg/ /home/makepkg/packages/ Lock File : /var/lib/pacman/db.lck Log File : /var/log/pacman.log Targets : None bjfilter: 5 total, 0 missing file(s) clearlooks: 51 total, 0 missing file(s) ebtables: 37 total, 0 missing file(s) icc: 1729 total, 0 missing file(s) intel-compilers-common: 104 total, 0 missing file(s) jre_beta: 825 total, 0 missing file(s) kcachegrind: 60 total, 0 missing file(s) libcnbj: 36 total, 0 missing file(s) metasploit3: 16330 total, 0 missing file(s) mixxx: 2072 total, 0 missing file(s) munin-node: 159 total, 0 missing file(s) picasa-beta: 1133 total, 0 missing file(s) pstocanonbj: 24 total, 0 missing file(s) python-markdown: 65 total, 0 missing file(s) rng-tools: 15 total, 0 missing file(s) tkinfo: 9 total, 0 missing file(s) weka: 11 total, 0 missing file(s) OK, that last one looks a bit silly with the paths at the top, doesn't it. Any ideas? I'd be fine with showing the 0 errors lines all the time, it would just require some grep foo for people to screen those out. That way, you can do things like this (note that the output is slightly edited from what this patch will produce, it is showing the output even with 0 missing files): ''''' $ ./src/pacman/pacman -Qiik pacman-git Name : pacman-git Version : 20090715-1 URL : http://www.archlinux.org/pacman/ Licenses : GPL Groups : None Provides : pacman=3.2.2 Depends On : gcc-libs bash libarchive>=2.6.0 libfetch pacman-mirrorlist Optional Deps : fakeroot: for makepkg usage as normal user python: for rankmirrors script usage Required By : pacman-contrib pkgstats Conflicts With : pacman Replaces : None Installed Size : 2036.00 K Packager : Dan McGee <dan@archlinux.org> Architecture : x86_64 Build Date : Wed 15 Jul 2009 09:15:00 PM CDT Install Date : Wed 15 Jul 2009 11:40:07 PM CDT Install Reason : Explicitly installed Install Script : No Description : A library-based package manager with dependency support Backup Files: MODIFIED /etc/pacman.conf MODIFIED /etc/makepkg.conf pacman-git: 114 total, 0 missing file(s) ''''' -Dan doc/pacman.8.txt | 4 ++ src/pacman/conf.h | 1 + src/pacman/pacman.c | 7 ++++- src/pacman/query.c | 83 ++++++++++++++++++++++++++++++++++++++++++++------- 4 files changed, 83 insertions(+), 12 deletions(-) diff --git a/doc/pacman.8.txt b/doc/pacman.8.txt index af85a15..ccff167 100644 --- a/doc/pacman.8.txt +++ b/doc/pacman.8.txt @@ -196,6 +196,10 @@ Query Options[[QO]] '\--info' or '-i' flags will also display the list of backup files and their modification states. +*-k \--check*:: + Check that all files owned by the given package(s) are present on the + system. If packages are not specified, check all installed packages. + *-l, \--list*:: List all files owned by a given package. Multiple packages can be specified on the command line. diff --git a/src/pacman/conf.h b/src/pacman/conf.h index 39802ca..6523d49 100644 --- a/src/pacman/conf.h +++ b/src/pacman/conf.h @@ -51,6 +51,7 @@ typedef struct __config_t { unsigned short op_q_search; unsigned short op_q_changelog; unsigned short op_q_upgrade; + unsigned short op_q_check; unsigned short op_s_clean; unsigned short op_s_downloadonly; diff --git a/src/pacman/pacman.c b/src/pacman/pacman.c index 7f86489..48d45ad 100644 --- a/src/pacman/pacman.c +++ b/src/pacman/pacman.c @@ -108,6 +108,7 @@ static void usage(int op, const char * const myname) printf(_(" -e, --explicit list packages explicitly installed [filter]\n")); printf(_(" -g, --groups view all members of a package group\n")); printf(_(" -i, --info view package information (-ii for backup files)\n")); + printf(_(" -k, --check check that the files owned by the package(s) are present\n")); printf(_(" -l, --list list the contents of the queried package\n")); printf(_(" -m, --foreign list installed packages not found in sync db(s) [filter]\n")); printf(_(" -o, --owns <file> query the package that owns <file>\n")); @@ -345,6 +346,7 @@ static int parseargs(int argc, char *argv[]) {"help", no_argument, 0, 'h'}, {"info", no_argument, 0, 'i'}, {"dbonly", no_argument, 0, 'k'}, + {"check", no_argument, 0, 'k'}, {"list", no_argument, 0, 'l'}, {"foreign", no_argument, 0, 'm'}, {"nosave", no_argument, 0, 'n'}, @@ -473,7 +475,10 @@ static int parseargs(int argc, char *argv[]) case 'g': (config->group)++; break; case 'h': config->help = 1; break; case 'i': (config->op_q_info)++; (config->op_s_info)++; break; - case 'k': config->flags |= PM_TRANS_FLAG_DBONLY; break; + case 'k': + config->flags |= PM_TRANS_FLAG_DBONLY; + config->op_q_check = 1; + break; case 'l': config->op_q_list = 1; break; case 'm': config->op_q_foreign = 1; break; case 'n': config->flags |= PM_TRANS_FLAG_NOSAVE; break; diff --git a/src/pacman/query.c b/src/pacman/query.c index 4997202..b70c713 100644 --- a/src/pacman/query.c +++ b/src/pacman/query.c @@ -309,8 +309,58 @@ static int filter(pmpkg_t *pkg) return(1); } -static void display(pmpkg_t *pkg) +/* Loop through the packages. For each package, + * loop through files to check if they exist. */ +static int check(pmpkg_t *pkg) { + alpm_list_t *i; + const char *root; + int allfiles = 0, errors = 0; + size_t rootlen; + char f[PATH_MAX]; + + root = alpm_option_get_root(); + rootlen = strlen(root); + if(rootlen + 1 > PATH_MAX) { + /* we are in trouble here */ + pm_printf(PM_LOG_ERROR, _("root path too long\n")); + return(1); + } + strcpy(f, root); + + const char *pkgname = alpm_pkg_get_name(pkg); + for(i = alpm_pkg_get_files(pkg); i; i = alpm_list_next(i)) { + struct stat st; + const char *path = alpm_list_getdata(i); + + if(rootlen + 1 + strlen(path) > PATH_MAX) { + pm_printf(PM_LOG_WARNING, _("file path too long\n")); + continue; + } + strcpy(f + rootlen, path); + allfiles++; + /* use lstat to prevent errors from symlinks */ + if(lstat(f, &st) != 0) { + if(config->quiet) { + fprintf(stderr, "%s %s\n", pkgname, f); + } else { + fprintf(stderr, "%s: missing %s (%s)\n", pkgname, f, strerror(errno)); + } + errors++; + } + } + + if((errors > 0 && !config->quiet) || config->verbose) { + printf("%s: %d total, %d missing file(s)\n", pkgname, allfiles, errors); + } + + return(errors != 0 ? 1 : 0); +} + +static int display(pmpkg_t *pkg) +{ + int ret = 0; + if(config->op_q_info) { if(config->op_q_isfile) { /* omit info that isn't applicable for a file package */ @@ -325,19 +375,25 @@ static void display(pmpkg_t *pkg) if(config->op_q_changelog) { dump_pkg_changelog(pkg); } - if(!config->op_q_info && !config->op_q_list && !config->op_q_changelog) { + if(config->op_q_check) { + ret = check(pkg); + } + if(!config->op_q_info && !config->op_q_list + && !config->op_q_changelog && !config->op_q_check) { if (!config->quiet) { printf("%s %s\n", alpm_pkg_get_name(pkg), alpm_pkg_get_version(pkg)); } else { printf("%s\n", alpm_pkg_get_name(pkg)); } } + return(ret); } int pacman_query(alpm_list_t *targets) { int ret = 0; alpm_list_t *i; + pmpkg_t *pkg = NULL; /* First: operations that do not require targets */ @@ -358,12 +414,12 @@ int pacman_query(alpm_list_t *targets) alpm_list_t *sync_dbs = alpm_option_get_syncdbs(); if(sync_dbs == NULL || alpm_list_count(sync_dbs) == 0) { pm_printf(PM_LOG_ERROR, _("no usable package repositories configured.\n")); - return(-1); + return(1); } } /* operations on all packages in the local DB - * valid: no-op (plain -Q), list, info + * valid: no-op (plain -Q), list, info, check * invalid: isfile, owns */ if(targets == NULL) { if(config->op_q_isfile || config->op_q_owns) { @@ -372,12 +428,15 @@ int pacman_query(alpm_list_t *targets) } for(i = alpm_db_get_pkgcache(db_local); i; i = alpm_list_next(i)) { - pmpkg_t *pkg = alpm_list_getdata(i); + pkg = alpm_list_getdata(i); if(filter(pkg)) { - display(pkg); + int value = display(pkg); + if(value != 0) { + ret = 1; + } } } - return(0); + return(ret); } /* Second: operations that require target(s) */ @@ -389,10 +448,9 @@ int pacman_query(alpm_list_t *targets) } /* operations on named packages in the local DB - * valid: no-op (plain -Q), list, info */ + * valid: no-op (plain -Q), list, info, check */ for(i = targets; i; i = alpm_list_next(i)) { char *strname = alpm_list_getdata(i); - pmpkg_t *pkg = NULL; if(config->op_q_isfile) { alpm_pkg_load(strname, 1, &pkg); @@ -402,12 +460,15 @@ int pacman_query(alpm_list_t *targets) if(pkg == NULL) { pm_fprintf(stderr, PM_LOG_ERROR, _("package \"%s\" not found\n"), strname); - ret++; + ret = 1; continue; } if(filter(pkg)) { - display(pkg); + int value = display(pkg); + if(value != 0) { + ret = 1; + } } if(config->op_q_isfile) { -- 1.6.3.3
On Wed, Jul 22, 2009 at 5:58 AM, Dan McGee<dan@archlinux.org> wrote:
This implements FS#13877. Add a new option "-Qk" which checks if all of the files for a given package (or packages) are really on the system (i.e. not accidentally deleted). This can be combined with filters and other display options. It also respects both the --quiet and --verbose flags to give varying levels of output.
Based on the original patch by Charly Coste <changaco@laposte.net>, thanks for your work!
Signed-off-by: Dan McGee <dan@archlinux.org> ---
Some additional notes and example output:
dmcgee@galway ~/projects/pacman (master) $ ./src/pacman/pacman -Qkq wpa_supplicant /etc/wpa_supplicant.conf
dmcgee@galway ~/projects/pacman (master) $ ./src/pacman/pacman -Qk wpa_supplicant: missing /etc/wpa_supplicant.conf (No such file or directory) wpa_supplicant: 22 total, 1 missing file(s)
$ ./src/pacman/pacman -Qkmv Root : / Conf File : /etc/pacman.conf DB Path : /var/lib/pacman/ Cache Dirs: /var/cache/pacman/pkg/ /home/makepkg/packages/ Lock File : /var/lib/pacman/db.lck Log File : /var/log/pacman.log Targets : None bjfilter: 5 total, 0 missing file(s) clearlooks: 51 total, 0 missing file(s) ebtables: 37 total, 0 missing file(s) icc: 1729 total, 0 missing file(s) intel-compilers-common: 104 total, 0 missing file(s) jre_beta: 825 total, 0 missing file(s) kcachegrind: 60 total, 0 missing file(s) libcnbj: 36 total, 0 missing file(s) metasploit3: 16330 total, 0 missing file(s) mixxx: 2072 total, 0 missing file(s) munin-node: 159 total, 0 missing file(s) picasa-beta: 1133 total, 0 missing file(s) pstocanonbj: 24 total, 0 missing file(s) python-markdown: 65 total, 0 missing file(s) rng-tools: 15 total, 0 missing file(s) tkinfo: 9 total, 0 missing file(s) weka: 11 total, 0 missing file(s)
OK, that last one looks a bit silly with the paths at the top, doesn't it. Any ideas? I'd be fine with showing the 0 errors lines all the time, it would just require some grep foo for people to screen those out. That way, you can do things like this (note that the output is slightly edited from what this patch will produce, it is showing the output even with 0 missing files):
''''' $ ./src/pacman/pacman -Qiik pacman-git Name : pacman-git Version : 20090715-1 URL : http://www.archlinux.org/pacman/ Licenses : GPL Groups : None Provides : pacman=3.2.2 Depends On : gcc-libs bash libarchive>=2.6.0 libfetch pacman-mirrorlist Optional Deps : fakeroot: for makepkg usage as normal user python: for rankmirrors script usage Required By : pacman-contrib pkgstats Conflicts With : pacman Replaces : None Installed Size : 2036.00 K Packager : Dan McGee <dan@archlinux.org> Architecture : x86_64 Build Date : Wed 15 Jul 2009 09:15:00 PM CDT Install Date : Wed 15 Jul 2009 11:40:07 PM CDT Install Reason : Explicitly installed Install Script : No Description : A library-based package manager with dependency support Backup Files: MODIFIED /etc/pacman.conf MODIFIED /etc/makepkg.conf
pacman-git: 114 total, 0 missing file(s) '''''
-Dan
Indeed, this is much better without the progressbar, I didn't like either that we couldn't use display :) But I am not sure about the --verbose output. This would be the only option affected by verbose, which is currently only used to display the paths. But well, maybe it is fine.
On Wed, Jul 22, 2009 at 5:58 AM, Dan McGee<dan@archlinux.org> wrote:
OK, that last one looks a bit silly with the paths at the top, doesn't it. Any ideas? I'd be fine with showing the 0 errors lines all the time, it would just require some grep foo for people to screen those out. That way, you can do things like this (note that the output is slightly edited from what this patch will produce, it is showing the output even with 0 missing files):
I would be fine with never printing 0 error line, and dropping the verbose output. -Qk and -Qkq are just fine like that in my opinion.
On Wed, Jul 22, 2009 at 5:58 AM, Dan McGee<dan@archlinux.org> wrote:
This implements FS#13877. Add a new option "-Qk" which checks if all of the files for a given package (or packages) are really on the system (i.e. not accidentally deleted). This can be combined with filters and other display options. It also respects both the --quiet and --verbose flags to give varying levels of output.
Hopefully last comment : Nagy also remembered that we should keep bash and zsh completion files up-to-date for 3.3 release. I just checked with a diff on pacman.c, and actually the only option changes are not committed yet (-k and --print), so it is probably best to include completion files updates in the respective patches.
participants (7)
-
Allan McRae
-
changaco
-
Dan McGee
-
Dan McGee
-
Roman Kyrylych
-
Xavier
-
Xavier Chantry