[GSoC PATCH] promisor-remote: fix promisor.quiet to use the correct repository

[GSoC PATCH] promisor-remote: fix promisor.quiet to use the correct repository

[Trieu Huynh]

fetch_objects() reads the promisor.quiet configuration from
the_repository instead of the repo parameter it receives.

This means that when git lazy-fetches objects for a non-main
repository, eg. a submodule that is itself a partial clone opened
via repo_submodule_init(). The submodule's own promisor.quiet
setting is ignored and the superproject's setting is used instead.

Fix by replacing the_repository with repo in the repo_config_get_bool()
call. The practical trigger is git grep --recurse-submodules on a
superproject where the submodule is a partial clone.

Add a test where promisor.quiet is set only in a partial-clone
submodule; a lazy fetch triggered by "git grep --recurse-submodules"
must honor that setting.

Signed-off-by: Trieu Huynh <vikingtc4@gmail.com>
---
nit: This cleanup was explicitly noted as needed in 5d215a7b3e
("promisor-remote: make promisor_remote_reinit() not depend on
the_repository", 2024-10-17), where Patrick Steinhardt wrote:
"Those sites should eventually be cleaned up in a later patch series."

 promisor-remote.c        |  2 +-
 t/t0410-partial-clone.sh | 45 +++++++++++++++++++++++++++++++++++++++-
 2 files changed, 45 insertions(+), 2 deletions(-)

diff --git a/promisor-remote.c b/promisor-remote.c
index 96fa215b06..225260b05f 100644
--- a/promisor-remote.c
+++ b/promisor-remote.c
@@ -46,7 +46,7 @@ static int fetch_objects(struct repository *repo,
 		     "fetch", remote_name, "--no-tags",
 		     "--no-write-fetch-head", "--recurse-submodules=no",
 		     "--filter=blob:none", "--stdin", NULL);
-	if (!repo_config_get_bool(the_repository, "promisor.quiet", &quiet) && quiet)
+	if (!repo_config_get_bool(repo, "promisor.quiet", &quiet) && quiet)
 		strvec_push(&child.args, "--quiet");
 	if (start_command(&child))
 		die(_("promisor-remote: unable to fork off fetch subprocess"));
diff --git a/t/t0410-partial-clone.sh b/t/t0410-partial-clone.sh
index 52e19728a3..dff442da20 100755
--- a/t/t0410-partial-clone.sh
+++ b/t/t0410-partial-clone.sh
@@ -717,7 +717,29 @@ test_expect_success 'setup for promisor.quiet tests' '
 	git -C server rm foo.t &&
 	git -C server commit -m remove &&
 	git -C server config uploadpack.allowanysha1inwant 1 &&
-	git -C server config uploadpack.allowfilter 1
+	git -C server config uploadpack.allowfilter 1 &&
+
+	# Setup for submodule repo test: superproject whose submodule is a
+	# partial clone, so that promisor.quiet is read via a non-main repo.
+	rm -rf sub-pc-src sub-pc-srv.bare super-src super-work &&
+	git init sub-pc-src &&
+	test_commit -C sub-pc-src initial file.txt "hello" &&
+
+	git clone --bare sub-pc-src sub-pc-srv.bare &&
+	git -C sub-pc-srv.bare config uploadpack.allowfilter 1 &&
+	git -C sub-pc-srv.bare config uploadpack.allowanysha1inwant 1 &&
+
+	git init super-src &&
+	git -C super-src -c protocol.file.allow=always \
+		submodule add "file://$(pwd)/sub-pc-srv.bare" sub &&
+	git -C super-src commit -m "add submodule" &&
+
+	git -c protocol.file.allow=always clone super-src super-work &&
+	git -C super-work -c protocol.file.allow=always \
+		submodule update --init --filter=blob:none sub &&
+
+	# Allow file:// in the submodule so that lazy-fetch subprocesses work.
+	git -C super-work/sub config protocol.file.allow always
 '
 
 test_expect_success TTY 'promisor.quiet=false shows progress messages' '
@@ -752,6 +774,27 @@ test_expect_success TTY 'promisor.quiet=unconfigured shows progress messages' '
 	grep "Receiving objects" err
 '
 
+test_expect_success 'promisor.quiet from submodule repo is honored' '
+	rm -f pc-quiet-trace &&
+
+	# Set promisor.quiet only in the submodule, not the superproject.
+	git -C super-work/sub config promisor.quiet true &&
+
+	# Push a new commit+blob to the server; the blob stays missing in the
+	# partial-clone submodule until a lazy fetch is triggered.
+	test_commit -C sub-pc-src updated new-file.txt "world" &&
+	git -C sub-pc-src push "$(pwd)/sub-pc-srv.bare" HEAD:master &&
+	git -C super-work/sub -c protocol.file.allow=always fetch origin &&
+	git -C super-work/sub reset --mixed origin/master &&
+
+	# grep descends into the submodule and triggers a lazy fetch for the
+	# missing blob; verify the fetch subprocess carries --quiet.
+	GIT_TRACE2_EVENT="$(pwd)/pc-quiet-trace" \
+		git -C super-work grep --cached --recurse-submodules "world" \
+		2>/dev/null &&
+	grep negotiationAlgorithm pc-quiet-trace | grep -e --quiet
+'
+
 . "$TEST_DIRECTORY"/lib-httpd.sh
 start_httpd
 
-- 
2.43.0

Re: [GSoC PATCH] promisor-remote: fix promisor.quiet to use the correct repository

[Tian Yuchen]

On 4/7/26 02:30, Trieu Huynh wrote:

I strongly recommend you to run a patch locally before sending it.

	not ok 38 - promisor.quiet from submodule repo is honored

> +test_expect_success 'promisor.quiet from submodule repo is honored' '
> +	rm -f pc-quiet-trace &&
> +
> +	# Set promisor.quiet only in the submodule, not the superproject.
> +	git -C super-work/sub config promisor.quiet true &&
> +
> +	# Push a new commit+blob to the server; the blob stays missing in the
> +	# partial-clone submodule until a lazy fetch is triggered.
> +	test_commit -C sub-pc-src updated new-file.txt "world" &&
> +	git -C sub-pc-src push "$(pwd)/sub-pc-srv.bare" HEAD:master &&
> +	git -C super-work/sub -c protocol.file.allow=always fetch origin &&
> +	git -C super-work/sub reset --mixed origin/master &&
> +
> +	# grep descends into the submodule and triggers a lazy fetch for the
> +	# missing blob; verify the fetch subprocess carries --quiet.
> +	GIT_TRACE2_EVENT="$(pwd)/pc-quiet-trace" \
> +		git -C super-work grep --cached --recurse-submodules "world" \
> +		2>/dev/null &&
> +	grep negotiationAlgorithm pc-quiet-trace | grep -e --quiet
> +'
> +
>   . "$TEST_DIRECTORY"/lib-httpd.sh
>   start_httpd
>   

Regards, Yuchen

Re: [GSoC PATCH] promisor-remote: fix promisor.quiet to use the correct repository

[Tian Yuchen]

On 4/7/26 02:30, Trieu Huynh wrote:

I strongly recommend you to run a patch locally before sending it.

	 not ok 38 - promisor.quiet from submodule repo is honored

> +test_expect_success 'promisor.quiet from submodule repo is honored' '
> +	rm -f pc-quiet-trace &&
> +
> +	# Set promisor.quiet only in the submodule, not the superproject.
> +	git -C super-work/sub config promisor.quiet true &&
> +
> +	# Push a new commit+blob to the server; the blob stays missing in the
> +	# partial-clone submodule until a lazy fetch is triggered.
> +	test_commit -C sub-pc-src updated new-file.txt "world" &&
> +	git -C sub-pc-src push "$(pwd)/sub-pc-srv.bare" HEAD:master &&
> +	git -C super-work/sub -c protocol.file.allow=always fetch origin &&
> +	git -C super-work/sub reset --mixed origin/master &&
> +
> +	# grep descends into the submodule and triggers a lazy fetch for the
> +	# missing blob; verify the fetch subprocess carries --quiet.
> +	GIT_TRACE2_EVENT="$(pwd)/pc-quiet-trace" \
> +		git -C super-work grep --cached --recurse-submodules "world" \
> +		2>/dev/null &&
> +	grep negotiationAlgorithm pc-quiet-trace | grep -e --quiet
> +'
> +
>   . "$TEST_DIRECTORY"/lib-httpd.sh
>   start_httpd
>   

Regards, Yuchen

Re: [GSoC PATCH] promisor-remote: fix promisor.quiet to use the correct repository

[Junio C Hamano]

Tian Yuchen <cat@malon.dev> writes:

> On 4/7/26 02:30, Trieu Huynh wrote:
>
> I strongly recommend you to run a patch locally before sending it.
>
> 	not ok 38 - promisor.quiet from submodule repo is honored

FWIW, I didn't see it fail in my local environment (both in 'seen'
and also standalone) or at GitHub CI (in 'seen'), so it is a bit
hasty to conclude that the patch was sent without proper testing.

