* [Outreachy][PATCH] promisor-remote: honor promisor.fetchOrder when trying remotes
@ 2026-11-04 17:24 Yopa Nelly
2026-11-05 14:30 ` Tanga Joris
` (2 more replies)
0 siblings, 3 replies; 11+ messages in thread
From: Yopa Nelly @ 2026-11-04 17:24 UTC (permalink / raw)
To: git; +Cc: Nelly Yopa
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
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [Outreachy][PATCH] promisor-remote: honor promisor.fetchOrder when trying remotes
2026-11-04 17:24 [Outreachy][PATCH] promisor-remote: honor promisor.fetchOrder when trying remotes Yopa Nelly
@ 2026-11-05 14:30 ` Tanga Joris
2026-11-05 16:31 ` Kaartic Sivaraam
2026-11-05 22:52 ` [Outreachy][PATCH v2] " Yopa Nelly
2 siblings, 0 replies; 11+ messages in thread
From: Tanga Joris @ 2026-11-05 14:30 UTC (permalink / raw)
To: Yopa Nelly; +Cc: git, Kaartic Sivaraam, christian.couder
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
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Outreachy][PATCH] promisor-remote: honor promisor.fetchOrder when trying remotes
2026-11-04 17:24 [Outreachy][PATCH] promisor-remote: honor promisor.fetchOrder when trying remotes Yopa Nelly
2026-11-05 14:30 ` Tanga Joris
@ 2026-11-05 16:31 ` Kaartic Sivaraam
2026-11-05 22:55 ` Tatchou Yopa Nelly
2026-11-05 22:52 ` [Outreachy][PATCH v2] " Yopa Nelly
2 siblings, 1 reply; 11+ messages in thread
From: Kaartic Sivaraam @ 2026-11-05 16:31 UTC (permalink / raw)
To: Yopa Nelly, git
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
^ permalink raw reply [flat|nested] 11+ messages in thread
* [Outreachy][PATCH v2] promisor-remote: honor promisor.fetchOrder when trying remotes
2026-11-04 17:24 [Outreachy][PATCH] promisor-remote: honor promisor.fetchOrder when trying remotes Yopa Nelly
2026-11-05 14:30 ` Tanga Joris
2026-11-05 16:31 ` Kaartic Sivaraam
@ 2026-11-05 22:52 ` Yopa Nelly
2026-11-05 23:09 ` Junio C Hamano
2 siblings, 1 reply; 11+ messages in thread
From: Yopa Nelly @ 2026-11-05 22:52 UTC (permalink / raw)
To: git; +Cc: tangajoris, kaartic.sivaraam, Nelly Yopa
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
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [Outreachy][PATCH] promisor-remote: honor promisor.fetchOrder when trying remotes
2026-11-05 16:31 ` Kaartic Sivaraam
@ 2026-11-05 22:55 ` Tatchou Yopa Nelly
2026-11-13 9:51 ` Kaartic Sivaraam
0 siblings, 1 reply; 11+ messages in thread
From: Tatchou Yopa Nelly @ 2026-11-05 22:55 UTC (permalink / raw)
To: Kaartic Sivaraam; +Cc: git
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
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Outreachy][PATCH v2] promisor-remote: honor promisor.fetchOrder when trying remotes
2026-11-05 22:52 ` [Outreachy][PATCH v2] " Yopa Nelly
@ 2026-11-05 23:09 ` Junio C Hamano
2026-11-13 17:10 ` Christian Couder
0 siblings, 1 reply; 11+ messages in thread
From: Junio C Hamano @ 2026-11-05 23:09 UTC (permalink / raw)
To: Christian Couder; +Cc: Yopa Nelly, git, tangajoris, kaartic.sivaraam
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;
> }
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Outreachy][PATCH] promisor-remote: honor promisor.fetchOrder when trying remotes
2026-11-05 22:55 ` Tatchou Yopa Nelly
@ 2026-11-13 9:51 ` Kaartic Sivaraam
2026-11-13 12:13 ` Tatchou Yopa Nelly
0 siblings, 1 reply; 11+ messages in thread
From: Kaartic Sivaraam @ 2026-11-13 9:51 UTC (permalink / raw)
To: Tatchou Yopa Nelly; +Cc: git
On 11/6/26 04:25, Tatchou Yopa Nelly wrote:
> Hi Tanga,
>
Next time try to make sure the person you are writing to is in the To/Cc
fields as appropriate. Otherwise, there is a chance that they will not
even receive your message since not everyone reads the list by
subscribing to it.> Kaartic, I've also re-read
SubmittingPatches#describe-changes and
> rewritten the commit message to describe the change rather than
> address the reviewers directly.
That is good!
> 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?
>
I think treating the current patch as a RFC patch and discussing the
shape is a good idea. The only concern I have is that this patch is
submitted in the context of Outreachy and the candidate that we may
potentially pick will work[note 1] on this during their Outreachy
internship period. So, discussing this now sounds a little premature to me
Of course, I don't mean to suggest that no two people should work on
related areas. I'm just thinking it might be ideal to avoid premature
work among Outreachy contributors before even the Outreachy selections
are out.
[note 1]: During the candidate discussion, it may also happen that we
may not pick a candidate for the project. It is totally fine to work on
this in that case as there will be no conflicts. But none of this is
clear now, TBH.
--
Sivaraam
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Outreachy][PATCH] promisor-remote: honor promisor.fetchOrder when trying remotes
2026-11-13 9:51 ` Kaartic Sivaraam
@ 2026-11-13 12:13 ` Tatchou Yopa Nelly
0 siblings, 0 replies; 11+ messages in thread
From: Tatchou Yopa Nelly @ 2026-11-13 12:13 UTC (permalink / raw)
To: Kaartic Sivaraam; +Cc: git
Hi Kaartic,
Thanks for the clarification, and for pointing out the
To/Cc issue in my earlier reply; I'll be more careful about
that going forward.
Treating the current patch as an RFC prototype and leaving the
actual implementation work for the internship makes sense to me.
I won't push further on it before the Outreachy selections are
out. I'll keep an eye on the list in case anything changes.
Thanks,
Yopa Nelly
On Fri, 13 Nov 2026 at 10:51, Kaartic Sivaraam
<kaartic.sivaraam@gmail.com> wrote:
>
> On 11/6/26 04:25, Tatchou Yopa Nelly wrote:
> > Hi Tanga,
> >
>
> Next time try to make sure the person you are writing to is in the To/Cc
> fields as appropriate. Otherwise, there is a chance that they will not
> even receive your message since not everyone reads the list by
> subscribing to it.> Kaartic, I've also re-read
> SubmittingPatches#describe-changes and
> > rewritten the commit message to describe the change rather than
> > address the reviewers directly.
>
> That is good!
>
> > 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?
> >
>
> I think treating the current patch as a RFC patch and discussing the
> shape is a good idea. The only concern I have is that this patch is
> submitted in the context of Outreachy and the candidate that we may
> potentially pick will work[note 1] on this during their Outreachy
> internship period. So, discussing this now sounds a little premature to me
>
> Of course, I don't mean to suggest that no two people should work on
> related areas. I'm just thinking it might be ideal to avoid premature
> work among Outreachy contributors before even the Outreachy selections
> are out.
>
>
> [note 1]: During the candidate discussion, it may also happen that we
> may not pick a candidate for the project. It is totally fine to work on
> this in that case as there will be no conflicts. But none of this is
> clear now, TBH.
>
> --
> Sivaraam
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Outreachy][PATCH v2] promisor-remote: honor promisor.fetchOrder when trying remotes
2026-11-05 23:09 ` Junio C Hamano
@ 2026-11-13 17:10 ` Christian Couder
2026-11-13 19:46 ` Tatchou Yopa Nelly
2026-11-13 19:58 ` Junio C Hamano
0 siblings, 2 replies; 11+ messages in thread
From: Christian Couder @ 2026-11-13 17:10 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Yopa Nelly, git, tangajoris, kaartic.sivaraam
On Fri, Nov 6, 2026 at 12:09 AM Junio C Hamano <gitster@pobox.com> wrote:
>
> 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?
Sorry, for the late reply. I didn't have much time this week.
First in https://git.github.io/General-Microproject-Information/ we
have a "Don’t work on a proposed project right away" section, but
unfortunately this patch is addressing the main issue of a project we
are proposing.
In general I think that there should be a design doc or a long
discussion before we start implementing something, because the problem
could be complex. When you are fetching from different remotes, remote
A might want the client to get some objects from a set of promisor
remotes, like A1 and A2, while remote B, might advertise others like
B1, B2 and B3. Also after a few regular fetches from let's say A and
B, the client might need to lazy fetch some objects. Then it's not
clear which order would be best. A single order might not work well.
The best order might depend on the remote from which related objects
were fetched.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Outreachy][PATCH v2] promisor-remote: honor promisor.fetchOrder when trying remotes
2026-11-13 17:10 ` Christian Couder
@ 2026-11-13 19:46 ` Tatchou Yopa Nelly
2026-11-13 19:58 ` Junio C Hamano
1 sibling, 0 replies; 11+ messages in thread
From: Tatchou Yopa Nelly @ 2026-11-13 19:46 UTC (permalink / raw)
To: Christian Couder; +Cc: Junio C Hamano, git, tangajoris, kaartic.sivaraam
Hi Christian,
Thanks for the reply, and for pointing me to the "Don't work on a
proposed project right away" section I read it when I first found
the project page but I misread it as only applying to projects
someone else was already working on, not to the main problem of the
project I was applying for. That was my mistake.
Your description of the real problem is very clear. I had not
considered that the best order could depend on which remote the
related objects came from, and that a single list would not be able
to express that. I can see now why this needs a design discussion
before any code.
I'll leave the patch as a prototype and wait for the Outreachy
selections before touching it again, as Kaartic suggested.
Thanks,
Yopa Nelly
On Fri, 13 Nov 2026 at 18:10, Christian Couder
<christian.couder@gmail.com> wrote:
>
> On Fri, Nov 6, 2026 at 12:09 AM Junio C Hamano <gitster@pobox.com> wrote:
> >
> > 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?
>
> Sorry, for the late reply. I didn't have much time this week.
>
> First in https://git.github.io/General-Microproject-Information/ we
> have a "Don’t work on a proposed project right away" section, but
> unfortunately this patch is addressing the main issue of a project we
> are proposing.
>
> In general I think that there should be a design doc or a long
> discussion before we start implementing something, because the problem
> could be complex. When you are fetching from different remotes, remote
> A might want the client to get some objects from a set of promisor
> remotes, like A1 and A2, while remote B, might advertise others like
> B1, B2 and B3. Also after a few regular fetches from let's say A and
> B, the client might need to lazy fetch some objects. Then it's not
> clear which order would be best. A single order might not work well.
> The best order might depend on the remote from which related objects
> were fetched.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Outreachy][PATCH v2] promisor-remote: honor promisor.fetchOrder when trying remotes
2026-11-13 17:10 ` Christian Couder
2026-11-13 19:46 ` Tatchou Yopa Nelly
@ 2026-11-13 19:58 ` Junio C Hamano
1 sibling, 0 replies; 11+ messages in thread
From: Junio C Hamano @ 2026-11-13 19:58 UTC (permalink / raw)
To: Christian Couder; +Cc: Yopa Nelly, git, tangajoris, kaartic.sivaraam
Christian Couder <christian.couder@gmail.com> writes:
> First in https://git.github.io/General-Microproject-Information/ we
> have a "Don’t work on a proposed project right away" section, but
> unfortunately this patch is addressing the main issue of a project we
> are proposing.
>
> In general I think that there should be a design doc or a long
> discussion before we start implementing something, because the problem
> could be complex.
Thanks for clarification. Perhaps you would want three tier set of
projects next year? Microprojects to help you get your toes into
the community, middle sized projects to let you demonstrate how
capable you are to help you get chosen as an intern, and the real
projects?
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2026-11-13 19:58 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-11-04 17:24 [Outreachy][PATCH] promisor-remote: honor promisor.fetchOrder when trying remotes Yopa Nelly
2026-11-05 14:30 ` Tanga Joris
2026-11-05 16:31 ` Kaartic Sivaraam
2026-11-05 22:55 ` Tatchou Yopa Nelly
2026-11-13 9:51 ` Kaartic Sivaraam
2026-11-13 12:13 ` Tatchou Yopa Nelly
2026-11-05 22:52 ` [Outreachy][PATCH v2] " Yopa Nelly
2026-11-05 23:09 ` Junio C Hamano
2026-11-13 17:10 ` Christian Couder
2026-11-13 19:46 ` Tatchou Yopa Nelly
2026-11-13 19:58 ` Junio C Hamano
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.