Re: [PATCH 12/26] ls-refs: introduce ls-refs server command

[Brandon Williams]

On 02/01, Jeff Hostetler wrote:
> 
> 
> On 1/2/2018 7:18 PM, Brandon Williams wrote:
> > Introduce the ls-refs server command.  In protocol v2, the ls-refs
> > command is used to request the ref advertisement from the server.  Since
> > it is a command which can be requested (as opposed to mandatory in v1),
> > a client can sent a number of parameters in its request to limit the ref
> > advertisement based on provided ref-patterns.
> > 
> > Signed-off-by: Brandon Williams 
> > ---
> >   Documentation/technical/protocol-v2.txt | 26 +
> >   Makefile|  1 +
> >   ls-refs.c   | 97 
> > +
> >   ls-refs.h   |  9 +++
> >   serve.c |  2 +
> >   5 files changed, 135 insertions(+)
> >   create mode 100644 ls-refs.c
> >   create mode 100644 ls-refs.h
> > 
> > diff --git a/Documentation/technical/protocol-v2.txt 
> > b/Documentation/technical/protocol-v2.txt
> > index b87ba3816..5f4d0e719 100644
> > --- a/Documentation/technical/protocol-v2.txt
> > +++ b/Documentation/technical/protocol-v2.txt
> > @@ -89,3 +89,29 @@ terminate the connection.
> >   Commands are the core actions that a client wants to perform (fetch, push,
> >   etc).  Each command will be provided with a list capabilities and
> >   arguments as requested by a client.
> > +
> > + Ls-refs
> > +-
> > +
> > +Ls-refs is the command used to request a reference advertisement in v2.
> > +Unlike the current reference advertisement, ls-refs takes in parameters
> > +which can be used to limit the refs sent from the server.
> > +
> > +Ls-ref takes in the following parameters wraped in packet-lines:
> > +
> > +  symrefs: In addition to the object pointed by it, show the underlying
> > +  ref pointed by it when showing a symbolic ref.
> > +  peel: Show peeled tags.
> > +  ref-pattern : When specified, only references matching the
> > +given patterns are displayed.
> > +
> > +The output of ls-refs is as follows:
> > +
> > +output = *ref
> > +flush-pkt
> > +ref = PKT-LINE((tip | peeled) LF)
> > +tip = obj-id SP refname (SP symref-target)
> > +peeled = obj-id SP refname "^{}"
> > +
> > +symref = PKT-LINE("symref" SP symbolic-ref SP resolved-ref LF)
> > +shallow = PKT-LINE("shallow" SP obj-id LF)
> 
> Do you want to talk about ordering requirements on this?
> I think packed-refs has one, but I'm not sure it matters here
> where the client or server sorts it.
> 
> Are there any provisions for compressing the renames, like in the
> reftable spec or in index-v4 ?

Not currently but it would be rather easy to just add a feature to
ls-refs to transmit the resultant list of refs into something like
reftable.  So this is something that can be added later.

> 
> It doesn't need to be in the initial version.  Just asking.  We could
> always add a "ls-refs-2" command that builds upon this.
> 
> Thanks,
> Jeff

-- 
Brandon Williams

Re: [PATCH 12/26] ls-refs: introduce ls-refs server command

[Jeff Hostetler]

On 1/2/2018 7:18 PM, Brandon Williams wrote:

Introduce the ls-refs server command.  In protocol v2, the ls-refs
command is used to request the ref advertisement from the server.  Since
it is a command which can be requested (as opposed to mandatory in v1),
a client can sent a number of parameters in its request to limit the ref
advertisement based on provided ref-patterns.

Signed-off-by: Brandon Williams 
---
  Documentation/technical/protocol-v2.txt | 26 +
  Makefile|  1 +
  ls-refs.c   | 97 +
  ls-refs.h   |  9 +++
  serve.c |  2 +
  5 files changed, 135 insertions(+)
  create mode 100644 ls-refs.c
  create mode 100644 ls-refs.h

diff --git a/Documentation/technical/protocol-v2.txt 
b/Documentation/technical/protocol-v2.txt
index b87ba3816..5f4d0e719 100644
--- a/Documentation/technical/protocol-v2.txt
+++ b/Documentation/technical/protocol-v2.txt
@@ -89,3 +89,29 @@ terminate the connection.
  Commands are the core actions that a client wants to perform (fetch, push,
  etc).  Each command will be provided with a list capabilities and
  arguments as requested by a client.