I wonder what is different in _your_ environment (note, I am not
saying your environment is _wrong_.  It is just different, perhaps
the compiler I use and your build environment may align things
differently, or perhaps on-stack "uninitialied" pieces of memory
happen to have different values that the code is reading that causes
different behaviours---in which case it is the code that is wrong.
Asking for environment differences is the first step to figure out
what incorrect environment dependencies the code has).

>> +test_expect_success 'promisor.quiet from submodule repo is honored' '
>> +	rm -f pc-quiet-trace &&
>> +
>> +	# Set promisor.quiet only in the submodule, not the superproject.
>> +	git -C super-work/sub config promisor.quiet true &&
>> +
>> +	# Push a new commit+blob to the server; the blob stays missing in the
>> +	# partial-clone submodule until a lazy fetch is triggered.
>> +	test_commit -C sub-pc-src updated new-file.txt "world" &&
>> +	git -C sub-pc-src push "$(pwd)/sub-pc-srv.bare" HEAD:master &&
>> +	git -C super-work/sub -c protocol.file.allow=always fetch origin &&
>> +	git -C super-work/sub reset --mixed origin/master &&
>> +
>> +	# grep descends into the submodule and triggers a lazy fetch for the
>> +	# missing blob; verify the fetch subprocess carries --quiet.
>> +	GIT_TRACE2_EVENT="$(pwd)/pc-quiet-trace" \
>> +		git -C super-work grep --cached --recurse-submodules "world" \
>> +		2>/dev/null &&
>> +	grep negotiationAlgorithm pc-quiet-trace | grep -e --quiet
>> +'
>> +
>>   . "$TEST_DIRECTORY"/lib-httpd.sh
>>   start_httpd
>>   
>
> Regards, Yuchen

