[pacman-dev] [PATCHv2] pacsort: add -f, --files option for sorting filenames

[Allan McRae]

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 at 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
>