Thread (329 messages) 329 messages, 12 authors, 2018-03-14

Re: [PATCH v3 13/35] ls-refs: introduce ls-refs server command

From: Jeff King <hidden>
Date: 2018-02-22 09:48:38

On Tue, Feb 06, 2018 at 05:12:50PM -0800, Brandon Williams wrote:
+ls-refs takes in the following parameters wrapped 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 <pattern>
+	When specified, only references matching the one of the provided
+	patterns are displayed.
How do we match those patterns? That's probably an important thing to
include in the spec.

Looking at the code, I see:
+/*
+ * 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)
This kind of tail matching can't quite implement all of the current
behavior. Because we actually do the normal dwim_ref() matching, which
includes stuff like "refs/remotes/%s/HEAD".

The other problem with tail-matching is that it's inefficient on the
server. Ideally we could get a request for "master" and only look up
refs/heads/master, refs/tags/master, etc. And if there are 50,000 refs
in refs/pull, we wouldn't have to process those at all. Of course this
is no worse than the current code, which not only looks at each ref but
actually _sends_ it. But it would be nice if we could fix this.

There's some more discussion in this old thread:

  https://public-inbox.org/git/20161024132932.i42rqn2vlpocqmkq@sigill.intra.peff.net/
+{
+	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;
+}
Does the client have to be aware that we're using wildmatch? I think
they'd need "refs/heads/**" to actually implement what we usually
specify in refspecs as "refs/heads/*". Or does the lack of WM_PATHNAME
make this work with just "*"?

Do we anticipate that the client would left-anchor the refspec like
"/refs/heads/*" so that in theory the server could avoid looking outside
of /refs/heads/?

-Peff
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help