Re: [GSoC PATCH] promisor-remote: fix promisor.quiet to use the correct repository

[Tian Yuchen]

On 4/9/26 02:23, Junio C Hamano wrote:

> FWIW, I didn't see it fail in my local environment (both in 'seen'
> and also standalone) or at GitHub CI (in 'seen'), so it is a bit
> hasty to conclude that the patch was sent without proper testing.

You’re right, I was a bit hasty.

> I wonder what is different in _your_ environment (note, I am not
> saying your environment is _wrong_.  It is just different, perhaps
> the compiler I use and your build environment may align things
> differently, or perhaps on-stack "uninitialied" pieces of memory
> happen to have different values that the code is reading that causes
> different behaviours---in which case it is the code that is wrong.
> Asking for environment differences is the first step to figure out
> what incorrect environment dependencies the code has).   
>

---

git version 2.43.0
cpu: x86_64
no commit associated with this build
sizeof-long: 8
sizeof-size_t: 8

Using built-in specs.
COLLECT_GCC=gcc
COLLECT_LTO_WRAPPER=/usr/libexec/gcc/x86_64-linux-gnu/13/lto-wrapper
OFFLOAD_TARGET_NAMES=nvptx-none:amdgcn-amdhsa
OFFLOAD_TARGET_DEFAULT=1
Target: x86_64-linux-gnu
Configured with: ../src/configure -v --with-pkgversion='Ubuntu 
13.3.0-6ubuntu2~24.04.1' 
--with-bugurl=file:///usr/share/doc/gcc-13/README.Bugs 
--enable-languages=c,ada,c++,go,d,fortran,objc,obj-c++,m2 --prefix=/usr 
--with-gcc-major-version-only --program-suffix=-13 
--program-prefix=x86_64-linux-gnu- --enable-shared 
--enable-linker-build-id --libexecdir=/usr/libexec 
--without-included-gettext --enable-threads=posix --libdir=/usr/lib 
--enable-nls --enable-bootstrap --enable-clocale=gnu 
--enable-libstdcxx-debug --enable-libstdcxx-time=yes 
--with-default-libstdcxx-abi=new --enable-libstdcxx-backtrace 
--enable-gnu-unique-object --disable-vtable-verify --enable-plugin 
--enable-default-pie --with-system-zlib 
--enable-libphobos-checking=release --with-target-system-zlib=auto 
--enable-objc-gc=auto --enable-multiarch --disable-werror --enable-cet 
--with-arch-32=i686 --with-abi=m64 --with-multilib-list=m32,m64,mx32 
--enable-multilib --with-tune=generic 
--enable-offload-targets=nvptx-none=/build/gcc-13-EldibY/gcc-13-13.3.0/debian/tmp-nvptx/usr,amdgcn-amdhsa=/build/gcc-13-EldibY/gcc-13-13.3.0/debian/tmp-gcn/usr 
--enable-offload-defaulted --without-cuda-driver 
--enable-checking=release --build=x86_64-linux-gnu 
--host=x86_64-linux-gnu --target=x86_64-linux-gnu 
--with-build-config=bootstrap-lto-lean --enable-link-serialization=2
Thread model: posix
Supported LTO compression algorithms: zlib zstd
gcc version 13.3.0 (Ubuntu 13.3.0-6ubuntu2~24.04.1)

