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

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

[Yopa Nelly]

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

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

[Tanga Joris]

Hello Nelly,

Thanks for working on this, being able to choose the order of
promisor remotes sounds useful.

I tried your patch on top of master (0ffa3e35c7) in a partial clone,
and I think there is a problem when the new variable is set:

  $ git config promisor.fetchOrder origin
  $ git show HEAD:f
  BUG: string-list.c:340: string_list_split_in_place() called with
strdup_strings

On Wed, 4 Nov 2026 at 20:48, Yopa Nelly <yopatatchou@gmail.com> wrote:
> +               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,

Without the variable it works fine. I think this is because
fetch_order is created with STRING_LIST_INIT_DUP, but
string_list_split_in_place_f() is used on it. Maybe
string_list_split_f() would work here instead?

Also, each time the variable is seen the names are added to the
same list. The list is never cleared, so values from several config
files pile up.

It also looks like the list is file-static, so every repository in the
process (for example submodules) shares it, while the rest of
the promisor config lives in each repository's struct
promisor_remote_config.

> No test, and I want to be upfront about why. The t5710 fixture

Thank you for being upfront about this. I noticed that
t0410-partial-clone.sh already has a test with two promisor remotes
("fetching from another promisor remote"), so maybe a test for the
order could go there. It would also catch the crash above.

I also could not find the new variable in
Documentation/config/promisor.adoc. I suggest adding it there
too.

> +    if (!strcmp(var, "promisor.fetchorder")) {

One small thing: this line is indented with spaces instead of a tab.

Best,
Tanga

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

[Kaartic Sivaraam]

On 11/4/26 22:54, Yopa Nelly wrote:
> 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.
> 

To be honest, only part of the commit describes the change. The rest of 
it reads part like you're talking to the people on the lis. You might 
want to give the SubmittingPatches section a read:

   https://git-scm.com/docs/SubmittingPatches#describe-changes

Also, this patch kind of tries achieve a part of the project without 
actually going through the RFC part of it. That feels a bit strange. Did 
you possibly misunderstand what a microproject is supposed to be? Did 
you possibly read the following guides and have some difficulties with 
parts of it? If so, I'd be glad to hear more about it so that we can fix 
the same.

   https://git.github.io/General-Microproject-Information/

   https://git.github.io/General-Application-Information/

-- 
Sivaraam

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

[Yopa Nelly]

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

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

[Tatchou Yopa Nelly]

Hi Tanga,

Thanks for the careful review you caught real bugs. I've sent a v2
that addresses all of your points:

- fetch_order is now in struct promisor_remote_config, not a
  file-static, so it is per-repository and gets cleared with the
  rest of the promisor config.
- The list is now initialized with string_list_init_nodup() on first
  use, matching what string_list_split_in_place_f() expects, so the
  BUG is gone. I reproduced your exact sequence to confirm.
- The docs entry is added to Documentation/config/promisor.adoc.
- The whitespace is fixed.

I also dropped the linked-list insertion sort. Instead,
try_promisor_remotes() now walks the list twice: once in fetchOrder
order for the named remotes, then in original config order for the
rest. That's simpler than sorting a linked list at fetch time, and
it keeps the accepted/unaccepted two-pass logic unchanged.

I didn't add a test yet. You mentioned t0410-partial-clone.sh already
has a test with two promisor remotes ("fetching from another
promisor remote"). I'll look at whether the ordering can be verified
there, either as part of a v3 or as a follow-up.

Kaartic, I've also re-read SubmittingPatches#describe-changes and
rewritten the commit message to describe the change rather than
address the reviewers directly. On the RFC point: I treated the
Phase 2 config work as the micro-patch and skipped the RFC step
you pointed out. Should I step back and send the design RFC first,
or keep the current patch as a prototype while we discuss the
config shape on the list?

Thanks,
Yopa Nelly


On Thu, 5 Nov 2026 at 17:31, Kaartic Sivaraam
<kaartic.sivaraam@gmail.com> wrote:
>
> On 11/4/26 22:54, Yopa Nelly wrote:
> > 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.
> >
>
> To be honest, only part of the commit describes the change. The rest of
> it reads part like you're talking to the people on the lis. You might
> want to give the SubmittingPatches section a read:
>
>    https://git-scm.com/docs/SubmittingPatches#describe-changes
>
> Also, this patch kind of tries achieve a part of the project without
> actually going through the RFC part of it. That feels a bit strange. Did
> you possibly misunderstand what a microproject is supposed to be? Did
> you possibly read the following guides and have some difficulties with
> parts of it? If so, I'd be glad to hear more about it so that we can fix
> the same.
>
>    https://git.github.io/General-Microproject-Information/
>
>    https://git.github.io/General-Application-Information/
>
> --
> Sivaraam
>

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

[Junio C Hamano]

Christian, I thought you are the go-to person for promisor remote,
and if I am not mistaken, you worked on letting the server side tell
the clients which promisors to use first?  How would this mesh well
with that?

Thanks.

Yopa Nelly <yopatatchou@gmail.com> writes:

> 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;
>  }