{"thread":{"id":"53238","subject":"git ls-remote and protocolv2","startedAt":"2020-04-15T13:10:25Z","lastAt":"2020-04-15T16:04:40Z","messageCount":2,"participants":["Keegan Carruthers-Smith","Jeff King"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"395490","messageId":"m2k12g7v5u.fsf@gmail.com","threadId":"53238","inReplyTo":null,"subject":"git ls-remote and protocolv2","fromName":"Keegan Carruthers-Smith","fromEmail":"keegan.csmith@gmail.com","sentAt":"2020-04-15T13:09:17Z","receivedAt":"2020-04-15T13:10:25Z","isPatch":false,"sender":{"key":"keegan.csmith@gmail.com","avatar":"https://gravatar.com/avatar/42716e751b8473bf2e823218d835b52507c5654a93561d1a2cd32a9a14a2293a?d=mp&s=160"},"body":"Hi,\n\nI would like to take advantage of protocol v2 in \"git\nls-remote\". However, it can't reasonably use it because the patterns for\nls-remote aren't prefix based patterns. See\nhttps://public-inbox.org/git/20181031042405.GA5503@sigill.intra.peff.net/\n\nThis is behaviour I would like to implement. At Sourcegraph (were I\nwork) we use \"git ls-remote HEAD\" to test if a remote is reachable and a\nvalid git remote. Additionally we use it to get the symref for the\ndefault branch.\n\nThere may be a better way to do this, but so far it seems ls-remote is\nthe most reliable. However, we don't get to take advantage of protocol\nv2. A simple hack I did gives much better perf for our use case:\n\n  diff --git a/builtin/ls-remote.c b/builtin/ls-remote.c\n  index 6ef519514b..12d3af177a 100644\n  --- a/builtin/ls-remote.c\n  +++ b/builtin/ls-remote.c\n  @@ -91,6 +91,8 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)\n   \t\t}\n   \t}\n   \n  +\targv_array_push(&ref_prefixes, \"HEAD\");\n  +\n   \tif (flags & REF_TAGS)\n   \t\targv_array_push(&ref_prefixes, \"refs/tags/\");\n   \tif (flags & REF_HEADS)\n\n\nWhat are the options to allow the use of protocol v2? The ideas I have\nin mind are the following.\n\n\"--ref-prefixes\" flag. This changes the behaviour of the patterns to\ninstead be ref prefixes so we can pass them as ref prefixes.\n\n\"--ref-prefix=PREFIX\" flag. Can be passed in multiple times. Each PREFIX\nis set as a ref prefix.\n\n\"refs/\" prefix in pattern. If all patterns have a prefix of \"refs/\" pass\nin the relevant prefixes to remote refs. This would be a breaking change\nfor the rare case of refs named like \"refs/heads/refs/foo\". You also\nwouldn't be able to pass in the symref \"HEAD\" which is what I want for\nmy usecase.\n\nI'm happy to implement any of these.\n\nCheers,\nKeegan\n"},{"id":"395512","messageId":"20200415160427.GE2464307@coredump.intra.peff.net","threadId":"53238","inReplyTo":"m2k12g7v5u.fsf@gmail.com","subject":"Re: git ls-remote and protocolv2","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-04-15T16:04:27Z","receivedAt":"2020-04-15T16:04:40Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Apr 15, 2020 at 03:09:17PM +0200, Keegan Carruthers-Smith wrote:\n\n> I would like to take advantage of protocol v2 in \"git\n> ls-remote\". However, it can't reasonably use it because the patterns for\n> ls-remote aren't prefix based patterns. See\n> https://public-inbox.org/git/20181031042405.GA5503@sigill.intra.peff.net/\n\nYep. It's rather unfortunate, because I suspect most people would be\njust as happy to have the patterns match prefixes anyway.\n\nThe fix you linked was just making sure the existing patterns continued\nto work and maintained backwards-compatibility. But an option to tell\nls-remote \"my patterns are prefixes\" would be perfectly fine to add.\n\n> This is behaviour I would like to implement. At Sourcegraph (were I\n> work) we use \"git ls-remote HEAD\" to test if a remote is reachable and a\n> valid git remote. Additionally we use it to get the symref for the\n> default branch.\n\nThose both seem like sensible use cases for ls-remote. And while we have\n--heads which can take advantage of the v2 filtering, I don't think\nthere's a way to do the same for HEAD. One simple fix would be to add a\n\"--head\" option, though the name is confusingly similar to --heads. :)\n\n> There may be a better way to do this, but so far it seems ls-remote is\n> the most reliable. However, we don't get to take advantage of protocol\n> v2. A simple hack I did gives much better perf for our use case:\n> \n>   diff --git a/builtin/ls-remote.c b/builtin/ls-remote.c\n>   index 6ef519514b..12d3af177a 100644\n>   --- a/builtin/ls-remote.c\n>   +++ b/builtin/ls-remote.c\n>   @@ -91,6 +91,8 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)\n>    \t\t}\n>    \t}\n>    \n>   +\targv_array_push(&ref_prefixes, \"HEAD\");\n>   +\n\n..and then this would basically just become:\n\n  if (flags & REF_HEAD)\n\targv_array_push(&ref_prefixes, \"HEAD\");\n\nThat said, I think I prefer one of the more general solutions below.\n\n> What are the options to allow the use of protocol v2? The ideas I have\n> in mind are the following.\n> \n> \"--ref-prefixes\" flag. This changes the behaviour of the patterns to\n> instead be ref prefixes so we can pass them as ref prefixes.\n\nThis seems like a good addition for other reasons (because people saying\n\"refs/heads/foo\" probably _didn't_ mean to match \"refs/bar/refs/heads/foo\"),\nand probably not significant work to implement. I'd have probably used\nthe word \"pattern\" in the option name, but looking at the manpage for\nls-remote, it calls the pattern arguments \"<refs>\". ;)\n\nProbably as part of this patch, it would make sense to clarify the\ncurrent pattern scheme (in addition to documenting the new rules).\n\nI suspect that --heads and --tags could be reimplemented in terms of\nthis option (and described as such in the documentation).\n\n> \"--ref-prefix=PREFIX\" flag. Can be passed in multiple times. Each PREFIX\n> is set as a ref prefix.\n\nThis could be useful independent of --ref-prefixes, if you really like\nthe current pattern matching but want to restrict the output. E.g.:\n\n  git ls-remote --ref-prefix=refs/notes/ amlog\n\nwould show refs/notes/amlog.\n\nSo I think you could do this in addition to the suggestion above. But as\nthe first one would solve your problem, and nobody has asked for the use\ncase I gave above, I would be fine to leave it until somebody does.\n\n> \"refs/\" prefix in pattern. If all patterns have a prefix of \"refs/\" pass\n> in the relevant prefixes to remote refs. This would be a breaking change\n> for the rare case of refs named like \"refs/heads/refs/foo\". You also\n> wouldn't be able to pass in the symref \"HEAD\" which is what I want for\n> my usecase.\n\nI agree this would _probably_ do what people expect and help more than\nhurt, but I'd prefer not to go this direction. It breaks\nbackwards-compatibility, and it introduces a subtle behavior rule.\nHaving an obvious and explicit like --ref-prefixes solves both of those.\n\n-Peff\n"}]}