+
+ Ls-refs
+-
+
+Ls-refs is the command used to request a reference advertisement in v2.
+Unlike the current reference advertisement, ls-refs takes in parameters
+which can be used to limit the refs sent from the server.
+
+Ls-ref takes in the following parameters wraped in packet-lines:
+
+  symrefs: In addition to the object pointed by it, show the underlying
+  ref pointed by it when showing a symbolic ref.
+  peel: Show peeled tags.
+  ref-pattern : When specified, only references matching the
+given patterns are displayed.
+
+The output of ls-refs is as follows:
+
+output = *ref
+flush-pkt
+ref = PKT-LINE((tip | peeled) LF)
+tip = obj-id SP refname (SP symref-target)
+peeled = obj-id SP refname "^{}"
+
+symref = PKT-LINE("symref" SP symbolic-ref SP resolved-ref LF)
+shallow = PKT-LINE("shallow" SP obj-id LF)


Do you want to talk about ordering requirements on this?
I think packed-refs has one, but I'm not sure it matters here
where the client or server sorts it.

Are there any provisions for compressing the renames, like in the
reftable spec or in index-v4 ?

It doesn't need to be in the initial version.  Just asking.  We could
always add a "ls-refs-2" command that builds upon this.

Thanks,
Jeff

Re: [PATCH 12/26] ls-refs: introduce ls-refs server command

[Brandon Williams]

