Re: [PATCH v4 17/35] ls-remote: pass ref patterns when requesting a remote's refs

[Junio C Hamano]

Brandon Williams  writes:

> Construct an argv_array of the ref patterns supplied via the command
> line and pass them to 'transport_get_remote_refs()' to be used when
> communicating protocol v2 so that the server can limit the ref
> advertisement based on the supplied patterns.
>
> Signed-off-by: Brandon Williams 
> ---
>  builtin/ls-remote.c| 12 ++--
>  refs.c | 14 ++
>  refs.h |  7 +++
>  t/t5702-protocol-v2.sh | 26 ++
>  4 files changed, 57 insertions(+), 2 deletions(-)
>
> diff --git a/builtin/ls-remote.c b/builtin/ls-remote.c
> index c6e9847c5..083ba8b29 100644
> --- a/builtin/ls-remote.c
> +++ b/builtin/ls-remote.c
> @@ -2,6 +2,7 @@
>  #include "cache.h"
>  #include "transport.h"
>  #include "remote.h"
> +#include "refs.h"
>  
>  static const char * const ls_remote_usage[] = {
>   N_("git ls-remote [--heads] [--tags] [--refs] [--upload-pack=]\n"
> @@ -43,6 +44,7 @@ int cmd_ls_remote(int argc, const char **argv, const char 
> *prefix)
>   int show_symref_target = 0;
>   const char *uploadpack = NULL;
>   const char **pattern = NULL;
> + struct argv_array ref_patterns = ARGV_ARRAY_INIT;
>  
>   struct remote *remote;
>   struct transport *transport;
> @@ -74,8 +76,14 @@ int cmd_ls_remote(int argc, const char **argv, const char 
> *prefix)
>   if (argc > 1) {
>   int i;
>   pattern = xcalloc(argc, sizeof(const char *));
> - for (i = 1; i < argc; i++)
> + for (i = 1; i < argc; i++) {
>   pattern[i - 1] = xstrfmt("*/%s", argv[i]);
> +
> + if (strchr(argv[i], '*'))
> + argv_array_push(_patterns, argv[i]);
> + else
> + expand_ref_pattern(_patterns, argv[i]);
> + }
>   }
>  
>   remote = remote_get(dest);
> @@ -96,7 +104,7 @@ int cmd_ls_remote(int argc, const char **argv, const char 
> *prefix)
>   if (uploadpack != NULL)
>   transport_set_option(transport, TRANS_OPT_UPLOADPACK, 
> uploadpack);
>  
> - ref = transport_get_remote_refs(transport, NULL);
> + ref = transport_get_remote_refs(transport, _patterns);

Yup, this is a logical and an obvious conclusion of the past handful
of steps ;-) I actually was wondering why the previous step didn't
do this already, but the resulting series is easier to understand if
this is kept as a separate step.

However, this also means that traditional pattern language ls-remote
used to support dictates what ls-refs command over the wire can
take, which may not be optimal.

[PATCH v4 17/35] ls-remote: pass ref patterns when requesting a remote's refs

[Brandon Williams]

Construct an argv_array of the ref patterns supplied via the command
line and pass them to 'transport_get_remote_refs()' to be used when
communicating protocol v2 so that the server can limit the ref
advertisement based on the supplied patterns.

Signed-off-by: Brandon Williams 
---
 builtin/ls-remote.c| 12 ++--
 refs.c | 14 ++
 refs.h |  7 +++
 t/t5702-protocol-v2.sh | 26 ++
 4 files changed, 57 insertions(+), 2 deletions(-)

diff --git a/builtin/ls-remote.c b/builtin/ls-remote.c
index c6e9847c5..083ba8b29 100644
--- a/builtin/ls-remote.c
+++ b/builtin/ls-remote.c
@@ -2,6 +2,7 @@
 #include "cache.h"
 #include "transport.h"
 #include "remote.h"
+#include "refs.h"
 
 static const char * const ls_remote_usage[] = {
N_("git ls-remote [--heads] [--tags] [--refs] [--upload-pack=]\n"
@@ -43,6 +44,7 @@ int cmd_ls_remote(int argc, const char **argv, const char 
*prefix)
int show_symref_target = 0;
const char *uploadpack = NULL;
const char **pattern = NULL;
+   struct argv_array ref_patterns = ARGV_ARRAY_INIT;
 
struct remote *remote;
struct transport *transport;
@@ -74,8 +76,14 @@ int cmd_ls_remote(int argc, const char **argv, const char 
*prefix)
if (argc > 1) {
int i;
pattern = xcalloc(argc, sizeof(const char *));
-   for (i = 1; i < argc; i++)
+   for (i = 1; i < argc; i++) {
pattern[i - 1] = xstrfmt("*/%s", argv[i]);
+
+   if (strchr(argv[i], '*'))
+   argv_array_push(_patterns, argv[i]);
+   else
+   expand_ref_pattern(_patterns, argv[i]);
+   }
}
 
remote = remote_get(dest);
@@ -96,7 +104,7 @@ int cmd_ls_remote(int argc, const char **argv, const char 
*prefix)
if (uploadpack != NULL)
transport_set_option(transport, TRANS_OPT_UPLOADPACK, 
uploadpack);
 
-   ref = transport_get_remote_refs(transport, NULL);
+   ref = transport_get_remote_refs(transport, _patterns);
if (transport_disconnect(transport))
return 1;
 
diff --git a/refs.c b/refs.c
index 20ba82b43..58e9f88fb 100644
--- a/refs.c
+++ b/refs.c
@@ -13,6 +13,7 @@
 #include "tag.h"
 #include "submodule.h"
 #include "worktree.h"
+#include "argv-array.h"
 
 /*
  * List of all available backends
@@ -501,6 +502,19 @@ int refname_match(const char *abbrev_name, const char 
*full_name)
return 0;
 }
 
+/*
+ * Given a 'pattern' expand it by the rules in 'ref_rev_parse_rules' and add
+ * the results to 'patterns'
+ */
+void expand_ref_pattern(struct argv_array *patterns, const char *pattern)
+{
+   const char **p;
+   for (p = ref_rev_parse_rules; *p; p++) {
+   int len = strlen(pattern);
+   argv_array_pushf(patterns, *p, len, pattern);
+   }
+}
+
 /*
  * *string and *len will only be substituted, and *string returned (for
  * later free()ing) if the string passed in is a magic short-hand form
diff --git a/refs.h b/refs.h
index 01be5ae32..292ca35ce 100644
--- a/refs.h
+++ b/refs.h
@@ -139,6 +139,13 @@ int resolve_gitlink_ref(const char *submodule, const char 
*refname,
  */
 int refname_match(const char *abbrev_name, const char *full_name);
 
+/*
+ * Given a 'pattern' expand it by the rules in 'ref_rev_parse_rules' and add
+ * the results to 'patterns'
+ */
+struct argv_array;
+void expand_ref_pattern(struct argv_array *patterns, const char *pattern);
+
 int expand_ref(const char *str, int len, struct object_id *oid, char **ref);
 int dwim_ref(const char *str, int len, struct object_id *oid, char **ref);
 int dwim_log(const char *str, int len, struct object_id *oid, char **ref);
diff --git a/t/t5702-protocol-v2.sh b/t/t5702-protocol-v2.sh
index dc5f813be..562610fd2 100755
--- a/t/t5702-protocol-v2.sh
+++ b/t/t5702-protocol-v2.sh
@@ -32,6 +32,19 @@ test_expect_success 'list refs with git:// using protocol 
v2' '
test_cmp actual expect
 '
 
+test_expect_success 'ref advertisment is filtered with ls-remote using 
protocol v2' '
+   test_when_finished "rm -f log" &&
+
+   GIT_TRACE_PACKET="$(pwd)/log" git -c protocol.version=2 \
+   ls-remote "$GIT_DAEMON_URL/parent" master >actual &&
+
+   cat >expect <<-EOF &&
+   $(git -C "$daemon_parent" rev-parse refs/heads/master)$(printf 
"\t")refs/heads/master
+   EOF
+
+   test_cmp actual expect
+'
+
 stop_git_daemon
 
 # Test protocol v2 with 'file://' transport
@@ -54,4 +67,17 @@ test_expect_success 'list refs with file:// using protocol 
v2' '
test_cmp actual expect
 '
 
+test_expect_success 'ref advertisment is filtered with ls-remote using 
protocol v2' '
+   test_when_finished "rm -f log" &&
+
+   GIT_TRACE_PACKET="$(pwd)/log" git -c protocol.version=2 \
+   ls-remote