git ls-remote and protocolv2

git ls-remote and protocolv2

[Keegan Carruthers-Smith]

Hi,

I would like to take advantage of protocol v2 in "git
ls-remote". However, it can't reasonably use it because the patterns for
ls-remote aren't prefix based patterns. See
https://public-inbox.org/git/20181031042405.GA5503@sigill.intra.peff.net/

This is behaviour I would like to implement. At Sourcegraph (were I
work) we use "git ls-remote HEAD" to test if a remote is reachable and a
valid git remote. Additionally we use it to get the symref for the
default branch.

There may be a better way to do this, but so far it seems ls-remote is
the most reliable. However, we don't get to take advantage of protocol
v2. A simple hack I did gives much better perf for our use case:

  diff --git a/builtin/ls-remote.c b/builtin/ls-remote.c
  index 6ef519514b..12d3af177a 100644
  --- a/builtin/ls-remote.c
  +++ b/builtin/ls-remote.c
  @@ -91,6 +91,8 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)
   		}
   	}
   
  +	argv_array_push(&ref_prefixes, "HEAD");
  +
   	if (flags & REF_TAGS)
   		argv_array_push(&ref_prefixes, "refs/tags/");
   	if (flags & REF_HEADS)


What are the options to allow the use of protocol v2? The ideas I have
in mind are the following.

"--ref-prefixes" flag. This changes the behaviour of the patterns to
instead be ref prefixes so we can pass them as ref prefixes.

"--ref-prefix=PREFIX" flag. Can be passed in multiple times. Each PREFIX
is set as a ref prefix.

"refs/" prefix in pattern. If all patterns have a prefix of "refs/" pass
in the relevant prefixes to remote refs. This would be a breaking change
for the rare case of refs named like "refs/heads/refs/foo". You also
wouldn't be able to pass in the symref "HEAD" which is what I want for
my usecase.

I'm happy to implement any of these.

Cheers,
Keegan

Re: git ls-remote and protocolv2

[Jeff King]

On Wed, Apr 15, 2020 at 03:09:17PM +0200, Keegan Carruthers-Smith wrote:

> I would like to take advantage of protocol v2 in "git
> ls-remote". However, it can't reasonably use it because the patterns for
> ls-remote aren't prefix based patterns. See
> https://public-inbox.org/git/20181031042405.GA5503@sigill.intra.peff.net/

Yep. It's rather unfortunate, because I suspect most people would be
just as happy to have the patterns match prefixes anyway.

The fix you linked was just making sure the existing patterns continued
to work and maintained backwards-compatibility. But an option to tell
ls-remote "my patterns are prefixes" would be perfectly fine to add.

> This is behaviour I would like to implement. At Sourcegraph (were I
> work) we use "git ls-remote HEAD" to test if a remote is reachable and a
> valid git remote. Additionally we use it to get the symref for the
> default branch.

Those both seem like sensible use cases for ls-remote. And while we have
--heads which can take advantage of the v2 filtering, I don't think
there's a way to do the same for HEAD. One simple fix would be to add a
"--head" option, though the name is confusingly similar to --heads. :)

> There may be a better way to do this, but so far it seems ls-remote is
> the most reliable. However, we don't get to take advantage of protocol
> v2. A simple hack I did gives much better perf for our use case:
> 
>   diff --git a/builtin/ls-remote.c b/builtin/ls-remote.c
>   index 6ef519514b..12d3af177a 100644
>   --- a/builtin/ls-remote.c
>   +++ b/builtin/ls-remote.c
>   @@ -91,6 +91,8 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)
>    		}
>    	}
>    
>   +	argv_array_push(&ref_prefixes, "HEAD");
>   +

..and then this would basically just become:

  if (flags & REF_HEAD)
	argv_array_push(&ref_prefixes, "HEAD");

That said, I think I prefer one of the more general solutions below.

> What are the options to allow the use of protocol v2? The ideas I have
> in mind are the following.
> 
> "--ref-prefixes" flag. This changes the behaviour of the patterns to
> instead be ref prefixes so we can pass them as ref prefixes.

This seems like a good addition for other reasons (because people saying
"refs/heads/foo" probably _didn't_ mean to match "refs/bar/refs/heads/foo"),
and probably not significant work to implement. I'd have probably used
the word "pattern" in the option name, but looking at the manpage for
ls-remote, it calls the pattern arguments "<refs>". ;)

Probably as part of this patch, it would make sense to clarify the
current pattern scheme (in addition to documenting the new rules).

I suspect that --heads and --tags could be reimplemented in terms of
this option (and described as such in the documentation).

> "--ref-prefix=PREFIX" flag. Can be passed in multiple times. Each PREFIX
> is set as a ref prefix.

This could be useful independent of --ref-prefixes, if you really like
the current pattern matching but want to restrict the output. E.g.:

  git ls-remote --ref-prefix=refs/notes/ amlog

would show refs/notes/amlog.

So I think you could do this in addition to the suggestion above. But as
the first one would solve your problem, and nobody has asked for the use
case I gave above, I would be fine to leave it until somebody does.

> "refs/" prefix in pattern. If all patterns have a prefix of "refs/" pass
> in the relevant prefixes to remote refs. This would be a breaking change
> for the rare case of refs named like "refs/heads/refs/foo". You also
> wouldn't be able to pass in the symref "HEAD" which is what I want for
> my usecase.

I agree this would _probably_ do what people expect and help more than
hurt, but I'd prefer not to go this direction. It breaks
backwards-compatibility, and it introduces a subtle behavior rule.
Having an obvious and explicit like --ref-prefixes solves both of those.

-Peff