On 01/09, Jonathan Tan wrote:
> On Tue,  2 Jan 2018 16:18:14 -0800
> Brandon Williams  wrote:
> 
> > +  symrefs: In addition to the object pointed by it, show the underlying
> > +  ref pointed by it when showing a symbolic ref.
> > +  peel: Show peeled tags.
> > +  ref-pattern : When specified, only references matching the
> > +given patterns are displayed.
> 
> I notice "symrefs" being tested in patch 13 and "ref-pattern" being
> tested in patch 16. Is it possible to make a test for "peel" as well?
> (Or is it being tested somewhere I didn't notice?)

Really good suggestion.  I'll introduce unit tests for both the
git-serve cmdline as well as for more simple server commands (ls-refs).

-- 
Brandon Williams

Re: [PATCH 12/26] ls-refs: introduce ls-refs server command

[Jonathan Tan]

On Tue,  2 Jan 2018 16:18:14 -0800
Brandon Williams  wrote:

> +  symrefs: In addition to the object pointed by it, show the underlying
> +ref pointed by it when showing a symbolic ref.
> +  peel: Show peeled tags.
> +  ref-pattern : When specified, only references matching the
> +  given patterns are displayed.

I notice "symrefs" being tested in patch 13 and "ref-pattern" being
tested in patch 16. Is it possible to make a test for "peel" as well?
(Or is it being tested somewhere I didn't notice?)

Re: [PATCH 12/26] ls-refs: introduce ls-refs server command

[Brandon Williams]

On 01/03, Stefan Beller wrote:
> On Tue, Jan 2, 2018 at 4:18 PM, Brandon Williams  wrote:
> > Introduce the ls-refs server command.  In protocol v2, the ls-refs
> > command is used to request the ref advertisement from the server.  Since
> > it is a command which can be requested (as opposed to mandatory in v1),
> > a client can sent a number of parameters in its request to limit the ref
> > advertisement based on provided ref-patterns.
> >
> > Signed-off-by: Brandon Williams 
> > ---
> >  Documentation/technical/protocol-v2.txt | 26 +
> >  Makefile|  1 +
> >  ls-refs.c   | 97 
> > +
> >  ls-refs.h   |  9 +++
> 
> Maybe consider putting any served command into a sub directory?
> 
> For example the code in builtin/ has laxer rules w.r.t. die()ing
> as it is a user facing command, whereas some devs want to see
> code at the root of the repo to not die() at all as the eventual goal
> is to have a library there.
> All this code is on the remote side, which also has different traits than
> the code at the root of the git.git repo; non-localisation comes to mind,
> but there might be other aspects as well (security?).

Well if we were to do this then we should move upload-pack and
receive-pack into this same "server code" directory.

> 
> 
> >  serve.c |  2 +
> >  5 files changed, 135 insertions(+)
> >  create mode 100644 ls-refs.c
> >  create mode 100644 ls-refs.h
> >
> > diff --git a/Documentation/technical/protocol-v2.txt 
> > b/Documentation/technical/protocol-v2.txt
> > index b87ba3816..5f4d0e719 100644
> > --- a/Documentation/technical/protocol-v2.txt
> > +++ b/Documentation/technical/protocol-v2.txt
> > @@ -89,3 +89,29 @@ terminate the connection.
> >  Commands are the core actions that a client wants to perform (fetch, push,
> >  etc).  Each command will be provided with a list capabilities and
> >  arguments as requested by a client.
> > +
> > + Ls-refs
> 
> So is it ls-refs or Ls-refs or is any capitalization valid?

"ls-refs"  I'll make sure to change this.
> 
> > +-
> > +
> > +Ls-refs is the command used to request a reference advertisement in v2.
> > +Unlike the current reference advertisement, ls-refs takes in parameters
> > +which can be used to limit the refs sent from the server.
> > +
> > +Ls-ref takes in the following parameters wraped in packet-lines:
> > +
> > +  symrefs: In addition to the object pointed by it, show the underlying
> > +  ref pointed by it when showing a symbolic ref.
> > +  peel: Show peeled tags.
> > +  ref-pattern : When specified, only references matching the
> > +given patterns are displayed.
> 
> What kind of pattern matching is allowed here?
> strictly prefix only, or globbing, regexes?
> Is there a given grammar to follow? Maybe a link to the git
> glossary is or somewhere else might be fine.
> 
> Seeing that we do wildmatch() down there (as opposed to regexes),
> I wonder if it provides an entry for a denial of service attack, by crafting
> a pattern that is very expensive for the server to compute but cheap to
> ask for from a client. (c.f. 94da9193a6 (grep: add support for PCRE v2,
> 2017-06-01, but that is regexes!)
> 
> > +The output of ls-refs is as follows:
> > +
> > +output = *ref
> > +flush-pkt
> > +ref = PKT-LINE((tip | peeled) LF)
> > +tip = obj-id SP refname (SP symref-target)
> > +peeled = obj-id SP refname "^{}"
> > +
> > +symref = PKT-LINE("symref" SP symbolic-ref SP resolved-ref LF)
> > +shallow = PKT-LINE("shallow" SP obj-id LF)
> > diff --git a/Makefile b/Makefile
> > index 5f3b5fe8b..152a73bec 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -820,6 +820,7 @@ LIB_OBJS += list-objects-filter-options.o
> >  LIB_OBJS += ll-merge.o
> >  LIB_OBJS += lockfile.o
> >  LIB_OBJS += log-tree.o
> > +LIB_OBJS += ls-refs.o
> >  LIB_OBJS += mailinfo.o
> >  LIB_OBJS += mailmap.o
> >  LIB_OBJS += match-trees.o
> > diff --git a/ls-refs.c b/ls-refs.c
> > new file mode 100644
> > index 0..ac4904a40
> > --- /dev/null
> > +++ b/ls-refs.c
> > @@ -0,0 +1,97 @@
> > +#include "cache.h"
> > +#include "repository.h"
> > +#include "refs.h"
> > +#include "remote.h"
> > +#include "argv-array.h"
> > +#include "ls-refs.h"
> > +#include "pkt-line.h"
> > +
> > +struct ls_refs_data {
> > +   unsigned peel;
> > +   unsigned symrefs;
> > +   struct argv_array patterns;
> > +};
> > +
> > +/*
> > + * Check if one of the patterns matches the tail part of the ref.
> > + * If no patterns were provided, all refs match.
> > + */
> > +static int ref_match(const struct argv_array *patterns, const char 
> > *refname)
> > +{
> > +   char *pathbuf;
> > +   int i;
> > +
> > +   if (!patterns->argc)
> > +   return 1; /* no restriction */
> > +
> > +   pathbuf = xstrfmt("/%s", 

Re: [PATCH 12/26] ls-refs: introduce ls-refs server command

[Stefan Beller]

On Tue, Jan 2, 2018 at 4:18 PM, Brandon Williams  wrote:
> Introduce the ls-refs server command.  In protocol v2, the ls-refs
> command is used to request the ref advertisement from the server.  Since
> it is a command which can be requested (as opposed to mandatory in v1),
> a client can sent a number of parameters in its request to limit the ref
> advertisement based on provided ref-patterns.
>
> Signed-off-by: Brandon Williams 
> ---
>  Documentation/technical/protocol-v2.txt | 26 +
>  Makefile|  1 +
>  ls-refs.c   | 97 
> +
>  ls-refs.h   |  9 +++

Maybe consider putting any served command into a sub directory?

For example the code in builtin/ has laxer rules w.r.t. die()ing
as it is a user facing command, whereas some devs want to see
code at the root of the repo to not die() at all as the eventual goal
is to have a library there.
All this code is on the remote side, which also has different traits than
the code at the root of the git.git repo; non-localisation comes to mind,
but there might be other aspects as well (security?).


>  serve.c |  2 +
>  5 files changed, 135 insertions(+)
>  create mode 100644 ls-refs.c
>  create mode 100644 ls-refs.h
>
> diff --git a/Documentation/technical/protocol-v2.txt 
> b/Documentation/technical/protocol-v2.txt
> index b87ba3816..5f4d0e719 100644
> --- a/Documentation/technical/protocol-v2.txt
> +++ b/Documentation/technical/protocol-v2.txt
> @@ -89,3 +89,29 @@ terminate the connection.
>  Commands are the core actions that a client wants to perform (fetch, push,
>  etc).  Each command will be provided with a list capabilities and
>  arguments as requested by a client.
> +
> + Ls-refs

So is it ls-refs or Ls-refs or is any capitalization valid?

> +-
> +
> +Ls-refs is the command used to request a reference advertisement in v2.
> +Unlike the current reference advertisement, ls-refs takes in parameters
> +which can be used to limit the refs sent from the server.
> +
> +Ls-ref takes in the following parameters wraped in packet-lines:
> +
> +  symrefs: In addition to the object pointed by it, show the underlying
> +  ref pointed by it when showing a symbolic ref.
> +  peel: Show peeled tags.
> +  ref-pattern : When specified, only references matching the
> +given patterns are displayed.

What kind of pattern matching is allowed here?
strictly prefix only, or globbing, regexes?
Is there a given grammar to follow? Maybe a link to the git
glossary is or somewhere else might be fine.

Seeing that we do wildmatch() down there (as opposed to regexes),
I wonder if it provides an entry for a denial of service attack, by crafting
a pattern that is very expensive for the server to compute but cheap to
ask for from a client. (c.f. 94da9193a6 (grep: add support for PCRE v2,
2017-06-01, but that is regexes!)

> +The output of ls-refs is as follows:
> +
> +output = *ref
> +flush-pkt
> +ref = PKT-LINE((tip | peeled) LF)
> +tip = obj-id SP refname (SP symref-target)
> +peeled = obj-id SP refname "^{}"
> +
> +symref = PKT-LINE("symref" SP symbolic-ref SP resolved-ref LF)
> +shallow = PKT-LINE("shallow" SP obj-id LF)
> diff --git a/Makefile b/Makefile
> index 5f3b5fe8b..152a73bec 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -820,6 +820,7 @@ LIB_OBJS += list-objects-filter-options.o
>  LIB_OBJS += ll-merge.o
>  LIB_OBJS += lockfile.o
>  LIB_OBJS += log-tree.o
> +LIB_OBJS += ls-refs.o
>  LIB_OBJS += mailinfo.o
>  LIB_OBJS += mailmap.o
>  LIB_OBJS += match-trees.o
> diff --git a/ls-refs.c b/ls-refs.c
> new file mode 100644
> index 0..ac4904a40
> --- /dev/null
> +++ b/ls-refs.c
> @@ -0,0 +1,97 @@
> +#include "cache.h"
> +#include "repository.h"
> +#include "refs.h"
> +#include "remote.h"
> +#include "argv-array.h"
> +#include "ls-refs.h"
> +#include "pkt-line.h"
> +
> +struct ls_refs_data {
> +   unsigned peel;
> +   unsigned symrefs;
> +   struct argv_array patterns;
> +};
> +
> +/*
> + * Check if one of the patterns matches the tail part of the ref.
> + * If no patterns were provided, all refs match.
> + */
> +static int ref_match(const struct argv_array *patterns, const char *refname)
> +{
> +   char *pathbuf;
> +   int i;
> +
> +   if (!patterns->argc)
> +   return 1; /* no restriction */
> +
> +   pathbuf = xstrfmt("/%s", refname);
> +   for (i = 0; i < patterns->argc; i++) {
> +   if (!wildmatch(patterns->argv[i], pathbuf, 0)) {
> +   free(pathbuf);
> +   return 1;
> +   }
> +   }
> +   free(pathbuf);
> +   return 0;
> +}
> +
> +static int send_ref(const char *refname, const struct object_id *oid,
> +   int flag, void *cb_data)
> +{
> +   struct ls_refs_data *data = 

[PATCH 12/26] ls-refs: introduce ls-refs server command

[Brandon Williams]

Introduce the ls-refs server command.  In protocol v2, the ls-refs
command is used to request the ref advertisement from the server.  Since
it is a command which can be requested (as opposed to mandatory in v1),
a client can sent a number of parameters in its request to limit the ref
advertisement based on provided ref-patterns.

Signed-off-by: Brandon Williams 
---
 Documentation/technical/protocol-v2.txt | 26 +
 Makefile|  1 +
 ls-refs.c   | 97 +
 ls-refs.h   |  9 +++
 serve.c |  2 +
 5 files changed, 135 insertions(+)
 create mode 100644 ls-refs.c
 create mode 100644 ls-refs.h

diff --git a/Documentation/technical/protocol-v2.txt 
b/Documentation/technical/protocol-v2.txt
index b87ba3816..5f4d0e719 100644
--- a/Documentation/technical/protocol-v2.txt
+++ b/Documentation/technical/protocol-v2.txt
@@ -89,3 +89,29 @@ terminate the connection.
 Commands are the core actions that a client wants to perform (fetch, push,
 etc).  Each command will be provided with a list capabilities and
 arguments as requested by a client.
+
+ Ls-refs
+-
+
+Ls-refs is the command used to request a reference advertisement in v2.
+Unlike the current reference advertisement, ls-refs takes in parameters
+which can be used to limit the refs sent from the server.
+
+Ls-ref takes in the following parameters wraped in packet-lines:
+
+  symrefs: In addition to the object pointed by it, show the underlying
+  ref pointed by it when showing a symbolic ref.
+  peel: Show peeled tags.
+  ref-pattern : When specified, only references matching the
+given patterns are displayed.
+
+The output of ls-refs is as follows:
+
+output = *ref
+flush-pkt
+ref = PKT-LINE((tip | peeled) LF)
+tip = obj-id SP refname (SP symref-target)
+peeled = obj-id SP refname "^{}"
+
+symref = PKT-LINE("symref" SP symbolic-ref SP resolved-ref LF)
+shallow = PKT-LINE("shallow" SP obj-id LF)
diff --git a/Makefile b/Makefile
index 5f3b5fe8b..152a73bec 100644
--- a/Makefile
+++ b/Makefile
@@ -820,6 +820,7 @@ LIB_OBJS += list-objects-filter-options.o
 LIB_OBJS += ll-merge.o
 LIB_OBJS += lockfile.o
 LIB_OBJS += log-tree.o
+LIB_OBJS += ls-refs.o
 LIB_OBJS += mailinfo.o
 LIB_OBJS += mailmap.o
 LIB_OBJS += match-trees.o
diff --git a/ls-refs.c b/ls-refs.c
new file mode 100644
index 0..ac4904a40
--- /dev/null
+++ b/ls-refs.c
@@ -0,0 +1,97 @@
+#include "cache.h"
+#include "repository.h"
+#include "refs.h"
+#include "remote.h"
+#include "argv-array.h"
+#include "ls-refs.h"
+#include "pkt-line.h"
+
+struct ls_refs_data {
+   unsigned peel;
+   unsigned symrefs;
+   struct argv_array patterns;
+};
+
+/*
+ * Check if one of the patterns matches the tail part of the ref.
+ * If no patterns were provided, all refs match.
+ */
+static int ref_match(const struct argv_array *patterns, const char *refname)
+{
+   char *pathbuf;
+   int i;
+
+   if (!patterns->argc)
+   return 1; /* no restriction */
+
+   pathbuf = xstrfmt("/%s", refname);
+   for (i = 0; i < patterns->argc; i++) {
+   if (!wildmatch(patterns->argv[i], pathbuf, 0)) {
+   free(pathbuf);
+   return 1;
+   }
+   }
+   free(pathbuf);
+   return 0;
+}
+
+static int send_ref(const char *refname, const struct object_id *oid,
+   int flag, void *cb_data)
+{
+   struct ls_refs_data *data = cb_data;
+   const char *refname_nons = strip_namespace(refname);
+   struct strbuf refline = STRBUF_INIT;
+
+   if (!ref_match(>patterns, refname))
+   return 0;
+
+   strbuf_addf(, "%s %s", oid_to_hex(oid), refname_nons);
+   if (data->symrefs && flag & REF_ISSYMREF) {
+   struct object_id unused;
+   const char *symref_target = resolve_ref_unsafe(refname, 0,
+  ,
+  );
+
+   if (!symref_target)
+   die("'%s' is a symref but it is not?", refname);
+
+   strbuf_addf(, " %s", symref_target);
+   }
+
+   strbuf_addch(, '\n');
+
+   packet_write(1, refline.buf, refline.len);
+   if (data->peel) {
+   struct object_id peeled;
+   if (!peel_ref(refname, ))
+   packet_write_fmt(1, "%s %s^{}\n", oid_to_hex(),
+refname_nons);
+   }
+
+   strbuf_release();
+   return 0;
+}
+
+int ls_refs(struct repository *r, struct argv_array *keys, struct argv_array 
*args)
+{
+   int i;
+   struct ls_refs_data data = { 0, 0, ARGV_ARRAY_INIT };
+
+   for (i = 0; i < args->argc; i++) {
+   const char *arg = args->argv[i];
+