Git development
 help / color / mirror / Atom feed
* [PATCH v2] doc: describe promisor remote fetch order
@ 2026-11-08  7:19 Hussain Sitamouwala
  2026-11-08 18:45 ` Junio C Hamano
                   ` (2 more replies)
  0 siblings, 3 replies; 12+ messages in thread
From: Hussain Sitamouwala @ 2026-11-08  7:19 UTC (permalink / raw)
  To: git; +Cc: gitster, christian.couder, Hussain Sitamouwala

Explain that promisor remotes accepted from a server are tried
before other promisor remotes, and that the remaining objects are
requested from the next remote when a fetch fails. The ordering
was introduced by 8808e61fd3 (promisor-remote: try accepted
remotes before others in get_direct(), 2026-04-07).

Signed-off-by: Hussain Sitamouwala <husainsitamuwala21@gmail.com>
---
Changes since v1:
- shortened the subject and wrapped the commit message
- clarified what "accepted" means and the fetch fallback wording
- checked the wording against try_promisor_remotes()

 Documentation/technical/partial-clone.adoc | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/Documentation/technical/partial-clone.adoc b/Documentation/technical/partial-clone.adoc
index 18718a3840..efe9acac65 100644
--- a/Documentation/technical/partial-clone.adoc
+++ b/Documentation/technical/partial-clone.adoc
@@ -195,7 +195,10 @@ cache servers for fetching missing blobs while continuing to do
 filtered `git-fetch` commands from the central server.
 
 When fetching objects, promisor remotes are tried one after the other
-until all the objects have been fetched.
+until all the objects have been fetched. Promisor remotes that the
+client accepted from a server's advertisement are tried before the
+other promisor remotes. If a fetch from a remote fails, the objects
+that are still missing are requested from the next remote.
 
 Remotes that are considered "promisor" remotes are those specified by
 the following configuration variables:
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH v2] doc: describe promisor remote fetch order
  2026-11-08  7:19 [PATCH v2] doc: describe promisor remote fetch order Hussain Sitamouwala
@ 2026-11-08 18:45 ` Junio C Hamano
  2026-11-08 19:07   ` Hussain Sitamouwala
  2026-11-08 19:03 ` Junio C Hamano
  2026-11-08 19:55 ` [PATCH v3] doc: describe promisor remote fetch order Hussain Sitamouwala
  2 siblings, 1 reply; 12+ messages in thread
From: Junio C Hamano @ 2026-11-08 18:45 UTC (permalink / raw)
  To: Hussain Sitamouwala; +Cc: git, christian.couder

Hussain Sitamouwala <husainsitamuwala21@gmail.com> writes:

> Explain that promisor remotes accepted from a server are tried
> before other promisor remotes, and that the remaining objects are
> requested from the next remote when a fetch fails. The ordering
> was introduced by 8808e61fd3 (promisor-remote: try accepted
> remotes before others in get_direct(), 2026-04-07).
>
> Signed-off-by: Hussain Sitamouwala <husainsitamuwala21@gmail.com>
> ---
> Changes since v1:
> - shortened the subject and wrapped the commit message
> - clarified what "accepted" means and the fetch fallback wording
> - checked the wording against try_promisor_remotes()

Please thread your patches correctly.

This message, which sends the v2 iteration of the patch, should be a
reply to your v1.  However, there is no In-Reply-To (or References)
header linking this message to the message that contained the v1
iteration.  This hurts automation.

Also, it is not considered good manners to send a new iteration
before first finishing the discussion on the reviews you received on
your previous iteration.  You respond to reviews, saying "I will fix
this point", "I think this one is fine as-is because...", and so
forth, wait for responses to your response, and eventually come to
an agreement with those in the discussion.  Only after finishing all
of that do you prepare and send a new iteration.

Thanks.

