On Sat, Jul 18, 2009 at 1:09 PM, Xavier Chantry wrote:

From: Charly COSTE

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 [Xav : don't ignore tmp, don't repeat package names in quiet output, add errno message in verbose output] Signed-off-by: Xavier Chantry

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