Merge branch 'ds/maintenance-prefetch-fix' into seen

The prefetch task in "git maintenance" assumed that "git fetch"
from any remote would fetch all its local branches, which would
fetch too much if the user is interested in only a subset of
branches there.

* ds/maintenance-prefetch-fix:
  maintenance: allow custom refspecs during prefetch
  test-tool: test refspec input/output
  refspec: output a refspec item
  test-lib: use exact match for test_subcommand
  maintenance: simplify prefetch logic

Documentation/git-maintenance.txt

@@ -94,8 +94,7 @@ prefetch::
 	objects from all registered remotes. For each remote, a `git fetch`
 	command is run. The refmap is custom to avoid updating local or remote
 	branches (those in `refs/heads` or `refs/remotes`). Instead, the
-	remote refs are stored in `refs/prefetch/<remote>/`. Also, tags are
-	not updated.
+	refs are stored in `refs/prefetch/`. Also, tags are not updated.
 +
 This is done to avoid disrupting the remote-tracking branches. The end users
 expect these refs to stay unmoved unless they initiate a fetch.  With prefetch

Makefile

@@ -735,6 +735,7 @@ TEST_BUILTINS_OBJS += test-reach.o
 TEST_BUILTINS_OBJS += test-read-cache.o
 TEST_BUILTINS_OBJS += test-read-graph.o
 TEST_BUILTINS_OBJS += test-read-midx.o
+TEST_BUILTINS_OBJS += test-refspec.o
 TEST_BUILTINS_OBJS += test-ref-store.o
 TEST_BUILTINS_OBJS += test-regex.o
 TEST_BUILTINS_OBJS += test-repository.o

builtin/gc.c

@@ -32,6 +32,7 @@
 #include "remote.h"
 #include "object-store.h"
 #include "exec-cmd.h"
+#include "refspec.h"
 
 #define FAILED_RUN "failed to run %s"
 
@@ -873,55 +874,72 @@ static int maintenance_task_commit_graph(struct maintenance_run_opts *opts)
 	return 0;
 }
 
