[PATCH 2/2] ls-remote: create option to sort by versions

[PATCH 2/2] ls-remote: create option to sort by versions

[Harald Nordgren]

Updating tests and documentation

Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
---
 Documentation/git-ls-remote.txt |  5 +++++
 t/t5512-ls-remote.sh            | 19 ++++++++++++++++++-
 2 files changed, 23 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-ls-remote.txt b/Documentation/git-ls-remote.txt
index 5f2628c8f..81c518a90 100644
--- a/Documentation/git-ls-remote.txt
+++ b/Documentation/git-ls-remote.txt
@@ -30,6 +30,11 @@ OPTIONS
 	both, references stored in refs/heads and refs/tags are
 	displayed.
 
+-V::
+--version-sort::
+	Sort ref names by version semantics, so that refs/tags/v1.2 is sorted
+	before refs/tags/v1.10.
+
 --refs::
 	Do not show peeled tags or pseudorefs like HEAD	in the output.
 
diff --git a/t/t5512-ls-remote.sh b/t/t5512-ls-remote.sh
index 02106c922..680a31223 100755
--- a/t/t5512-ls-remote.sh
+++ b/t/t5512-ls-remote.sh
@@ -10,6 +10,9 @@ test_expect_success setup '
 	test_tick &&
 	git commit -m initial &&
 	git tag mark &&
