[Outreachy][PATCH] promisor-remote: honor promisor.fetchOrder when trying remotes

[Yopa Nelly <yopatatchou@gmail.com>]

From: Nelly Yopa <yopatatchou@gmail.com>

When a repo has multiple promisor remotes, Git just tries them in
the order they happen to be in the config. If you want to try one
remote before another, the only way is to move lines around in the
config file, which is awkward, especially once you have per-worktree
or system/global configs in play.

This adds a promisor.fetchOrder config variable that takes a
comma-separated list of remote names. Remotes listed there are tried
first, in the order they're listed. Remotes that aren't listed are
tried afterwards, in their original config order. The
extensions.partialClone remote still ends up last (that's done by
promisor_remote_move_to_tail() in promisor_remote_init()), unless
someone names it in fetchOrder, in which case its listed position
wins.

The sort happens inside each pass in try_promisor_remotes(), so the
existing "accepted remotes before the rest" behavior is unchanged.
Within each pass, the order from fetchOrder wins.

I put the parser in promisor_remote_config() next to the other
promisor keys. It stores the parsed list in a file-static
string_list. The per-remote order field is set in
promisor_remote_apply_fetch_order(), which runs at the end of
promisor_remote_init() after the partialClone remote has been moved
to the tail. The actual sort is a small stable insertion sort in
promisor_remote_sort(); order == 0 means "not listed" so unnamed
remotes still sort after named ones without losing their relative
order. It's called once at the start of try_promisor_remotes() and
is effectively a no-op on the second pass if the first one already
succeeded.

I didn't validate the names in fetchOrder against the set of
configured promisor remotes. A typo just means that remote doesn't
appear in the list and keeps its default position. That's how other
list-valued config variables in Git behave, and it keeps the parser
small.

No test, and I want to be upfront about why. The t5710 fixture
always has the server satisfy a filtered clone from its own
promisor remote, so the client never actually triggers a lazy fetch
inside that test, which means the ordering change can't be observed
there. I'd need a different fixture, one that gives the client two
promisor remotes and keeps the server out of the picture. I'm happy
to write that as a follow-up if that's the right direction.

Signed-off-by: Yopa Nelly <yopatatchou@gmail.com>
---
 promisor-remote.c | 70 ++++++++++++++++++++++++++++++++++++++++++++++-
 promisor-remote.h |  1 +
 2 files changed, 70 insertions(+), 1 deletion(-)

diff --git a/promisor-remote.c b/promisor-remote.c
index 316d9950aa..56e9eeabe3 100644
--- a/promisor-remote.c
+++ b/promisor-remote.c
@@ -21,6 +21,9 @@ struct promisor_remote_config {
 	struct promisor_remote **promisors_tail;
 };
 
+static struct string_list fetch_order = STRING_LIST_INIT_DUP;
+static int fetch_order_initialized;
+
 static int fetch_objects(struct repository *repo,
 			 const char *remote_name,
 			 const struct object_id *oids,
@@ -120,6 +123,20 @@ static int promisor_remote_config(const char *var, const char *value,
 	size_t namelen;
 	const char *subkey;
 
+    if (!strcmp(var, "promisor.fetchorder")) {
+		char *value_copy;
+
+		if (!value)
+			return config_error_nonbool(var);
+		value_copy = xstrdup(value);
+		string_list_split_in_place_f(&fetch_order, value_copy, ",", -1,
+					     STRING_LIST_SPLIT_TRIM |
+					     STRING_LIST_SPLIT_NONEMPTY);
+		free(value_copy);
+		fetch_order_initialized = 1;
+		return 0;
+	}
+
 	if (parse_config_key(var, "remote", &name, &namelen, &subkey) < 0)
 		return 0;
 
@@ -157,6 +174,24 @@ static int promisor_remote_config(const char *var, const char *value,
 	return 0;
 }
 
+static void promisor_remote_apply_fetch_order(struct promisor_remote_config *config)
+{
+	struct promisor_remote *r;
+	size_t i;
+
+	if (!fetch_order_initialized)
+		return;
+
+	for (i = 0; i < fetch_order.nr; i++) {
+		for (r = config->promisors; r; r = r->next) {
+			if (!strcmp(r->name, fetch_order.items[i].string)) {
+				r->order = i + 1;
+				break;
+			}
+		}
+	}
+}
+
 static void promisor_remote_init(struct repository *r)
 {
 	struct promisor_remote_config *config;
@@ -180,6 +215,8 @@ static void promisor_remote_init(struct repository *r)
 		else
 			promisor_remote_new(config, r->repository_format_partial_clone);
 	}
+
+	promisor_remote_apply_fetch_order(config);
 }
 
 void promisor_remote_clear(struct promisor_remote_config *config)
@@ -262,6 +299,34 @@ static int remove_fetched_oids(struct repository *repo,
 	return remaining_nr;
 }
 
+static void promisor_remote_sort(struct promisor_remote_config *config)
+{
+	struct promisor_remote *sorted = NULL;
+	struct promisor_remote *r = config->promisors;
+
+	if (!fetch_order_initialized)
+		return;
+
+	while (r) {
+		struct promisor_remote *next = r->next;
+		struct promisor_remote **pp = &sorted;
+
+		while (*pp && ((*pp)->order == 0
+			       ? r->order != 0
+			       : r->order == 0 || (*pp)->order <= r->order))
+			pp = &(*pp)->next;
+
+		r->next = *pp;
+		*pp = r;
+		r = next;
+	}
+
+	config->promisors = sorted;
+	config->promisors_tail = &config->promisors;
+	while (*config->promisors_tail)
+		config->promisors_tail = &(*config->promisors_tail)->next;
+}
+
 /*
  * Fetch the remaining objects (given in '*remaining_oids', which
  * contains '*remaining_nr' object ids) from the known promisor
@@ -286,7 +351,10 @@ static int try_promisor_remotes(struct repository *repo,
 				int depth,
 				bool accepted_only)
 {
-	struct promisor_remote *r = repo->promisor_remote_config->promisors;
+	struct promisor_remote *r;
+
+	promisor_remote_sort(repo->promisor_remote_config);
+	r = repo->promisor_remote_config->promisors;
 
 	for (; r; r = r->next) {
 		if (accepted_only != r->accepted)
diff --git a/promisor-remote.h b/promisor-remote.h
index 301f5ac5cb..679a138bb1 100644
--- a/promisor-remote.h
+++ b/promisor-remote.h
@@ -17,6 +17,7 @@ struct promisor_remote {
 	char *partial_clone_filter;
 	char *advertised_filter;
 	unsigned int accepted : 1;
+	int order;
 	const char name[FLEX_ARRAY];
 };
 
-- 
2.43.0