>  Documentation/technical/partial-clone.adoc | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/technical/partial-clone.adoc b/Documentation/technical/partial-clone.adoc
> index 18718a3840..efe9acac65 100644
> --- a/Documentation/technical/partial-clone.adoc
> +++ b/Documentation/technical/partial-clone.adoc
> @@ -195,7 +195,10 @@ cache servers for fetching missing blobs while continuing to do
>  filtered `git-fetch` commands from the central server.
>  
>  When fetching objects, promisor remotes are tried one after the other
> -until all the objects have been fetched.
> +until all the objects have been fetched. Promisor remotes that the
> +client accepted from a server's advertisement are tried before the
> +other promisor remotes. If a fetch from a remote fails, the objects
> +that are still missing are requested from the next remote.
>  
>  Remotes that are considered "promisor" remotes are those specified by
>  the following configuration variables:

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2] doc: describe promisor remote fetch order
  2026-11-08  7:19 [PATCH v2] doc: describe promisor remote fetch order Hussain Sitamouwala
  2026-11-08 18:45 ` Junio C Hamano
@ 2026-11-08 19:03 ` Junio C Hamano
  2026-11-08 19:12   ` Hussain Sitamouwala
  2026-11-08 19:16   ` Junio C Hamano
  2026-11-08 19:55 ` [PATCH v3] doc: describe promisor remote fetch order Hussain Sitamouwala
  2 siblings, 2 replies; 12+ messages in thread
From: Junio C Hamano @ 2026-11-08 19:03 UTC (permalink / raw)
  To: Hussain Sitamouwala; +Cc: git, christian.couder

Hussain Sitamouwala <husainsitamuwala21@gmail.com> writes:

> Explain that promisor remotes accepted from a server are tried
> before other promisor remotes, and that the remaining objects are
> requested from the next remote when a fetch fails. The ordering
> was introduced by 8808e61fd3 (promisor-remote: try accepted
> remotes before others in get_direct(), 2026-04-07).

Well written.  "next" sounded a bit strange thing to say in the
context, but it was understandable.

Assuming that I understood what the author wanted to say correctly,
that is ;-), so let's double-check.  Would it mean the same thing if
we replace "next remote" with "other promisor remotes"?

>  Documentation/technical/partial-clone.adoc | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/technical/partial-clone.adoc b/Documentation/technical/partial-clone.adoc
> index 18718a3840..efe9acac65 100644
> --- a/Documentation/technical/partial-clone.adoc
> +++ b/Documentation/technical/partial-clone.adoc
> @@ -195,7 +195,10 @@ cache servers for fetching missing blobs while continuing to do
>  filtered `git-fetch` commands from the central server.
>  
>  When fetching objects, promisor remotes are tried one after the other
> -until all the objects have been fetched.
> +until all the objects have been fetched. Promisor remotes that the
> +client accepted from a server's advertisement are tried before the
> +other promisor remotes. If a fetch from a remote fails, the objects
> +that are still missing are requested from the next remote.

The same comment on "next" here.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2] doc: describe promisor remote fetch order
  2026-11-08 18:45 ` Junio C Hamano
@ 2026-11-08 19:07   ` Hussain Sitamouwala
  0 siblings, 0 replies; 12+ messages in thread
From: Hussain Sitamouwala @ 2026-11-08 19:07 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, christian.couder

Hi Junio,

Thanks for the review.

> Please thread your patches correctly.

Sorry about that. I will send later iterations as replies to the
previous one using --in-reply-to.

> it is not considered good manners to send a new iteration before
> first finishing the discussion on the reviews you received on your
> previous iteration.

Understood, and sorry for sending v2 early. I will reply to the
remaining review comments on v1 first, and only send a new version
once we have agreed on the changes.

Thanks,
Hussain

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2] doc: describe promisor remote fetch order
  2026-11-08 19:03 ` Junio C Hamano
@ 2026-11-08 19:12   ` Hussain Sitamouwala
  2026-11-08 19:16   ` Junio C Hamano
  1 sibling, 0 replies; 12+ messages in thread
From: Hussain Sitamouwala @ 2026-11-08 19:12 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, christian.couder

Hi Junio,

Thanks for the review.

> Would it mean the same thing if we replace "next remote" with
> "other promisor remotes"?

Yes, that is what I meant. When a fetch from one promisor remote
fails, the objects that are still missing are requested from the
other promisor remotes, in turn. "Other promisor remotes" is
clearer, and it matches the previous sentence, so I will change
both places in the next version.

Thanks,
Hussain

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2] doc: describe promisor remote fetch order
  2026-11-08 19:03 ` Junio C Hamano
  2026-11-08 19:12   ` Hussain Sitamouwala