-static int fetch_remote(const char *remote, struct maintenance_run_opts *opts)
+static int fetch_remote(struct remote *remote, void *cbdata)
 {
+	struct maintenance_run_opts *opts = cbdata;
 	struct child_process child = CHILD_PROCESS_INIT;
+	int i;
 
 	child.git_cmd = 1;
-	strvec_pushl(&child.args, "fetch", remote, "--prune", "--no-tags",
+	strvec_pushl(&child.args, "fetch", remote->name, "--prune", "--no-tags",
 		     "--no-write-fetch-head", "--recurse-submodules=no",
 		     "--refmap=", NULL);
 
 	if (opts->quiet)
 		strvec_push(&child.args, "--quiet");
 
-	strvec_pushf(&child.args, "+refs/heads/*:refs/prefetch/%s/*", remote);
+	for (i = 0; i < remote->fetch.nr; i++) {
+		struct refspec_item replace;
+		struct refspec_item *rsi = &remote->fetch.items[i];
+		struct strbuf new_dst = STRBUF_INIT;
+		size_t ignore_len = 0;
+		char *replace_string;
 
-	return !!run_command(&child);
-}
+		if (rsi->negative) {
+			strvec_push(&child.args, remote->fetch.raw[i]);
+			continue;
+		}
 
-static int append_remote(struct remote *remote, void *cbdata)
-{
-	struct string_list *remotes = (struct string_list *)cbdata;
+		refspec_item_init(&replace, remote->fetch.raw[i], 1);
 
-	string_list_append(remotes, remote->name);
-	return 0;
+		/*
+		 * If a refspec dst starts with "refs/" at the start,
+		 * then we will replace "refs/" with "refs/prefetch/".
+		 * Otherwise, we will prepend the dst string with
+		 * "refs/prefetch/".
+		 */
+		if (!strncmp(replace.dst, "refs/", 5))
+			ignore_len = 5;
+
+		strbuf_addstr(&new_dst, "refs/prefetch/");
+		strbuf_addstr(&new_dst, replace.dst + ignore_len);
+		free(replace.dst);
+		replace.dst = strbuf_detach(&new_dst, NULL);
+
+		replace_string = refspec_item_format(&replace);
+		strvec_push(&child.args, replace_string);
+		free(replace_string);
+
+		refspec_item_clear(&replace);
+	}
+
+	return !!run_command(&child);
 }
 
 static int maintenance_task_prefetch(struct maintenance_run_opts *opts)
 {
-	int result = 0;
-	struct string_list_item *item;
-	struct string_list remotes = STRING_LIST_INIT_DUP;
-
 	git_config_set_multivar_gently("log.excludedecoration",
 					"refs/prefetch/",
 					"refs/prefetch/",
 					CONFIG_FLAGS_FIXED_VALUE |
 					CONFIG_FLAGS_MULTI_REPLACE);
 
-	if (for_each_remote(append_remote, &remotes)) {
-		error(_("failed to fill remotes"));
-		result = 1;
-		goto cleanup;
+	if (for_each_remote(fetch_remote, opts)) {
+		error(_("failed to prefetch remotes"));
+		return 1;
 	}
 
-	for_each_string_list_item(item, &remotes)
-		result |= fetch_remote(item->string, opts);
-
-cleanup:
-	string_list_clear(&remotes, 0);
-	return result;
+	return 0;
 }
 
 static int maintenance_task_gc(struct maintenance_run_opts *opts)

refspec.c

@@ -180,6 +180,29 @@ void refspec_item_clear(struct refspec_item *item)
 	item->exact_sha1 = 0;
 }
 
+char *refspec_item_format(const struct refspec_item *rsi)
+{
+	struct strbuf buf = STRBUF_INIT;
+
+	if (rsi->matching)
+		return xstrdup(":");
+
+	if (rsi->negative)
+		strbuf_addch(&buf, '^');
+	else if (rsi->force)
+		strbuf_addch(&buf, '+');
+
+	if (rsi->src)
+		strbuf_addstr(&buf, rsi->src);
+
+	if (rsi->dst) {
+		strbuf_addch(&buf, ':');
+		strbuf_addstr(&buf, rsi->dst);
+	}
+
+	return strbuf_detach(&buf, NULL);
+}
+
 void refspec_init(struct refspec *rs, int fetch)
 {
 	memset(rs, 0, sizeof(*rs));

refspec.h

@@ -56,6 +56,8 @@ int refspec_item_init(struct refspec_item *item, const char *refspec,
 void refspec_item_init_or_die(struct refspec_item *item, const char *refspec,
 			      int fetch);
 void refspec_item_clear(struct refspec_item *item);
+char *refspec_item_format(const struct refspec_item *rsi);
+
 void refspec_init(struct refspec *rs, int fetch);
 void refspec_append(struct refspec *rs, const char *refspec);
 __attribute__((format (printf,2,3)))

t/helper/test-refspec.c

@@ -0,0 +1,44 @@
+#include "cache.h"
+#include "parse-options.h"
+#include "refspec.h"
+#include "strbuf.h"
+#include "test-tool.h"
+
+static const char * const refspec_usage[] = {
+	N_("test-tool refspec [--fetch]"),
+	NULL
+};
+
+int cmd__refspec(int argc, const char **argv)
+{
+	struct strbuf line = STRBUF_INIT;
+	int fetch = 0;
+
+	struct option refspec_options [] = {
+		OPT_BOOL(0, "fetch", &fetch,
+			 N_("enable the 'fetch' option for parsing refpecs")),
+		OPT_END()
+	};
+
+	argc = parse_options(argc, argv, NULL, refspec_options,
+			     refspec_usage, 0);
+
+	while (strbuf_getline(&line, stdin) != EOF) {
+		struct refspec_item rsi;
+		char *buf;
+
+		if (!refspec_item_init(&rsi, line.buf, fetch)) {
+			printf("failed to parse %s\n", line.buf);
+			continue;
+		}
+
+		buf = refspec_item_format(&rsi);
+		printf("%s\n", buf);
+		free(buf);
+
+		refspec_item_clear(&rsi);
+	}
+
+	strbuf_release(&line);
+	return 0;
+}

t/helper/test-tool.c

@@ -56,6 +56,7 @@ static struct test_cmd cmds[] = {
 	{ "read-cache", cmd__read_cache },
 	{ "read-graph", cmd__read_graph },
 	{ "read-midx", cmd__read_midx },
+	{ "refspec", cmd__refspec },
 	{ "ref-store", cmd__ref_store },
 	{ "regex", cmd__regex },
 	{ "repository", cmd__repository },

t/helper/test-tool.h

@@ -45,6 +45,7 @@ int cmd__reach(int argc, const char **argv);
 int cmd__read_cache(int argc, const char **argv);
 int cmd__read_graph(int argc, const char **argv);
 int cmd__read_midx(int argc, const char **argv);
+int cmd__refspec(int argc, const char **argv);
 int cmd__ref_store(int argc, const char **argv);
 int cmd__regex(int argc, const char **argv);
 int cmd__repository(int argc, const char **argv);

t/t5511-refspec.sh

@@ -93,4 +93,45 @@ test_refspec fetch "refs/heads/${good}"
 bad=$(printf '\011tab')
 test_refspec fetch "refs/heads/${bad}"				invalid
 
+test_expect_success 'test input/output round trip' '
+	cat >input <<-\EOF &&
+	+refs/heads/*:refs/remotes/origin/*
+	refs/heads/*:refs/remotes/origin/*
+	refs/heads/main:refs/remotes/frotz/xyzzy
+	:refs/remotes/frotz/deleteme
+	^refs/heads/secrets
+	refs/heads/secret:refs/heads/translated
+	refs/heads/secret:heads/translated
+	refs/heads/secret:remotes/translated
+	secret:translated
+	refs/heads/*:remotes/xxy/*
+	refs/heads*/for-linus:refs/remotes/mine/*
+	2e36527f23b7f6ae15e6f21ac3b08bf3fed6ee48:refs/heads/fixed
+	HEAD
+	@
+	:
+	EOF
+	cat >expect <<-\EOF &&
+	+refs/heads/*:refs/remotes/origin/*
+	refs/heads/*:refs/remotes/origin/*
+	refs/heads/main:refs/remotes/frotz/xyzzy
+	:refs/remotes/frotz/deleteme
+	^refs/heads/secrets
+	refs/heads/secret:refs/heads/translated
+	refs/heads/secret:heads/translated
+	refs/heads/secret:remotes/translated
+	secret:translated
+	refs/heads/*:remotes/xxy/*
+	refs/heads*/for-linus:refs/remotes/mine/*
+	2e36527f23b7f6ae15e6f21ac3b08bf3fed6ee48:refs/heads/fixed
+	HEAD
+	HEAD
+	:
+	EOF
+	test-tool refspec <input >output &&
+	test_cmp expect output &&
+	test-tool refspec --fetch <input >output &&
+	test_cmp expect output
+'
+
 test_done

t/t7900-maintenance.sh

@@ -142,20 +142,51 @@ test_expect_success 'prefetch multiple remotes' '
 	test_commit -C clone2 two &&
 	GIT_TRACE2_EVENT="$(pwd)/run-prefetch.txt" git maintenance run --task=prefetch 2>/dev/null &&
 	fetchargs="--prune --no-tags --no-write-fetch-head --recurse-submodules=no --refmap= --quiet" &&
-	test_subcommand git fetch remote1 $fetchargs +refs/heads/\\*:refs/prefetch/remote1/\\* <run-prefetch.txt &&
-	test_subcommand git fetch remote2 $fetchargs +refs/heads/\\*:refs/prefetch/remote2/\\* <run-prefetch.txt &&
+	test_subcommand git fetch remote1 $fetchargs "+refs/heads/*:refs/prefetch/remotes/remote1/*" <run-prefetch.txt &&
+	test_subcommand git fetch remote2 $fetchargs "+refs/heads/*:refs/prefetch/remotes/remote2/*" <run-prefetch.txt &&
 	test_path_is_missing .git/refs/remotes &&
-	git log prefetch/remote1/one &&
-	git log prefetch/remote2/two &&
+	git log prefetch/remotes/remote1/one &&
+	git log prefetch/remotes/remote2/two &&
 	git fetch --all &&
-	test_cmp_rev refs/remotes/remote1/one refs/prefetch/remote1/one &&
-	test_cmp_rev refs/remotes/remote2/two refs/prefetch/remote2/two &&
+	test_cmp_rev refs/remotes/remote1/one refs/prefetch/remotes/remote1/one &&
+	test_cmp_rev refs/remotes/remote2/two refs/prefetch/remotes/remote2/two &&
 
 	test_cmp_config refs/prefetch/ log.excludedecoration &&
 	git log --oneline --decorate --all >log &&
 	! grep "prefetch" log
 '
 
+test_expect_success 'prefetch custom refspecs' '
+	git -C clone1 branch -f special/fetched HEAD &&
+	git -C clone1 branch -f special/secret/not-fetched HEAD &&
+
+	# create multiple refspecs for remote1
+	git config --add remote.remote1.fetch "+refs/heads/special/fetched:refs/heads/fetched" &&
+	git config --add remote.remote1.fetch "^refs/heads/special/secret/not-fetched" &&
+
+	GIT_TRACE2_EVENT="$(pwd)/prefetch-refspec.txt" git maintenance run --task=prefetch 2>/dev/null &&
+
+	fetchargs="--prune --no-tags --no-write-fetch-head --recurse-submodules=no --refmap= --quiet" &&
+
+	# skips second refspec because it is not a pattern type
+	rs1="+refs/heads/*:refs/prefetch/remotes/remote1/*" &&
+	rs2="+refs/heads/special/fetched:refs/prefetch/heads/fetched" &&
+	rs3="^refs/heads/special/secret/not-fetched" &&
+
+	test_subcommand git fetch remote1 $fetchargs "$rs1" "$rs2" "$rs3" <prefetch-refspec.txt &&
+	test_subcommand git fetch remote2 $fetchargs "+refs/heads/*:refs/prefetch/remotes/remote2/*" <prefetch-refspec.txt &&
+
+	# first refspec is overridden by second
+	test_must_fail git rev-parse refs/prefetch/special/fetched &&
+	git rev-parse refs/prefetch/heads/fetched &&
+
+	# possible incorrect places for the non-fetched ref
+	test_must_fail git rev-parse refs/prefetch/remotes/remote1/secret/not-fetched &&
+	test_must_fail git rev-parse refs/prefetch/remotes/remote1/not-fetched &&
+	test_must_fail git rev-parse refs/heads/secret/not-fetched &&
+	test_must_fail git rev-parse refs/heads/not-fetched
+'
+
 test_expect_success 'prefetch and existing log.excludeDecoration values' '
 	git config --unset-all log.excludeDecoration &&
 	git config log.excludeDecoration refs/remotes/remote1/ &&

t/test-lib-functions.sh

@@ -1652,9 +1652,9 @@ test_subcommand () {
 
 	if test -n "$negate"
 	then
-		! grep "\[$expr\]"
+		! grep -F "[$expr]"
 	else
-		grep "\[$expr\]"
+		grep -F "[$expr]"
 	fi
 }