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

[Yopa Nelly <yopatatchou@gmail.com>]

From: Nelly Yopa <yopatatchou@gmail.com>

When a repository is configured with more than one promisor
remote, Git tries them one after another until the missing
objects have been fetched. The order is currently determined
only by the order in which remote.<name>.promisor entries appear
in the configuration. There is no way to say "try this remote
first" without moving config lines around.

Add a promisor.fetchOrder configuration variable that takes a
comma-separated list of remote names. Remotes named in the list
are tried first, in the order listed. Remotes not named in the
list are tried afterwards, in their original configuration
order. The extensions.partialClone remote is still moved to the
end of the list by promisor_remote_init() unless it is named in
fetchOrder, in which case its listed position wins.

The order is applied inside each of the two passes in
lazy_fetch_objects() first the accepted remotes, then the
rest so the existing "accepted before configured" behavior is
unchanged. Within each pass, the fetchOrder order takes
precedence over the configuration order.

The parsed list is stored in struct promisor_remote_config and
is re-parsed each time the config is read. Instead of sorting
the linked list at fetch time, try_promisor_remotes() walks the
list twice: once in fetchOrder order for the remotes named
there, and once in the original config order for the rest.

Signed-off-by: Yopa Nelly <yopatatchou@gmail.com>
---
 Documentation/config/promisor.adoc |  11 +++
 promisor-remote.c                  | 108 +++++++++++++++++++++++++----
 2 files changed, 107 insertions(+), 12 deletions(-)

diff --git a/Documentation/config/promisor.adoc b/Documentation/config/promisor.adoc
index c1f3d766a6..ccc0925800 100644
--- a/Documentation/config/promisor.adoc
+++ b/Documentation/config/promisor.adoc
@@ -31,6 +31,17 @@ This list has no effect unless the "promisor.advertise" config
 variable is set to "true", and the "name" and "url" fields are always
 advertised regardless of this setting.
 
+`promisor.fetchOrder`::
+	A comma-separated list of promisor remote names. When Git
+	needs to lazily fetch a missing object, it tries the remotes
+	in this list first, in the order listed, then the remaining
+	configured promisor remotes in their configuration order.
+	The `extensions.partialClone` remote is tried last unless it
+	is named in this list.
++
+See linkgit:gitprotocol-v2[5] and the "Partial Clone Design
+Notes" in link:technical/partial-clone.html[Documentation/technical/partial-clone].
+
 promisor.acceptFromServer::
 	Controls which promisor remotes advertised by a server (using the
 	"promisor-remote" protocol capability) a client will accept. By
diff --git a/promisor-remote.c b/promisor-remote.c
index 316d9950aa..c664a48f88 100644
--- a/promisor-remote.c
+++ b/promisor-remote.c
@@ -19,8 +19,11 @@
 struct promisor_remote_config {
 	struct promisor_remote *promisors;
 	struct promisor_remote **promisors_tail;
+	struct string_list fetch_order;
+	int fetch_order_initialized;
 };
 
+
 static int fetch_objects(struct repository *repo,
 			 const char *remote_name,
 			 const struct object_id *oids,
@@ -120,6 +123,29 @@ 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);
+		/*
+		 * The list is re-read from scratch every time the config
+		 * is parsed, so start by clearing any previous entries.
+		 */
+		if (config->fetch_order_initialized)
+			string_list_clear(&config->fetch_order, 0);
+		else
+			string_list_init_nodup(&config->fetch_order);
+
+		value_copy = xstrdup(value);
+		string_list_split_in_place_f(&config->fetch_order, value_copy, ",", -1,
+					     STRING_LIST_SPLIT_TRIM |
+					     STRING_LIST_SPLIT_NONEMPTY);
+		free(value_copy);
+		config->fetch_order_initialized = 1;
+		return 0;
+	}
+
 	if (parse_config_key(var, "remote", &name, &namelen, &subkey) < 0)
 		return 0;
 
@@ -157,6 +183,8 @@ static int promisor_remote_config(const char *var, const char *value,
 	return 0;
 }
 