@ 2026-11-08 19:16   ` Junio C Hamano
  2026-11-08 19:22     ` Hussain Sitamouwala
  2026-11-08 21:38     ` [PATCH] environment: move assume_unchanged into repo_config_values Hussain Sitamouwala
  1 sibling, 2 replies; 12+ messages in thread
From: Junio C Hamano @ 2026-11-08 19:16 UTC (permalink / raw)
  To: Hussain Sitamouwala; +Cc: git, christian.couder

Junio C Hamano <gitster@pobox.com> writes:

> Hussain Sitamouwala <husainsitamuwala21@gmail.com> writes:
>
>> Explain that promisor remotes accepted from a server are tried
>> before other promisor remotes, and that the remaining objects are
>> requested from the next remote when a fetch fails. The ordering
>
> Well written.  "next" sounded a bit strange thing to say in the
> context, but it was understandable.
>
> Assuming that I understood what the author wanted to say correctly,
> that is ;-), so let's double-check.  Would it mean the same thing if
> we replace "next remote" with "other promisor remotes"?

After thinking about this a bit more, I know why I thought this was
strange.

>>  When fetching objects, promisor remotes are tried one after the other
>> +client accepted from a server's advertisement are tried before the
>> +other promisor remotes. If a fetch from a remote fails, the objects
>> +that are still missing are requested from the next remote.

I think the entire sentence after "If a fetch from a remote fails"
should be removed.  You already established that while fetching,
remotes are tried one after the other.  What does it mean?  We try
promisor A, and if the fetch from it fails to obtain certain
objects, we try promisor B, and then C, and so on.  We already said
that and you clarified in which order these promisors A and B and C 
are consulted.

So, there is absolutely nothing left to be said.  The last sentence,
while it may not say anything wrong, is simply redundant without
adding any new information.


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2] doc: describe promisor remote fetch order
  2026-11-08 19:16   ` Junio C Hamano
@ 2026-11-08 19:22     ` Hussain Sitamouwala
  2026-11-09 15:45       ` Junio C Hamano
  2026-11-08 21:38     ` [PATCH] environment: move assume_unchanged into repo_config_values Hussain Sitamouwala
  1 sibling, 1 reply; 12+ messages in thread
From: Hussain Sitamouwala @ 2026-11-08 19:22 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, christian.couder

Hi Junio,

Thanks for the review.

> I think the entire sentence after "If a fetch from a remote fails"
> should be removed.

You are right. The fallback is already implied by "tried one after
the other", so the sentence adds nothing. I will remove it from the
documentation and also drop the matching clause from the commit
message.

Thanks,
Hussain

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v3] doc: describe promisor remote fetch order
  2026-11-08  7:19 [PATCH v2] doc: describe promisor remote fetch order Hussain Sitamouwala
  2026-11-08 18:45 ` Junio C Hamano
  2026-11-08 19:03 ` Junio C Hamano
@ 2026-11-08 19:55 ` Hussain Sitamouwala
  2026-11-09  5:09   ` Junio C Hamano
  2 siblings, 1 reply; 12+ messages in thread
From: Hussain Sitamouwala @ 2026-11-08 19:55 UTC (permalink / raw)
  To: git; +Cc: gitster, christian.couder, Hussain Sitamouwala

Explain that promisor remotes accepted from a server are tried
before other promisor remotes. The ordering was introduced by
8808e61fd3 (promisor-remote: try accepted remotes before others in
get_direct(), 2026-04-07).

Signed-off-by: Hussain Sitamouwala <husainsitamuwala21@gmail.com>
---
Changes since v2:
- removed the redundant fetch-fallback sentence, as suggested by Junio
- dropped the matching clause from the commit message
 Documentation/technical/partial-clone.adoc | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/Documentation/technical/partial-clone.adoc b/Documentation/technical/partial-clone.adoc
index 18718a3840..52fce44384 100644
--- a/Documentation/technical/partial-clone.adoc
+++ b/Documentation/technical/partial-clone.adoc
@@ -195,7 +195,9 @@ cache servers for fetching missing blobs while continuing to do
 filtered `git-fetch` commands from the central server.
 
 When fetching objects, promisor remotes are tried one after the other
-until all the objects have been fetched.
+until all the objects have been fetched. Promisor remotes that the
+client accepted from a server's advertisement are tried before the
+other promisor remotes.
 
 Remotes that are considered "promisor" remotes are those specified by
 the following configuration variables:
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH] environment: move assume_unchanged into repo_config_values
  2026-11-08 19:16   ` Junio C Hamano
  2026-11-08 19:22     ` Hussain Sitamouwala
@ 2026-11-08 21:38     ` Hussain Sitamouwala
  1 sibling, 0 replies; 12+ messages in thread
From: Hussain Sitamouwala @ 2026-11-08 21:38 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, christian.couder, Hussain Sitamouwala

Move assume_unchanged from global state into repository-specific
configuration to avoid sharing the setting across repositories.