+	git tag mark1.1 &&
+	git tag mark1.2 &&
+	git tag mark1.10 &&
 	git show-ref --tags -d | sed -e "s/ /	/" >expected.tag &&
 	(
 		echo "$(git rev-parse HEAD)	HEAD"
@@ -39,6 +42,17 @@ test_expect_success 'ls-remote self' '
 	test_cmp expected.all actual
 '
 
+test_expect_success 'ls-remote --version-sort --tags self' '
+	cat >expect <<-\EOF &&
+	1bd44cb9d13204b0fe1958db0082f5028a16eb3a	refs/tags/mark
+	1bd44cb9d13204b0fe1958db0082f5028a16eb3a	refs/tags/mark1.1
+	1bd44cb9d13204b0fe1958db0082f5028a16eb3a	refs/tags/mark1.2
+	1bd44cb9d13204b0fe1958db0082f5028a16eb3a	refs/tags/mark1.10
+	EOF
+	git ls-remote --version-sort --tags self >actual &&
+	test_cmp expect actual
+'
+
 test_expect_success 'dies when no remote specified and no default remotes found' '
 	test_must_fail git ls-remote
 '
@@ -131,7 +145,7 @@ test_expect_success 'Report no-match with --exit-code' '
 
 test_expect_success 'Report match with --exit-code' '
 	git ls-remote --exit-code other.git "refs/tags/*" >actual &&
-	git ls-remote . tags/mark >expect &&
+	git ls-remote . tags/mark* >expect &&
 	test_cmp expect actual
 '
 
@@ -178,6 +192,9 @@ test_expect_success 'ls-remote --symref' '
 	1bd44cb9d13204b0fe1958db0082f5028a16eb3a	refs/remotes/origin/HEAD
 	1bd44cb9d13204b0fe1958db0082f5028a16eb3a	refs/remotes/origin/master
 	1bd44cb9d13204b0fe1958db0082f5028a16eb3a	refs/tags/mark
+	1bd44cb9d13204b0fe1958db0082f5028a16eb3a	refs/tags/mark1.1
+	1bd44cb9d13204b0fe1958db0082f5028a16eb3a	refs/tags/mark1.10
+	1bd44cb9d13204b0fe1958db0082f5028a16eb3a	refs/tags/mark1.2
 	EOF
 	git ls-remote --symref >actual &&
 	test_cmp expect actual
-- 
2.14.3 (Apple Git-98)

[PATCH] ls-remote: create '--sort' option

[Harald Nordgren]

Create a '--sort' option for ls-remote. This is useful e.g. for the Go
repository after the release of version 1.10, where by default v1.10 is
sorted before v1.2. See:

	$ git ls-remote -t https://go.googlesource.com/go
	...
	205f850ceacfc39d1e9d76a9569416284594ce8c        refs/tags/go1.1
	d260448f6b6ac10efe4ae7f6dfe944e72bc2a676        refs/tags/go1.1.1
	1d6d8fca241bb611af51e265c1b5a2e9ae904702        refs/tags/go1.1.2
	bf86aec25972f3a100c3aa58a6abcbcc35bdea49        refs/tags/go1.10
	ac7c0ee26dda18076d5f6c151d8f920b43340ae3        refs/tags/go1.10.1
	9ce6b5c2ed5d3d5251b9a6a0c548d5fb2c8567e8        refs/tags/go1.10beta1
	594668a5a96267a46282ce3007a584ec07adf705        refs/tags/go1.10beta2
	5348aed83e39bd1d450d92d7f627e994c2db6ebf        refs/tags/go1.10rc1
	20e228f2fdb44350c858de941dff4aea9f3127b8        refs/tags/go1.10rc2
	1c5438aae896edcd1e9f9618f4776517f08053b3        refs/tags/go1.1rc2
	46a6097aa7943a490e9bd2e04274845d0e5e200f        refs/tags/go1.1rc3
	402d3590b54e4a0df9fb51ed14b2999e85ce0b76        refs/tags/go1.2
	9c9802fad57c1bcb72ea98c5c55ea2652efc5772        refs/tags/go1.2.1
	...

Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
---
 Documentation/git-ls-remote.txt | 12 +++++++++++-
 builtin/ls-remote.c             | 27 +++++++++++++++++++++++++--
 t/t5512-ls-remote.sh            | 41 ++++++++++++++++++++++++++++++++++++++++-
 3 files changed, 76 insertions(+), 4 deletions(-)

diff --git a/Documentation/git-ls-remote.txt b/Documentation/git-ls-remote.txt
index 5f2628c8f..17fae7218 100644
--- a/Documentation/git-ls-remote.txt
+++ b/Documentation/git-ls-remote.txt
@@ -10,7 +10,7 @@ SYNOPSIS
 --------
 [verse]
 'git ls-remote' [--heads] [--tags] [--refs] [--upload-pack=<exec>]
-	      [-q | --quiet] [--exit-code] [--get-url]
+	      [-q | --quiet] [--exit-code] [--get-url] [--sort=<key>]
 	      [--symref] [<repository> [<refs>...]]
 
 DESCRIPTION
@@ -60,6 +60,16 @@ OPTIONS
 	upload-pack only shows the symref HEAD, so it will be the only
 	one shown by ls-remote.
 
+--sort=<key>::
+	Sort based on the key given.  Prefix `-` to sort in
+	descending order of the value. You may use the --sort=<key> option
+	multiple times, in which case the last key becomes the primary
+	key. Also supports "version:refname" or "v:refname" (tag
+	names are treated as versions). The "version:refname" sort
+	order can also be affected by the "versionsort.suffix"
+	configuration variable.
+	The keys supported are the same as those in `git for-each-ref`.
+
 <repository>::
 	The "remote" repository to query.  This parameter can be
 	either a URL or the name of a remote (see the GIT URLS and
diff --git a/builtin/ls-remote.c b/builtin/ls-remote.c
index 540d56429..5521c72f4 100644
--- a/builtin/ls-remote.c
+++ b/builtin/ls-remote.c
@@ -1,6 +1,7 @@
 #include "builtin.h"
 #include "cache.h"
 #include "transport.h"
+#include "ref-filter.h"
 #include "remote.h"
 
 static const char * const ls_remote_usage[] = {
@@ -47,6 +48,8 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)
 	struct remote *remote;
 	struct transport *transport;
 	const struct ref *ref;
+	struct ref_array array;
+	static struct ref_sorting *sorting = NULL, **sorting_tail = &sorting;
 
 	struct option options[] = {
 		OPT__QUIET(&quiet, N_("do not print remote URL")),
@@ -60,6 +63,8 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)
 		OPT_BIT(0, "refs", &flags, N_("do not show peeled tags"), REF_NORMAL),
 		OPT_BOOL(0, "get-url", &get_url,
 			 N_("take url.<base>.insteadOf into account")),
+		OPT_CALLBACK(0 , "sort", sorting_tail, N_("key"),
+			     N_("field name to sort on"), &parse_opt_ref_sorting),
 		OPT_SET_INT_F(0, "exit-code", &status,
 			      N_("exit with exit code 2 if no matching refs are found"),
 			      2, PARSE_OPT_NOCOMPLETE),
@@ -68,6 +73,8 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)
 		OPT_END()
 	};
 
+	memset(&array, 0, sizeof(array));
+
 	argc = parse_options(argc, argv, prefix, options, ls_remote_usage,
 			     PARSE_OPT_STOP_AT_NON_OPTION);
 	dest = argv[0];
@@ -108,9 +115,25 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)
 			continue;
 		if (!tail_match(pattern, ref->name))
 			continue;
+
+		struct ref_array_item *item;
+		FLEX_ALLOC_MEM(item, refname, ref->name, strlen(ref->name));
+		item->symref = ref->symref;
+		item->objectname = ref->old_oid;
+
+		REALLOC_ARRAY(array.items, array.nr + 1);
+		array.items[array.nr++] = item;
+	}
+
+	if (sorting) {
+		ref_array_sort(sorting, &array);
+	}
+
+	for (int i = 0; i < array.nr; i++) {
+		const struct ref_array_item *ref = array.items[i];
 		if (show_symref_target && ref->symref)
-			printf("ref: %s\t%s\n", ref->symref, ref->name);
-		printf("%s\t%s\n", oid_to_hex(&ref->old_oid), ref->name);
+			printf("ref: %s\t%s\n", ref->symref, ref->refname);
+		printf("%s\t%s\n", oid_to_hex(&ref->objectname), ref->refname);
 		status = 0; /* we found something */
 	}
 	return status;