Linux malon-Yoga-14sARE-2020 6.14.0-37-generic #37~24.04.1-Ubuntu SMP 
PREEMPT_DYNAMIC Thu Nov 20 10:25:38 UTC 2 x86_64 x86_64 x86_64 GNU/Linux

---

By the way, I find this bit of code rather confusing to me:

> +	GIT_TRACE2_EVENT="$(pwd)/pc-quiet-trace" \
> +		git -C super-work grep --cached --recurse-submodules "world" \
> +		2>/dev/null &&
> +	grep negotiationAlgorithm pc-quiet-trace | grep -e --quiet

Is this grep pattern correct?

Thanks, Yuchen

Re: [GSoC PATCH] promisor-remote: fix promisor.quiet to use the correct repository

[Trieu Huynh]

On Thu, Apr 09, 2026 at 12:34:47PM +0800, Tian Yuchen wrote:
> On 4/9/26 02:23, Junio C Hamano wrote:
> 
> > FWIW, I didn't see it fail in my local environment (both in 'seen'
> > and also standalone) or at GitHub CI (in 'seen'), so it is a bit
> > hasty to conclude that the patch was sent without proper testing.
> 
> You’re right, I was a bit hasty.
> 
> > I wonder what is different in _your_ environment (note, I am not
> > saying your environment is _wrong_.  It is just different, perhaps
> > the compiler I use and your build environment may align things
> > differently, or perhaps on-stack "uninitialied" pieces of memory
> > happen to have different values that the code is reading that causes
> > different behaviours---in which case it is the code that is wrong.
> > Asking for environment differences is the first step to figure out
> > what incorrect environment dependencies the code has).
> > 
> 
> ---
> 
> git version 2.43.0
> cpu: x86_64
> no commit associated with this build
> sizeof-long: 8
> sizeof-size_t: 8
> 
> Using built-in specs.
> COLLECT_GCC=gcc
> COLLECT_LTO_WRAPPER=/usr/libexec/gcc/x86_64-linux-gnu/13/lto-wrapper
> OFFLOAD_TARGET_NAMES=nvptx-none:amdgcn-amdhsa
> OFFLOAD_TARGET_DEFAULT=1
> Target: x86_64-linux-gnu
> Configured with: ../src/configure -v --with-pkgversion='Ubuntu
> 13.3.0-6ubuntu2~24.04.1'
> --with-bugurl=file:///usr/share/doc/gcc-13/README.Bugs
> --enable-languages=c,ada,c++,go,d,fortran,objc,obj-c++,m2 --prefix=/usr
> --with-gcc-major-version-only --program-suffix=-13
> --program-prefix=x86_64-linux-gnu- --enable-shared --enable-linker-build-id
> --libexecdir=/usr/libexec --without-included-gettext --enable-threads=posix
> --libdir=/usr/lib --enable-nls --enable-bootstrap --enable-clocale=gnu
> --enable-libstdcxx-debug --enable-libstdcxx-time=yes
> --with-default-libstdcxx-abi=new --enable-libstdcxx-backtrace
> --enable-gnu-unique-object --disable-vtable-verify --enable-plugin
> --enable-default-pie --with-system-zlib --enable-libphobos-checking=release
> --with-target-system-zlib=auto --enable-objc-gc=auto --enable-multiarch
> --disable-werror --enable-cet --with-arch-32=i686 --with-abi=m64
> --with-multilib-list=m32,m64,mx32 --enable-multilib --with-tune=generic --enable-offload-targets=nvptx-none=/build/gcc-13-EldibY/gcc-13-13.3.0/debian/tmp-nvptx/usr,amdgcn-amdhsa=/build/gcc-13-EldibY/gcc-13-13.3.0/debian/tmp-gcn/usr
> --enable-offload-defaulted --without-cuda-driver --enable-checking=release
> --build=x86_64-linux-gnu --host=x86_64-linux-gnu --target=x86_64-linux-gnu
> --with-build-config=bootstrap-lto-lean --enable-link-serialization=2
> Thread model: posix
> Supported LTO compression algorithms: zlib zstd
> gcc version 13.3.0 (Ubuntu 13.3.0-6ubuntu2~24.04.1)
> 
> Linux malon-Yoga-14sARE-2020 6.14.0-37-generic #37~24.04.1-Ubuntu SMP
> PREEMPT_DYNAMIC Thu Nov 20 10:25:38 UTC 2 x86_64 x86_64 x86_64 GNU/Linux
> 
> ---
> 
> By the way, I find this bit of code rather confusing to me:
> 
> > +	GIT_TRACE2_EVENT="$(pwd)/pc-quiet-trace" \
> > +		git -C super-work grep --cached --recurse-submodules "world" \
> > +		2>/dev/null &&
> > +	grep negotiationAlgorithm pc-quiet-trace | grep -e --quiet
> 
> Is this grep pattern correct?
> 
AFAICT, this intent is to check that "--quiet" appears as an argument
in the trace2 event that contains "negotiationAlgorithm". That said,
the "-e" flag tells grep to treat the next argument as the search
pattern, so "grep -e --quiet" searches for the literal string "--quiet"
rather than grep misinterpreting as one of its own options.

