[pacman-dev] [PATCH 1/2] pacsort: add -f, --files option for sorting filenames
Teach pacsort to understand package filenames and optionally strip away some of the context. alpm_pkg_vercmp() intentionally only understands pure versions, so strings such as '18.0-2-x86_64' and '18.0.1-1-x86_64' will be compared wrongly. Partially addresses FS#33455. Signed-off-by: Dave Reisner <dreisner@archlinux.org> --- src/util/pacsort.c | 48 +++++++++++++++++++++++++++++++++++++++++++++--- test/util/pacsorttest.sh | 15 +++++++++++++++ 2 files changed, 60 insertions(+), 3 deletions(-) diff --git a/src/util/pacsort.c b/src/util/pacsort.c index c7b8c94..b8c8aac 100644 --- a/src/util/pacsort.c +++ b/src/util/pacsort.c @@ -18,6 +18,7 @@ */ #include <errno.h> +#include <fnmatch.h> #include <getopt.h> #include <stdio.h> #include <stdlib.h> @@ -43,6 +44,7 @@ static struct options_t { int order; int sortkey; int null; + int filemode; char delim; } opts; @@ -259,15 +261,50 @@ static const char *nth_column(const char *string) static int vercmp(const void *p1, const void *p2) { const char *name1, *name2; + char *fn1 = NULL, *fn2 = NULL; + int r; name1 = *(const char **)p1; name2 = *(const char **)p2; + /* if we're operating in file mode, we modify the strings under certain + * conditions to appease alpm_pkg_vercmp(). If and only if both inputs end + * with a suffix that appears to be a package name, we strip the suffix and + * remove any leading paths. This means that strings such as: + * + * /var/cache/pacman/pkg/firefox-18.0-2-x86_64.pkg.tar.xz + * firefox-18.0-2-x86_64.pkg.tar.gz + * + * Will be considered equal by this version comparison + */ + if(opts.filemode) { + if(fnmatch("*-*.pkg.tar.?z", name1, 0) == 0 && + fnmatch("*-*.pkg.tar.?z", name2, 0) == 0) { + char *s, *e; + + s = strchr(name1, '/'); + e = strrchr(name1, '-'); + fn1 = strndup(name1, e - (s ? s : name1)); + + s = strchr(name2, '/'); + e = strrchr(name2, '-'); + fn2 = strndup(name2, e - (s ? s : name2)); + + name1 = fn1; + name2 = fn2; + } + } + if(opts.sortkey == 0) { - return opts.order * alpm_pkg_vercmp(name1, name2); + r = opts.order * alpm_pkg_vercmp(name1, name2); } else { - return opts.order * alpm_pkg_vercmp(nth_column(name1), nth_column(name2)); + r = opts.order * alpm_pkg_vercmp(nth_column(name1), nth_column(name2)); } + + free(fn1); + free(fn2); + + return r; } static char escape_char(const char *string) @@ -304,6 +341,7 @@ static void usage(void) { fprintf(stderr, "pacsort v" PACKAGE_VERSION "\n" "Usage: pacsort [options] [files...]\n\n" + " -f, --files assume inputs are filepaths of packages\n" " -h, --help display this help message\n" " -k, --key <index> sort input starting on specified column\n" " -r, --reverse sort in reverse order (default: oldest to newest)\n" @@ -316,6 +354,7 @@ static int parse_options(int argc, char **argv) int opt; static const struct option opttable[] = { + {"files", no_argument, 0, 'f'}, {"help", no_argument, 0, 'h'}, {"key", required_argument, 0, 'k'}, {"reverse", no_argument, 0, 'r'}, @@ -324,8 +363,11 @@ static int parse_options(int argc, char **argv) {0, 0, 0, 0} }; - while((opt = getopt_long(argc, argv, "hk:rt:z", opttable, NULL)) != -1) { + while((opt = getopt_long(argc, argv, "fhk:rt:z", opttable, NULL)) != -1) { switch(opt) { + case 'f': + opts.filemode = 1; + break; case 'h': return 1; case 'k': diff --git a/test/util/pacsorttest.sh b/test/util/pacsorttest.sh index 9d52d69..7ab0de4 100755 --- a/test/util/pacsorttest.sh +++ b/test/util/pacsorttest.sh @@ -66,6 +66,21 @@ runtest $in $ex "add trailing newline" in="1.0-1\n1.0\n1.0-2\n1.0\n" runtest $in $in "stable sort" +in="firefox-18.0-2-x86_64.pkg.tar.xz\nfirefox-18.0.1-1-x86_64.pkg.tar.xz\n" +runtest $in $in "filename sort" "--files" + +in="firefox-18.0-2\nfirefox-18.0.1-1-x86_64.pkg.tar.xz\n" +runtest $in $in "filename sort with invalid filename" "--files" + +in="firefox-18.0-2-x86_64.pkg.tar.xz\n/path2/firefox-18.0.1-1-x86_64.pkg.tar.xz\n" +runtest $in $in "filename sort maybe with leading paths" "--files" + +in="/path1/firefox-18.0-2-x86_64.pkg.tar.xz\n/path2/firefox-18.0.1-1-x86_64.pkg.tar.xz\n" +runtest $in $in "filename sort with different leading paths" "--files" + +in="firefox-18.0-2-i686.pkg.tar.xz\nfirefox-18.0.1-1-x86_64.pkg.tar.gz\n" +runtest $in $in "filename sort with different extensions" "--files" + # generate some long input/expected for the next few tests declare normal reverse names_normal names_reverse for ((i=1; i<600; i++)); do -- 1.8.1.1
Resolves FS#33455. Signed-off-by: Dave Reisner <dreisner@archlinux.org> --- contrib/paccache.sh.in | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/contrib/paccache.sh.in b/contrib/paccache.sh.in index e813655..29e2603 100644 --- a/contrib/paccache.sh.in +++ b/contrib/paccache.sh.in @@ -149,7 +149,7 @@ summarize() { else printf "%s$delim" "$pkg" fi - done < <(printf '%s\n' "$@" | pacsort) + done < <(printf '%s\n' "$@" | pacsort --files) fi printf -v output 'finished dry run: %d candidates' "$filecount" fi @@ -283,7 +283,7 @@ cd "$cachedir" >/dev/null || die "failed to chdir to \`%s'" "$cachedir" # note that these results are returned in an arbitrary order from awk, but # they'll be resorted (in summarize) iff we have a verbosity level set. IFS=$'\n' read -r -d '' -a candidates < \ - <(printf '%s\n' *.pkg.tar?(.+([^.])) | pacsort | + <(printf '%s\n' *.pkg.tar?(.+([^.])) | pacsort --files | pkgfilter "$keep" "$scanarch" \ "${#whitelist[*]}" "${whitelist[@]}" \ "${#blacklist[*]}" "${blacklist[@]}") -- 1.8.1.1
On Sat, 19 Jan 2013 11:06:45 -0500 Dave Reisner <dreisner@archlinux.org> wrote: <snip>
+ if(opts.filemode) { + if(fnmatch("*-*.pkg.tar.?z", name1, 0) == 0 && + fnmatch("*-*.pkg.tar.?z", name2, 0) == 0) { + char *s, *e; + + s = strchr(name1, '/'); + e = strrchr(name1, '-'); + fn1 = strndup(name1, e - (s ? s : name1)); + + s = strchr(name2, '/'); + e = strrchr(name2, '-'); + fn2 = strndup(name2, e - (s ? s : name2)); + + name1 = fn1; + name2 = fn2; + } + } +
If your intent is to remove the entire leading path, those should be: s = strrchr(nameX, '/'); You could add another test to catch this: in="/path2/firefox-18.0-2-x86_64.pkg.tar.xz\n/path1/firefox-18.0.1-1-x86_64.pkg.tar.xz\n" runtest $in $in "filename sort with different leading paths" "--files"
On Sat, Jan 19, 2013 at 11:28:47AM -0500, Andrew Gregory wrote:
On Sat, 19 Jan 2013 11:06:45 -0500 Dave Reisner <dreisner@archlinux.org> wrote:
<snip>
+ if(opts.filemode) { + if(fnmatch("*-*.pkg.tar.?z", name1, 0) == 0 && + fnmatch("*-*.pkg.tar.?z", name2, 0) == 0) { + char *s, *e; + + s = strchr(name1, '/'); + e = strrchr(name1, '-'); + fn1 = strndup(name1, e - (s ? s : name1)); + + s = strchr(name2, '/'); + e = strrchr(name2, '-'); + fn2 = strndup(name2, e - (s ? s : name2)); + + name1 = fn1; + name2 = fn2; + } + } +
If your intent is to remove the entire leading path, those should be: s = strrchr(nameX, '/');
You could add another test to catch this:
in="/path2/firefox-18.0-2-x86_64.pkg.tar.xz\n/path1/firefox-18.0.1-1-x86_64.pkg.tar.xz\n" runtest $in $in "filename sort with different leading paths" "--files"
Good catch, thanks. Requires a little more than just the function swap.
Teach pacsort to understand package filenames and optionally strip away some of the context. alpm_pkg_vercmp() intentionally only understands pure versions, so strings such as '18.0-2-x86_64' and '18.0.1-1-x86_64' will be compared wrongly. Partially addresses FS#33455. Signed-off-by: Dave Reisner <dreisner@archlinux.org> --- v2: * use strrchr, not strchr * more descriptive var names (since there's a little more juggling now) src/util/pacsort.c | 52 +++++++++++++++++++++++++++++++++++++++++++++--- test/util/pacsorttest.sh | 18 +++++++++++++++++ 2 files changed, 67 insertions(+), 3 deletions(-) diff --git a/src/util/pacsort.c b/src/util/pacsort.c index c7b8c94..e930dbd 100644 --- a/src/util/pacsort.c +++ b/src/util/pacsort.c @@ -18,6 +18,7 @@ */ #include <errno.h> +#include <fnmatch.h> #include <getopt.h> #include <stdio.h> #include <stdlib.h> @@ -43,6 +44,7 @@ static struct options_t { int order; int sortkey; int null; + int filemode; char delim; } opts; @@ -259,15 +261,54 @@ static const char *nth_column(const char *string) static int vercmp(const void *p1, const void *p2) { const char *name1, *name2; + char *fn1, *fn2; + int r; name1 = *(const char **)p1; name2 = *(const char **)p2; + /* if we're operating in file mode, we modify the strings under certain + * conditions to appease alpm_pkg_vercmp(). If and only if both inputs end + * with a suffix that appears to be a package name, we strip the suffix and + * remove any leading paths. This means that strings such as: + * + * /var/cache/pacman/pkg/firefox-18.0-2-x86_64.pkg.tar.xz + * firefox-18.0-2-x86_64.pkg.tar.gz + * + * Will be considered equal by this version comparison + */ + if(opts.filemode) { + if(fnmatch("*-*.pkg.tar.?z", name1, 0) == 0 && + fnmatch("*-*.pkg.tar.?z", name2, 0) == 0) { + const char *start, *end; + + start = strrchr(name1, '/'); + start = start ? start + 1 : name1; + end = strrchr(name1, '-'); + fn1 = strndup(start, end - start); + + start = strrchr(name2, '/'); + start = start ? start + 1 : name2; + end = strrchr(name2, '-'); + fn2 = strndup(start, end - start); + + name1 = fn1; + name2 = fn2; + } + } + if(opts.sortkey == 0) { - return opts.order * alpm_pkg_vercmp(name1, name2); + r = opts.order * alpm_pkg_vercmp(name1, name2); } else { - return opts.order * alpm_pkg_vercmp(nth_column(name1), nth_column(name2)); + r = opts.order * alpm_pkg_vercmp(nth_column(name1), nth_column(name2)); } + + if(opts.filemode) { + free(fn1); + free(fn2); + } + + return r; } static char escape_char(const char *string) @@ -304,6 +345,7 @@ static void usage(void) { fprintf(stderr, "pacsort v" PACKAGE_VERSION "\n" "Usage: pacsort [options] [files...]\n\n" + " -f, --files assume inputs are filepaths of packages\n" " -h, --help display this help message\n" " -k, --key <index> sort input starting on specified column\n" " -r, --reverse sort in reverse order (default: oldest to newest)\n" @@ -316,6 +358,7 @@ static int parse_options(int argc, char **argv) int opt; static const struct option opttable[] = { + {"files", no_argument, 0, 'f'}, {"help", no_argument, 0, 'h'}, {"key", required_argument, 0, 'k'}, {"reverse", no_argument, 0, 'r'}, @@ -324,8 +367,11 @@ static int parse_options(int argc, char **argv) {0, 0, 0, 0} }; - while((opt = getopt_long(argc, argv, "hk:rt:z", opttable, NULL)) != -1) { + while((opt = getopt_long(argc, argv, "fhk:rt:z", opttable, NULL)) != -1) { switch(opt) { + case 'f': + opts.filemode = 1; + break; case 'h': return 1; case 'k': diff --git a/test/util/pacsorttest.sh b/test/util/pacsorttest.sh index 9d52d69..9cbf619 100755 --- a/test/util/pacsorttest.sh +++ b/test/util/pacsorttest.sh @@ -66,6 +66,24 @@ runtest $in $ex "add trailing newline" in="1.0-1\n1.0\n1.0-2\n1.0\n" runtest $in $in "stable sort" +in="firefox-18.0-2-x86_64.pkg.tar.xz\nfirefox-18.0.1-1-x86_64.pkg.tar.xz\n" +runtest $in $in "filename sort" "--files" + +in="firefox-18.0-2\nfirefox-18.0.1-1-x86_64.pkg.tar.xz\n" +runtest $in $in "filename sort with invalid filename" "--files" + +in="firefox-18.0-2-x86_64.pkg.tar.xz\n/path2/firefox-18.0.1-1-x86_64.pkg.tar.xz\n" +runtest $in $in "filename sort maybe with leading paths" "--files" + +in="/path1/firefox-18.0-2-x86_64.pkg.tar.xz\n/path2/firefox-18.0.1-1-x86_64.pkg.tar.xz\n" +runtest $in $in "filename sort with different leading paths" "--files" + +in="/path2/firefox-18.0-2-x86_64.pkg.tar.xz\n/path1/path2/firefox-18.0.1-1-x86_64.pkg.tar.xz\n" +runtest $in $in "filename sort with uneven leading path components" "--files" + +in="firefox-18.0-2-i686.pkg.tar.xz\nfirefox-18.0.1-1-x86_64.pkg.tar.gz\n" +runtest $in $in "filename sort with different extensions" "--files" + # generate some long input/expected for the next few tests declare normal reverse names_normal names_reverse for ((i=1; i<600; i++)); do -- 1.8.1.1
On 20/01/13 03:00, Dave Reisner wrote:
Teach pacsort to understand package filenames and optionally strip away some of the context. alpm_pkg_vercmp() intentionally only understands pure versions, so strings such as '18.0-2-x86_64' and '18.0.1-1-x86_64' will be compared wrongly.
Partially addresses FS#33455.
Signed-off-by: Dave Reisner <dreisner@archlinux.org> --- v2: * use strrchr, not strchr * more descriptive var names (since there's a little more juggling now)
src/util/pacsort.c | 52 +++++++++++++++++++++++++++++++++++++++++++++--- test/util/pacsorttest.sh | 18 +++++++++++++++++ 2 files changed, 67 insertions(+), 3 deletions(-)
diff --git a/src/util/pacsort.c b/src/util/pacsort.c index c7b8c94..e930dbd 100644 --- a/src/util/pacsort.c +++ b/src/util/pacsort.c @@ -18,6 +18,7 @@ */
#include <errno.h> +#include <fnmatch.h> #include <getopt.h> #include <stdio.h> #include <stdlib.h> @@ -43,6 +44,7 @@ static struct options_t { int order; int sortkey; int null; + int filemode; char delim; } opts;
@@ -259,15 +261,54 @@ static const char *nth_column(const char *string) static int vercmp(const void *p1, const void *p2) { const char *name1, *name2; + char *fn1, *fn2; + int r;
name1 = *(const char **)p1; name2 = *(const char **)p2;
+ /* if we're operating in file mode, we modify the strings under certain + * conditions to appease alpm_pkg_vercmp(). If and only if both inputs end + * with a suffix that appears to be a package name, we strip the suffix and + * remove any leading paths. This means that strings such as: + * + * /var/cache/pacman/pkg/firefox-18.0-2-x86_64.pkg.tar.xz + * firefox-18.0-2-x86_64.pkg.tar.gz + * + * Will be considered equal by this version comparison + */ + if(opts.filemode) { + if(fnmatch("*-*.pkg.tar.?z", name1, 0) == 0 && + fnmatch("*-*.pkg.tar.?z", name2, 0) == 0) { + const char *start, *end; + + start = strrchr(name1, '/'); + start = start ? start + 1 : name1; + end = strrchr(name1, '-'); + fn1 = strndup(start, end - start); + + start = strrchr(name2, '/'); + start = start ? start + 1 : name2; + end = strrchr(name2, '-'); + fn2 = strndup(start, end - start); + + name1 = fn1; + name2 = fn2; + } + } + if(opts.sortkey == 0) { - return opts.order * alpm_pkg_vercmp(name1, name2); + r = opts.order * alpm_pkg_vercmp(name1, name2); } else { - return opts.order * alpm_pkg_vercmp(nth_column(name1), nth_column(name2)); + r = opts.order * alpm_pkg_vercmp(nth_column(name1), nth_column(name2)); } + + if(opts.filemode) { + free(fn1); + free(fn2);
CC pacsort.o <command-line>: In function ‘vercmp’: <command-line>:307:7: error: ‘fn2’ may be used uninitialized in this function [-Werror=maybe-uninitialized] <command-line>:306:7: error: ‘fn1’ may be used uninitialized in this function [-Werror=maybe-uninitialized] cc1: all warnings being treated as errors make[2]: *** [pacsort.o] Error 1 False positive... pull with these variables initialised to NULL.
+ } + + return r; }
static char escape_char(const char *string) @@ -304,6 +345,7 @@ static void usage(void) { fprintf(stderr, "pacsort v" PACKAGE_VERSION "\n" "Usage: pacsort [options] [files...]\n\n" + " -f, --files assume inputs are filepaths of packages\n" " -h, --help display this help message\n" " -k, --key <index> sort input starting on specified column\n" " -r, --reverse sort in reverse order (default: oldest to newest)\n" @@ -316,6 +358,7 @@ static int parse_options(int argc, char **argv) int opt;
static const struct option opttable[] = { + {"files", no_argument, 0, 'f'}, {"help", no_argument, 0, 'h'}, {"key", required_argument, 0, 'k'}, {"reverse", no_argument, 0, 'r'}, @@ -324,8 +367,11 @@ static int parse_options(int argc, char **argv) {0, 0, 0, 0} };
- while((opt = getopt_long(argc, argv, "hk:rt:z", opttable, NULL)) != -1) { + while((opt = getopt_long(argc, argv, "fhk:rt:z", opttable, NULL)) != -1) { switch(opt) { + case 'f': + opts.filemode = 1; + break; case 'h': return 1; case 'k': diff --git a/test/util/pacsorttest.sh b/test/util/pacsorttest.sh index 9d52d69..9cbf619 100755 --- a/test/util/pacsorttest.sh +++ b/test/util/pacsorttest.sh @@ -66,6 +66,24 @@ runtest $in $ex "add trailing newline" in="1.0-1\n1.0\n1.0-2\n1.0\n" runtest $in $in "stable sort"
+in="firefox-18.0-2-x86_64.pkg.tar.xz\nfirefox-18.0.1-1-x86_64.pkg.tar.xz\n" +runtest $in $in "filename sort" "--files" + +in="firefox-18.0-2\nfirefox-18.0.1-1-x86_64.pkg.tar.xz\n" +runtest $in $in "filename sort with invalid filename" "--files" + +in="firefox-18.0-2-x86_64.pkg.tar.xz\n/path2/firefox-18.0.1-1-x86_64.pkg.tar.xz\n" +runtest $in $in "filename sort maybe with leading paths" "--files" + +in="/path1/firefox-18.0-2-x86_64.pkg.tar.xz\n/path2/firefox-18.0.1-1-x86_64.pkg.tar.xz\n" +runtest $in $in "filename sort with different leading paths" "--files" + +in="/path2/firefox-18.0-2-x86_64.pkg.tar.xz\n/path1/path2/firefox-18.0.1-1-x86_64.pkg.tar.xz\n" +runtest $in $in "filename sort with uneven leading path components" "--files" + +in="firefox-18.0-2-i686.pkg.tar.xz\nfirefox-18.0.1-1-x86_64.pkg.tar.gz\n" +runtest $in $in "filename sort with different extensions" "--files" + # generate some long input/expected for the next few tests declare normal reverse names_normal names_reverse for ((i=1; i<600; i++)); do
participants (4)
-
Allan McRae
-
Andrew Gregory
-
Dave Reisner
-
Dave Reisner