diff --git a/t/t5512-ls-remote.sh b/t/t5512-ls-remote.sh
index 02106c922..66370cd88 100755
--- a/t/t5512-ls-remote.sh
+++ b/t/t5512-ls-remote.sh
@@ -10,6 +10,9 @@ test_expect_success setup '
 	test_tick &&
 	git commit -m initial &&
 	git tag mark &&
+	git tag mark1.1 &&
+	git tag mark1.2 &&
+	git tag mark1.10 &&
 	git show-ref --tags -d | sed -e "s/ /	/" >expected.tag &&
 	(
 		echo "$(git rev-parse HEAD)	HEAD"
@@ -39,6 +42,39 @@ test_expect_success 'ls-remote self' '
 	test_cmp expected.all actual
 '
 
+test_expect_success 'ls-remote --sort="version:refname" --tags self' '
+	cat >expect <<-\EOF &&
+	1bd44cb9d13204b0fe1958db0082f5028a16eb3a	refs/tags/mark
+	1bd44cb9d13204b0fe1958db0082f5028a16eb3a	refs/tags/mark1.1
+	1bd44cb9d13204b0fe1958db0082f5028a16eb3a	refs/tags/mark1.2
+	1bd44cb9d13204b0fe1958db0082f5028a16eb3a	refs/tags/mark1.10
+	EOF
+	git ls-remote --sort="version:refname" --tags self >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'ls-remote --sort="-version:refname" --tags self' '
+	cat >expect <<-\EOF &&
+	1bd44cb9d13204b0fe1958db0082f5028a16eb3a	refs/tags/mark1.10
+	1bd44cb9d13204b0fe1958db0082f5028a16eb3a	refs/tags/mark1.2
+	1bd44cb9d13204b0fe1958db0082f5028a16eb3a	refs/tags/mark1.1
+	1bd44cb9d13204b0fe1958db0082f5028a16eb3a	refs/tags/mark
+	EOF
+	git ls-remote --sort="-version:refname" --tags self >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'ls-remote --sort="-refname" --tags self' '
+	cat >expect <<-\EOF &&
+	1bd44cb9d13204b0fe1958db0082f5028a16eb3a	refs/tags/mark1.2
+	1bd44cb9d13204b0fe1958db0082f5028a16eb3a	refs/tags/mark1.10
+	1bd44cb9d13204b0fe1958db0082f5028a16eb3a	refs/tags/mark1.1
+	1bd44cb9d13204b0fe1958db0082f5028a16eb3a	refs/tags/mark
+	EOF
+	git ls-remote --sort="-refname" --tags self >actual &&
+	test_cmp expect actual
+'
+
 test_expect_success 'dies when no remote specified and no default remotes found' '
 	test_must_fail git ls-remote
 '
@@ -131,7 +167,7 @@ test_expect_success 'Report no-match with --exit-code' '
 
 test_expect_success 'Report match with --exit-code' '
 	git ls-remote --exit-code other.git "refs/tags/*" >actual &&
-	git ls-remote . tags/mark >expect &&
+	git ls-remote . tags/mark* >expect &&
 	test_cmp expect actual
 '
 
@@ -178,6 +214,9 @@ test_expect_success 'ls-remote --symref' '
 	1bd44cb9d13204b0fe1958db0082f5028a16eb3a	refs/remotes/origin/HEAD
 	1bd44cb9d13204b0fe1958db0082f5028a16eb3a	refs/remotes/origin/master
 	1bd44cb9d13204b0fe1958db0082f5028a16eb3a	refs/tags/mark
+	1bd44cb9d13204b0fe1958db0082f5028a16eb3a	refs/tags/mark1.1
+	1bd44cb9d13204b0fe1958db0082f5028a16eb3a	refs/tags/mark1.10
+	1bd44cb9d13204b0fe1958db0082f5028a16eb3a	refs/tags/mark1.2
 	EOF
 	git ls-remote --symref >actual &&
 	test_cmp expect actual
-- 
2.14.3 (Apple Git-98)

Re: [PATCH] ls-remote: create '--sort' option

[Junio C Hamano]

Harald Nordgren <haraldnordgren@gmail.com> writes:

> Subject: Re: [PATCH] ls-remote: create '--sort' option

It would be more helpful to mark as [PATCH v2] a rerolled patch like
this.

> Create a '--sort' option for ls-remote. This is useful e.g. for the Go
> repository after the release of version 1.10, where by default v1.10 is
> sorted before v1.2. See:
>
> 	$ git ls-remote -t https://go.googlesource.com/go

Does this sample command line also need updating for the refined
design?

> 	...
> 	205f850ceacfc39d1e9d76a9569416284594ce8c        refs/tags/go1.1
> 	d260448f6b6ac10efe4ae7f6dfe944e72bc2a676        refs/tags/go1.1.1
> 	1d6d8fca241bb611af51e265c1b5a2e9ae904702        refs/tags/go1.1.2
> 	bf86aec25972f3a100c3aa58a6abcbcc35bdea49        refs/tags/go1.10
> 	ac7c0ee26dda18076d5f6c151d8f920b43340ae3        refs/tags/go1.10.1
> ...
> +	git ls-remote --sort="-version:refname" --tags self >actual &&
> +	test_cmp expect actual
> +...
> +	git ls-remote --sort="-refname" --tags self >actual &&

These both look sensible (also the variant without the dash prefix
which I omitted from the quote).

Re: [PATCH] ls-remote: create '--sort' option

[Harald Nordgren]

I shortened the commit message. Since we already have this feature for
other sub-commands I guess I don't have to explain what version
semantics are.

Have to say I'm a bit confused about 'git send-mail'. This time I tried

    git send-email --subject-prefix="PATCH v4"
--in-reply-to='20180402005248.52418-1-haraldnordgren@gmail.com'
--compose -1

...and the supplied 'git@vger.kernel.org' on the prompt. hopefully
everyone received the latest patches. However, at least what it looks
like from Gmail / Google Inbox, the discussion is now split into two
threads. Hopefully the patches still make sense though.

I will post replies to the original email chain after this message.

On Mon, Apr 2, 2018 at 10:12 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Harald Nordgren <haraldnordgren@gmail.com> writes:
>
>> Subject: Re: [PATCH] ls-remote: create '--sort' option
>
> It would be more helpful to mark as [PATCH v2] a rerolled patch like
> this.
>
>> Create a '--sort' option for ls-remote. This is useful e.g. for the Go
>> repository after the release of version 1.10, where by default v1.10 is
>> sorted before v1.2. See:
>>
>>       $ git ls-remote -t https://go.googlesource.com/go
>
> Does this sample command line also need updating for the refined
> design?
>
>>       ...
>>       205f850ceacfc39d1e9d76a9569416284594ce8c        refs/tags/go1.1
>>       d260448f6b6ac10efe4ae7f6dfe944e72bc2a676        refs/tags/go1.1.1
>>       1d6d8fca241bb611af51e265c1b5a2e9ae904702        refs/tags/go1.1.2
>>       bf86aec25972f3a100c3aa58a6abcbcc35bdea49        refs/tags/go1.10
>>       ac7c0ee26dda18076d5f6c151d8f920b43340ae3        refs/tags/go1.10.1
>> ...
>> +     git ls-remote --sort="-version:refname" --tags self >actual &&
>> +     test_cmp expect actual
>> +...
>> +     git ls-remote --sort="-refname" --tags self >actual &&
>
> These both look sensible (also the variant without the dash prefix
> which I omitted from the quote).
>
>
>

Re: [PATCH] ls-remote: create '--sort' option

[Jeff King]

On Mon, Apr 02, 2018 at 10:07:36PM +0200, Harald Nordgren wrote:

> @@ -108,9 +115,25 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)
>  			continue;
>  		if (!tail_match(pattern, ref->name))
>  			continue;
> +
> +		struct ref_array_item *item;

We don't allow mixed declarations and code; this line needs to go at the
top of the block.

> +		FLEX_ALLOC_MEM(item, refname, ref->name, strlen(ref->name));
> +		item->symref = ref->symref;
> +		item->objectname = ref->old_oid;
> +
> +		REALLOC_ARRAY(array.items, array.nr + 1);
> +		array.items[array.nr++] = item;
> +	}

This will grow by one each time, which ends up doing quadratic work in
the number of items (although it can often be less if the realloc is
able to just extend the block). It should probably use ALLOC_GROW()
to move it more aggressively.

Interestingly, the ref-filter code itself seems to have the same
problem, but I couldn't measure any speedup. So either my analysis is
totally wrong, or we end up reusing the block most of the time in
practice.

> +
> +	if (sorting) {
> +		ref_array_sort(sorting, &array);
> +	}

Minor style nit: we usually omit the braces for a one-liner block.

The ref-filter code lets you sort on any arbitrary field. So you could
do:

  git ls-remote --sort=committerdate

What should that do?

With your patch, I think we'll just get something like:

  fatal: missing object 468165c1d8a442994a825f3684528361727cd8c0 for HEAD

which is perhaps the best we can do (it might even work if you've
recently fetched from the other side).

-Peff