BRs,
Trieu Huynh
> Thanks, Yuchen

Re: [GSoC PATCH] promisor-remote: fix promisor.quiet to use the correct repository

[Junio C Hamano]

Trieu Huynh <vikingtc4@gmail.com> writes:

> On Thu, Apr 09, 2026 at 12:34:47PM +0800, Tian Yuchen wrote:
>> On 4/9/26 02:23, Junio C Hamano wrote:
>> 
>> > FWIW, I didn't see it fail in my local environment (both in 'seen'
>> > and also standalone) or at GitHub CI (in 'seen'), so it is a bit
>> > hasty to conclude that the patch was sent without proper testing.
>> 
>> You’re right, I was a bit hasty.
>> 
>> > I wonder what is different in _your_ environment (note, I am not
>> > saying your environment is _wrong_.  It is just different, perhaps
>> > the compiler I use and your build environment may align things
>> > differently, or perhaps on-stack "uninitialied" pieces of memory
>> > happen to have different values that the code is reading that causes
>> > different behaviours---in which case it is the code that is wrong.
>> > Asking for environment differences is the first step to figure out
>> > what incorrect environment dependencies the code has).
>> > 
>> 
>> ---
>> 
>> git version 2.43.0
>> cpu: x86_64
>> no commit associated with this build
>> sizeof-long: 8
>> sizeof-size_t: 8
>> 
>> Using built-in specs.
>> COLLECT_GCC=gcc
>> COLLECT_LTO_WRAPPER=/usr/libexec/gcc/x86_64-linux-gnu/13/lto-wrapper
>> OFFLOAD_TARGET_NAMES=nvptx-none:amdgcn-amdhsa
>> OFFLOAD_TARGET_DEFAULT=1
>> Target: x86_64-linux-gnu
>> Configured with: ../src/configure -v --with-pkgversion='Ubuntu
>> 13.3.0-6ubuntu2~24.04.1'
>> --with-bugurl=file:///usr/share/doc/gcc-13/README.Bugs
>> --enable-languages=c,ada,c++,go,d,fortran,objc,obj-c++,m2 --prefix=/usr
>> --with-gcc-major-version-only --program-suffix=-13
>> --program-prefix=x86_64-linux-gnu- --enable-shared --enable-linker-build-id
>> --libexecdir=/usr/libexec --without-included-gettext --enable-threads=posix
>> --libdir=/usr/lib --enable-nls --enable-bootstrap --enable-clocale=gnu
>> --enable-libstdcxx-debug --enable-libstdcxx-time=yes
>> --with-default-libstdcxx-abi=new --enable-libstdcxx-backtrace
>> --enable-gnu-unique-object --disable-vtable-verify --enable-plugin
>> --enable-default-pie --with-system-zlib --enable-libphobos-checking=release
>> --with-target-system-zlib=auto --enable-objc-gc=auto --enable-multiarch
>> --disable-werror --enable-cet --with-arch-32=i686 --with-abi=m64
>> --with-multilib-list=m32,m64,mx32 --enable-multilib --with-tune=generic --enable-offload-targets=nvptx-none=/build/gcc-13-EldibY/gcc-13-13.3.0/debian/tmp-nvptx/usr,amdgcn-amdhsa=/build/gcc-13-EldibY/gcc-13-13.3.0/debian/tmp-gcn/usr
>> --enable-offload-defaulted --without-cuda-driver --enable-checking=release
>> --build=x86_64-linux-gnu --host=x86_64-linux-gnu --target=x86_64-linux-gnu
>> --with-build-config=bootstrap-lto-lean --enable-link-serialization=2
>> Thread model: posix
>> Supported LTO compression algorithms: zlib zstd
>> gcc version 13.3.0 (Ubuntu 13.3.0-6ubuntu2~24.04.1)
>> 
>> Linux malon-Yoga-14sARE-2020 6.14.0-37-generic #37~24.04.1-Ubuntu SMP
>> PREEMPT_DYNAMIC Thu Nov 20 10:25:38 UTC 2 x86_64 x86_64 x86_64 GNU/Linux
>> 
>> ---
>> 
>> By the way, I find this bit of code rather confusing to me:
>> 
>> > +	GIT_TRACE2_EVENT="$(pwd)/pc-quiet-trace" \
>> > +		git -C super-work grep --cached --recurse-submodules "world" \
>> > +		2>/dev/null &&
>> > +	grep negotiationAlgorithm pc-quiet-trace | grep -e --quiet
>> 
>> Is this grep pattern correct?
>> 
> AFAICT, this intent is to check that "--quiet" appears as an argument
> in the trace2 event that contains "negotiationAlgorithm". That said,
> the "-e" flag tells grep to treat the next argument as the search
> pattern, so "grep -e --quiet" searches for the literal string "--quiet"
> rather than grep misinterpreting as one of its own options.
>
> BRs,
> Trieu Huynh
>> Thanks, Yuchen

The discussion thread stalled at this point.  Are we happy with the
proposed changes?

Thanks.

Re: [GSoC PATCH] promisor-remote: fix promisor.quiet to use the correct repository

[Tian Yuchen]

On 4/16/26 01:39, Junio C Hamano wrote:
> 
> The discussion thread stalled at this point.  Are we happy with the
> proposed changes?
> 
> Thanks.

After running it a week later, the error had (surprisingly) disappeared, 
and I couldn’t reproduce it no matter what I tried.

Since the code works well on everyone’s computer and on the CI tests, I 
suppose I must have got it wrong earlier.

Thanks, Yuchen