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

[Jeff King <peff@peff.net>]

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