Add a repository getter and update the callers to retrieve the
value using the appropriate repository instance.

Signed-off-by: Hussain Sitamouwala <husainsitamuwala21@gmail.com>
---
 builtin/update-index.c |  2 +-
 environment.c          | 11 +++++++++--
 environment.h          |  3 ++-
 read-cache.c           |  6 +++---
 4 files changed, 15 insertions(+), 7 deletions(-)

diff --git a/builtin/update-index.c b/builtin/update-index.c
index 018b78d6bb..65e9858457 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -431,7 +431,7 @@ static int add_cacheinfo(unsigned int mode, const struct object_id *oid,
 	ce->ce_flags = create_ce_flags(stage);
 	ce->ce_namelen = len;
 	ce->ce_mode = create_ce_mode(mode);
-	if (assume_unchanged)
+	if (repo_assume_unchanged(the_repository))
 		ce->ce_flags |= CE_VALID;
 	option = allow_add ? ADD_CACHE_OK_TO_ADD : 0;
 	option |= allow_replace ? ADD_CACHE_OK_TO_REPLACE : 0;
diff --git a/environment.c b/environment.c
index c83cf44839..17398c36d0 100644
--- a/environment.c
+++ b/environment.c
@@ -42,7 +42,6 @@ static int pack_compression_seen;
 static int zlib_compression_seen;
 
 int minimum_abbrev = 4, default_abbrev = -1;
-int assume_unchanged;
 char *git_commit_encoding;
 char *git_log_output_encoding;
 int fsync_object_files = -1;
@@ -152,6 +151,13 @@ int repo_has_symlinks(struct repository *repo)
 		: platform_has_symlinks();
 }
 