+
+
 static void promisor_remote_init(struct repository *r)
 {
 	struct promisor_remote_config *config;
@@ -180,6 +208,7 @@ static void promisor_remote_init(struct repository *r)
 		else
 			promisor_remote_new(config, r->repository_format_partial_clone);
 	}
+
 }
 
 void promisor_remote_clear(struct promisor_remote_config *config)
@@ -193,6 +222,9 @@ void promisor_remote_clear(struct promisor_remote_config *config)
 	}
 
 	config->promisors_tail = &config->promisors;
+	if (config->fetch_order_initialized)
+		string_list_clear(&config->fetch_order, 0);
+	config->fetch_order_initialized = 0;
 }
 
 void repo_promisor_remote_reinit(struct repository *r)
@@ -262,6 +294,8 @@ static int remove_fetched_oids(struct repository *repo,
 	return remaining_nr;
 }
 
+
+
 /*
  * Fetch the remaining objects (given in '*remaining_oids', which
  * contains '*remaining_nr' object ids) from the known promisor
@@ -279,6 +313,24 @@ static int remove_fetched_oids(struct repository *repo,
  * Return 1 when all the requested objects have been fetched, 0
  * otherwise.
  */
+static int try_one_remote(struct repository *repo, struct promisor_remote *r,
+			  struct object_id **remaining_oids,
+			  int *remaining_nr, int *to_free, int depth)
+{
+	if (fetch_objects(repo, r->name,
+			  *remaining_oids, *remaining_nr, depth) < 0) {
+		if (*remaining_nr == 1)
+			return 0;
+		*remaining_nr = remove_fetched_oids(repo, remaining_oids,
+						    *remaining_nr, *to_free);
+		if (*remaining_nr) {
+			*to_free = 1;
+			return 0;
+		}
+	}
+	return 1; /* all fetched */
+}
+
 static int try_promisor_remotes(struct repository *repo,
 				struct object_id **remaining_oids,
 				int *remaining_nr,
@@ -286,24 +338,56 @@ 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_config *config = repo->promisor_remote_config;
+	struct promisor_remote *r;
+	size_t i;
+	int fetched;
+
+	/*
+	 * First pass: try the remotes in the order the user listed
+	 * them in promisor.fetchOrder.
+	 */
+	if (config->fetch_order_initialized) {
+		for (i = 0; i < config->fetch_order.nr; i++) {
+			r = promisor_remote_lookup(config,
+						   config->fetch_order.items[i].string,
+						   NULL);
+			if (!r || accepted_only != r->accepted)
+				continue;
 
-	for (; r; r = r->next) {
+			fetched = try_one_remote(repo, r, remaining_oids,
+						 remaining_nr, to_free, depth);
+			if (fetched)
+				return 1;
+		}
+	}
+
+	/*
+	 * Second pass: try the remaining remotes in their config order.
+	 */
+	for (r = config->promisors; r; r = r->next) {
 		if (accepted_only != r->accepted)
 			continue;
-		if (fetch_objects(repo, r->name,
-				  *remaining_oids, *remaining_nr, depth) < 0) {
-			if (*remaining_nr == 1)
-				continue;
-			*remaining_nr = remove_fetched_oids(repo, remaining_oids,
-							    *remaining_nr, *to_free);
-			if (*remaining_nr) {
-				*to_free = 1;
-				continue;
+
+		/* Skip ones we already tried in the first pass. */
+		if (config->fetch_order_initialized) {
+			bool already_tried = false;
+			for (i = 0; i < config->fetch_order.nr; i++) {
+				if (!strcmp(r->name, config->fetch_order.items[i].string)) {
+					already_tried = true;
+					break;
+				}
 			}
+			if (already_tried)
+				continue;
 		}
-		return 1; /* all fetched */
+
+		fetched = try_one_remote(repo, r, remaining_oids,
+					 remaining_nr, to_free, depth);
+		if (fetched)
+			return 1;
 	}
+
 	return 0;
 }
 
-- 
2.43.0