+int repo_assume_unchanged(struct repository *repo)
+{
+	return repo->initialized
+		? repo_config_values(repo)->assume_unchanged
+		: 0;
+}
+
 const char *repo_excludes_file(struct repository *repo)
 {
 	struct repo_config_values *cfg = repo_config_values(repo);
@@ -371,7 +377,7 @@ int git_default_core_config(const char *var, const char *value,
 	}
 
 	if (!strcmp(var, "core.ignorestat")) {
-		assume_unchanged = git_config_bool(var, value);
+		cfg->assume_unchanged = git_config_bool(var, value);
 		return 0;
 	}
 
@@ -775,6 +781,7 @@ void repo_config_values_init(struct repo_config_values *cfg)
 	cfg->protect_ntfs = PROTECT_NTFS_DEFAULT;
 	cfg->ignore_case = 0;
 	cfg->trust_executable_bit = 1;
+	cfg->assume_unchanged = 0;
 	cfg->has_symlinks = platform_has_symlinks();
 	cfg->use_nanosec = 0;
 
diff --git a/environment.h b/environment.h
index 49c86e8879..bff7a7f911 100644
--- a/environment.h
+++ b/environment.h
@@ -147,6 +147,7 @@ struct repo_config_values {
 	int protect_ntfs;
 	int ignore_case;
 	int trust_executable_bit;
+	int assume_unchanged;
 	int has_symlinks;
 	int use_nanosec;
 
@@ -200,6 +201,7 @@ int repo_protect_ntfs(struct repository *repo);
 int repo_ignore_case(struct repository *repo);
 
 int repo_trust_executable_bit(struct repository *repo);
+int repo_assume_unchanged(struct repository *repo);
 
 int repo_has_symlinks(struct repository *repo);
 
@@ -242,7 +244,6 @@ int have_git_dir(void);
 
 /* Environment bits from configuration mechanism */
 extern int minimum_abbrev, default_abbrev;
-extern int assume_unchanged;
 extern unsigned long pack_size_limit_cfg;
 
 extern int grafts_keep_true_parents;
diff --git a/read-cache.c b/read-cache.c
index 1d2b97c429..990bcc32c4 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -194,7 +194,7 @@ void fill_stat_cache_info(struct index_state *istate, struct cache_entry *ce, st
 {
 	fill_stat_data(&ce->ce_stat_data, st);
 
-	if (assume_unchanged)
+	if (repo_assume_unchanged(istate->repo))
 		ce->ce_flags |= CE_VALID;
 
 	if (S_ISREG(st->st_mode)) {
@@ -1415,7 +1415,7 @@ static struct cache_entry *refresh_cache_ent(struct index_state *istate,
 		 * is not marked VALID, this is the place to mark it
 		 * valid again, under "assume unchanged" mode.
 		 */
-		if (ignore_valid && assume_unchanged &&
+		if (ignore_valid && repo_assume_unchanged(istate->repo) &&
 		    !(ce->ce_flags & CE_VALID))
 			; /* mark this one VALID again */
 		else {
@@ -1450,7 +1450,7 @@ static struct cache_entry *refresh_cache_ent(struct index_state *istate,
 	 * (i.e. things to be edited) will reacquire CE_VALID bit
 	 * automatically, which is not really what we want.
 	 */
-	if (!ignore_valid && assume_unchanged &&
+	if (!ignore_valid && repo_assume_unchanged(istate->repo) &&
 	    !(ce->ce_flags & CE_VALID))
 		updated->ce_flags &= ~CE_VALID;
 
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH v3] doc: describe promisor remote fetch order
  2026-11-08 19:55 ` [PATCH v3] doc: describe promisor remote fetch order Hussain Sitamouwala
@ 2026-11-09  5:09   ` Junio C Hamano
  0 siblings, 0 replies; 12+ messages in thread
From: Junio C Hamano @ 2026-11-09  5:09 UTC (permalink / raw)
  To: Hussain Sitamouwala; +Cc: git, christian.couder

Hussain Sitamouwala <husainsitamuwala21@gmail.com> writes:

> Explain that promisor remotes accepted from a server are tried
> before other promisor remotes. The ordering was introduced by
> 8808e61fd3 (promisor-remote: try accepted remotes before others in
> get_direct(), 2026-04-07).

This iteration looks good.  Will queue.

> -until all the objects have been fetched.
> +until all the objects have been fetched. Promisor remotes that the
> +client accepted from a server's advertisement are tried before the
> +other promisor remotes.

Christian, perhaps time for an Ack?

Thanks.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2] doc: describe promisor remote fetch order
  2026-11-08 19:22     ` Hussain Sitamouwala
@ 2026-11-09 15:45       ` Junio C Hamano
  2026-11-10 12:58         ` Hussain Sitamouwala
  0 siblings, 1 reply; 12+ messages in thread
From: Junio C Hamano @ 2026-11-09 15:45 UTC (permalink / raw)
  To: Hussain Sitamouwala; +Cc: git, christian.couder

Hussain Sitamouwala <husainsitamuwala21@gmail.com> writes:

> Hi Junio,
>
> Thanks for the review.
>
>> I think the entire sentence after "If a fetch from a remote fails"
>> should be removed.
>
> You are right. The fallback is already implied by "tried one after
> the other", so the sentence adds nothing. I will remove it from the
> documentation and also drop the matching clause from the commit
> message.

Was that extra "not wrong per-se but totally unnecessary without
adding any new information" sentence added by LLM by any chance?
Just being curious.

Thanks.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2] doc: describe promisor remote fetch order
  2026-11-09 15:45       ` Junio C Hamano
@ 2026-11-10 12:58         ` Hussain Sitamouwala
  0 siblings, 0 replies; 12+ messages in thread
From: Hussain Sitamouwala @ 2026-11-10 12:58 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, christian.couder

On Mon, Nov 9, 2026 at 9:15 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> Was that extra "not wrong per-se but totally unnecessary without
> adding any new information" sentence added by LLM by any chance?
> Just being curious.

No, I wrote that sentence myself. I was trying to spell out the
fallback behavior explicitly, but you are right that "tried one
after the other" already covers it.

I do use an AI tool to clear up doubts while I am learning the
codebase, but I did not use it to generate the patch text.

Thanks for queueing v3.

Thanks,
Hussain

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2026-11-10 12:58 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-11-08  7:19 [PATCH v2] doc: describe promisor remote fetch order Hussain Sitamouwala
2026-11-08 18:45 ` Junio C Hamano
2026-11-08 19:07   ` Hussain Sitamouwala
2026-11-08 19:03 ` Junio C Hamano
2026-11-08 19:12   ` Hussain Sitamouwala
2026-11-08 19:16   ` Junio C Hamano
2026-11-08 19:22     ` Hussain Sitamouwala
2026-11-09 15:45       ` Junio C Hamano
2026-11-10 12:58         ` Hussain Sitamouwala
2026-11-08 21:38     ` [PATCH] environment: move assume_unchanged into repo_config_values Hussain Sitamouwala
2026-11-08 19:55 ` [PATCH v3] doc: describe promisor remote fetch order Hussain Sitamouwala
2026-11-09  5:09   ` Junio C Hamano

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox