* [PATCH] submodule-config: fix reading submodule.fetchJobs
@ 2026-05-03 10:14 Saagar Jha via GitGitGadget
2026-05-03 11:07 ` Pablo
2026-05-03 11:57 ` [PATCH v2] " Saagar Jha via GitGitGadget
0 siblings, 2 replies; 9+ messages in thread
From: Saagar Jha via GitGitGadget @ 2026-05-03 10:14 UTC (permalink / raw)
To: git; +Cc: Saagar Jha, Saagar Jha
From: Saagar Jha <saagar@saagarjha.com>
Signed-off-by: Saagar Jha <saagar@saagarjha.com>
---
submodule-config: fix reading submodule.fetchJobs
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2287%2Fsaagarjha%2Fmaint-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2287/saagarjha/maint-v1
Pull-Request: https://github.com/git/git/pull/2287
submodule-config.c | 2 +-
t/t7406-submodule-update.sh | 10 ++++++++++
2 files changed, 11 insertions(+), 1 deletion(-)
diff --git a/submodule-config.c b/submodule-config.c
index 1f19fe2077..57b190678e 100644
--- a/submodule-config.c
+++ b/submodule-config.c
@@ -1037,5 +1037,5 @@ static int gitmodules_update_clone_config(const char *var, const char *value,
void update_clone_config_from_gitmodules(int *max_jobs)
{
- config_from_gitmodules(gitmodules_update_clone_config, the_repository, &max_jobs);
+ config_from_gitmodules(gitmodules_update_clone_config, the_repository, max_jobs);
}
diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh
index 3adab12091..234a021fb3 100755
--- a/t/t7406-submodule-update.sh
+++ b/t/t7406-submodule-update.sh
@@ -1055,6 +1055,16 @@ test_expect_success 'submodule update can be run in parallel' '
)
'
+test_expect_success 'submodule update honors fetch jobs config from .gitmodules' '
+ test_when_finished "rm -rf super3" &&
+ git clone cloned super3 &&
+ (cd super3 &&
+ git config -f .gitmodules submodule.fetchJobs 67 &&
+ GIT_TRACE="$(pwd)/trace.out" git submodule update --init &&
+ grep "67 tasks" trace.out
+ )
+'
+
test_expect_success 'git clone passes the parallel jobs config on to submodules' '
test_when_finished "rm -rf super4" &&
GIT_TRACE=$(pwd)/trace.out git clone --recurse-submodules --jobs 7 . super4 &&
base-commit: 67ad42147a7acc2af6074753ebd03d904476118f
--
gitgitgadget
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] submodule-config: fix reading submodule.fetchJobs
2026-05-03 10:14 [PATCH] submodule-config: fix reading submodule.fetchJobs Saagar Jha via GitGitGadget
@ 2026-05-03 11:07 ` Pablo
2026-05-03 11:24 ` Saagar Jha
2026-05-03 11:57 ` [PATCH v2] " Saagar Jha via GitGitGadget
1 sibling, 1 reply; 9+ messages in thread
From: Pablo @ 2026-05-03 11:07 UTC (permalink / raw)
To: Saagar Jha via GitGitGadget; +Cc: git, Saagar Jha
El dom, 3 may 2026 a las 12:14, Saagar Jha via GitGitGadget
(<gitgitgadget@gmail.com>) escribió:
>
> From: Saagar Jha <saagar@saagarjha.com>
>
> Signed-off-by: Saagar Jha <saagar@saagarjha.com>
> ---
> submodule-config: fix reading submodule.fetchJobs
>
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2287%2Fsaagarjha%2Fmaint-v1
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2287/saagarjha/maint-v1
> Pull-Request: https://github.com/git/git/pull/2287
>
> submodule-config.c | 2 +-
> t/t7406-submodule-update.sh | 10 ++++++++++
> 2 files changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/submodule-config.c b/submodule-config.c
> index 1f19fe2077..57b190678e 100644
> --- a/submodule-config.c
> +++ b/submodule-config.c
> @@ -1037,5 +1037,5 @@ static int gitmodules_update_clone_config(const char *var, const char *value,
>
> void update_clone_config_from_gitmodules(int *max_jobs)
> {
> - config_from_gitmodules(gitmodules_update_clone_config, the_repository, &max_jobs);
> + config_from_gitmodules(gitmodules_update_clone_config, the_repository, max_jobs);
> }
> diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh
> index 3adab12091..234a021fb3 100755
> --- a/t/t7406-submodule-update.sh
> +++ b/t/t7406-submodule-update.sh
> @@ -1055,6 +1055,16 @@ test_expect_success 'submodule update can be run in parallel' '
> )
> '
>
> +test_expect_success 'submodule update honors fetch jobs config from .gitmodules' '
> + test_when_finished "rm -rf super3" &&
> + git clone cloned super3 &&
> + (cd super3 &&
> + git config -f .gitmodules submodule.fetchJobs 67 &&
> + GIT_TRACE="$(pwd)/trace.out" git submodule update --init &&
> + grep "67 tasks" trace.out
> + )
> +'
> +
> test_expect_success 'git clone passes the parallel jobs config on to submodules' '
> test_when_finished "rm -rf super4" &&
> GIT_TRACE=$(pwd)/trace.out git clone --recurse-submodules --jobs 7 . super4 &&
>
> base-commit: 67ad42147a7acc2af6074753ebd03d904476118f
> --
> gitgitgadget
>
Hi Saagar!
A few hints before this can be reviewable:
- There is no commit body which, not only is a must, it makes it very
hard reviewing because I cannot know what you wanted to do and I have
to imagine it.
- CI tests seem to be failing, but the file that reports fail doesn't
seem to be related with your changes.
About the code, I can understand the 'why': the function above
'fetch_config_from_gitmodules' builds an struct inside and sends the
address &config, 'update_clone_config_from_gitmodules' calls the same
function with &max_jobs but the error would be that max_jobs is
already a pointer so there would be no need to pass by reference
max_jobs.
But this kind of explanation is what should be on the commit body.
Maybe not this technical as the code can be easily seen, but a high
level explanation about what this is.
Looking forward for a v2,
--
Pablo
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] submodule-config: fix reading submodule.fetchJobs
2026-05-03 11:07 ` Pablo
@ 2026-05-03 11:24 ` Saagar Jha
2026-05-03 12:19 ` Pablo
0 siblings, 1 reply; 9+ messages in thread
From: Saagar Jha @ 2026-05-03 11:24 UTC (permalink / raw)
To: Pablo; +Cc: Saagar Jha via GitGitGadget, git
Hi Pablo! I figured the change was simple enough to be self-explanatory, but I expanded the description to indicate what the bug was. Let me know if there’s anything else you’d like me to fix (I agree that the CI failure is probably not related to my change, but I can try my hand at looking at it after if nobody has?)
> On May 3, 2026, at 04:07, Pablo <pabloosabaterr@gmail.com> wrote:
>
> El dom, 3 may 2026 a las 12:14, Saagar Jha via GitGitGadget
> (<gitgitgadget@gmail.com>) escribió:
>>
>> From: Saagar Jha <saagar@saagarjha.com>
>>
>> Signed-off-by: Saagar Jha <saagar@saagarjha.com>
>> ---
>> submodule-config: fix reading submodule.fetchJobs
>>
>> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2287%2Fsaagarjha%2Fmaint-v1
>> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2287/saagarjha/maint-v1
>> Pull-Request: https://github.com/git/git/pull/2287
>>
>> submodule-config.c | 2 +-
>> t/t7406-submodule-update.sh | 10 ++++++++++
>> 2 files changed, 11 insertions(+), 1 deletion(-)
>>
>> diff --git a/submodule-config.c b/submodule-config.c
>> index 1f19fe2077..57b190678e 100644
>> --- a/submodule-config.c
>> +++ b/submodule-config.c
>> @@ -1037,5 +1037,5 @@ static int gitmodules_update_clone_config(const char *var, const char *value,
>>
>> void update_clone_config_from_gitmodules(int *max_jobs)
>> {
>> - config_from_gitmodules(gitmodules_update_clone_config, the_repository, &max_jobs);
>> + config_from_gitmodules(gitmodules_update_clone_config, the_repository, max_jobs);
>> }
>> diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh
>> index 3adab12091..234a021fb3 100755
>> --- a/t/t7406-submodule-update.sh
>> +++ b/t/t7406-submodule-update.sh
>> @@ -1055,6 +1055,16 @@ test_expect_success 'submodule update can be run in parallel' '
>> )
>> '
>>
>> +test_expect_success 'submodule update honors fetch jobs config from .gitmodules' '
>> + test_when_finished "rm -rf super3" &&
>> + git clone cloned super3 &&
>> + (cd super3 &&
>> + git config -f .gitmodules submodule.fetchJobs 67 &&
>> + GIT_TRACE="$(pwd)/trace.out" git submodule update --init &&
>> + grep "67 tasks" trace.out
>> + )
>> +'
>> +
>> test_expect_success 'git clone passes the parallel jobs config on to submodules' '
>> test_when_finished "rm -rf super4" &&
>> GIT_TRACE=$(pwd)/trace.out git clone --recurse-submodules --jobs 7 . super4 &&
>>
>> base-commit: 67ad42147a7acc2af6074753ebd03d904476118f
>> --
>> gitgitgadget
>>
>
> Hi Saagar!
>
> A few hints before this can be reviewable:
> - There is no commit body which, not only is a must, it makes it very
> hard reviewing because I cannot know what you wanted to do and I have
> to imagine it.
> - CI tests seem to be failing, but the file that reports fail doesn't
> seem to be related with your changes.
>
> About the code, I can understand the 'why': the function above
> 'fetch_config_from_gitmodules' builds an struct inside and sends the
> address &config, 'update_clone_config_from_gitmodules' calls the same
> function with &max_jobs but the error would be that max_jobs is
> already a pointer so there would be no need to pass by reference
> max_jobs.
>
> But this kind of explanation is what should be on the commit body.
> Maybe not this technical as the code can be easily seen, but a high
> level explanation about what this is.
>
> Looking forward for a v2,
> --
> Pablo
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2] submodule-config: fix reading submodule.fetchJobs
2026-05-03 10:14 [PATCH] submodule-config: fix reading submodule.fetchJobs Saagar Jha via GitGitGadget
2026-05-03 11:07 ` Pablo
@ 2026-05-03 11:57 ` Saagar Jha via GitGitGadget
2026-05-03 13:07 ` Pablo
2026-05-03 13:52 ` [PATCH v3] " Saagar Jha via GitGitGadget
1 sibling, 2 replies; 9+ messages in thread
From: Saagar Jha via GitGitGadget @ 2026-05-03 11:57 UTC (permalink / raw)
To: git; +Cc: Pablo, Saagar Jha, Saagar Jha
From: Saagar Jha <saagar@saagarjha.com>
The old code accidentally passed &max_jobs rather than max_jobs into
config_from_gitmodules, which caused the setting to be written to the
wrong place and dropped.
Signed-off-by: Saagar Jha <saagar@saagarjha.com>
---
submodule-config: fix reading submodule.fetchJobs
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2287%2Fsaagarjha%2Fmaint-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2287/saagarjha/maint-v2
Pull-Request: https://github.com/git/git/pull/2287
Range-diff vs v1:
1: 094c641227 ! 1: 868901f1a6 submodule-config: fix reading submodule.fetchJobs
@@ Metadata
## Commit message ##
submodule-config: fix reading submodule.fetchJobs
+ The old code accidentally passed &max_jobs rather than max_jobs into
+ config_from_gitmodules, which caused the setting to be written to the
+ wrong place and dropped.
+
Signed-off-by: Saagar Jha <saagar@saagarjha.com>
## submodule-config.c ##
submodule-config.c | 2 +-
t/t7406-submodule-update.sh | 10 ++++++++++
2 files changed, 11 insertions(+), 1 deletion(-)
diff --git a/submodule-config.c b/submodule-config.c
index 1f19fe2077..57b190678e 100644
--- a/submodule-config.c
+++ b/submodule-config.c
@@ -1037,5 +1037,5 @@ static int gitmodules_update_clone_config(const char *var, const char *value,
void update_clone_config_from_gitmodules(int *max_jobs)
{
- config_from_gitmodules(gitmodules_update_clone_config, the_repository, &max_jobs);
+ config_from_gitmodules(gitmodules_update_clone_config, the_repository, max_jobs);
}
diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh
index 3adab12091..234a021fb3 100755
--- a/t/t7406-submodule-update.sh
+++ b/t/t7406-submodule-update.sh
@@ -1055,6 +1055,16 @@ test_expect_success 'submodule update can be run in parallel' '
)
'
+test_expect_success 'submodule update honors fetch jobs config from .gitmodules' '
+ test_when_finished "rm -rf super3" &&
+ git clone cloned super3 &&
+ (cd super3 &&
+ git config -f .gitmodules submodule.fetchJobs 67 &&
+ GIT_TRACE="$(pwd)/trace.out" git submodule update --init &&
+ grep "67 tasks" trace.out
+ )
+'
+
test_expect_success 'git clone passes the parallel jobs config on to submodules' '
test_when_finished "rm -rf super4" &&
GIT_TRACE=$(pwd)/trace.out git clone --recurse-submodules --jobs 7 . super4 &&
base-commit: 67ad42147a7acc2af6074753ebd03d904476118f
--
gitgitgadget
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] submodule-config: fix reading submodule.fetchJobs
2026-05-03 11:24 ` Saagar Jha
@ 2026-05-03 12:19 ` Pablo
0 siblings, 0 replies; 9+ messages in thread
From: Pablo @ 2026-05-03 12:19 UTC (permalink / raw)
To: Saagar Jha; +Cc: Saagar Jha via GitGitGadget, git
El dom, 3 may 2026 a las 13:24, Saagar Jha (<saagar@saagarjha.com>) escribió:
>
> Hi Pablo! I figured the change was simple enough to be self-explanatory, but I expanded the description to indicate what the bug was. Let me know if there’s anything else you’d like me to fix (I agree that the CI failure is probably not related to my change, but I can try my hand at looking at it after if nobody has?)
No, by doing a quick search you find:
https://lore.kernel.org/git/20260402041433.GA3501120@coredump.intra.peff.net/
I was noting it so other reviewers know that your patch has nothing to
do with the CI failing.
Nit: Try to have 80 columns at most per line so the lines doesn't get too long
--
Pablo
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] submodule-config: fix reading submodule.fetchJobs
2026-05-03 11:57 ` [PATCH v2] " Saagar Jha via GitGitGadget
@ 2026-05-03 13:07 ` Pablo
2026-05-10 0:01 ` Junio C Hamano
2026-05-03 13:52 ` [PATCH v3] " Saagar Jha via GitGitGadget
1 sibling, 1 reply; 9+ messages in thread
From: Pablo @ 2026-05-03 13:07 UTC (permalink / raw)
To: Saagar Jha via GitGitGadget; +Cc: git, Saagar Jha
El dom, 3 may 2026 a las 13:57, Saagar Jha via GitGitGadget
(<gitgitgadget@gmail.com>) escribió:
>
> From: Saagar Jha <saagar@saagarjha.com>
>
> The old code accidentally passed &max_jobs rather than max_jobs into
> config_from_gitmodules, which caused the setting to be written to the
> wrong place and dropped.
Better, but we can improve this, following
Documentation/SubmittingPatches [1], it is better to write in present
tense something like:
update_clone_config_from_gitmodules() passes &max_jobs to
config_from_gitmodules(), but max_jobs is already a pointer.
This causes the config value to be written to the wrong address
and dropped.
Pass max_jobs directly.
I recommend reading Documentation/SubmittingPatches:
[1]: https://github.com/git/git/blob/master/Documentation/SubmittingPatches
>
> Signed-off-by: Saagar Jha <saagar@saagarjha.com>
> ---
> submodule-config: fix reading submodule.fetchJobs
>
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2287%2Fsaagarjha%2Fmaint-v2
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2287/saagarjha/maint-v2
> Pull-Request: https://github.com/git/git/pull/2287
>
> Range-diff vs v1:
>
> 1: 094c641227 ! 1: 868901f1a6 submodule-config: fix reading submodule.fetchJobs
> @@ Metadata
> ## Commit message ##
> submodule-config: fix reading submodule.fetchJobs
>
> + The old code accidentally passed &max_jobs rather than max_jobs into
> + config_from_gitmodules, which caused the setting to be written to the
> + wrong place and dropped.
> +
> Signed-off-by: Saagar Jha <saagar@saagarjha.com>
>
> ## submodule-config.c ##
>
>
> submodule-config.c | 2 +-
> t/t7406-submodule-update.sh | 10 ++++++++++
> 2 files changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/submodule-config.c b/submodule-config.c
> index 1f19fe2077..57b190678e 100644
> --- a/submodule-config.c
> +++ b/submodule-config.c
> @@ -1037,5 +1037,5 @@ static int gitmodules_update_clone_config(const char *var, const char *value,
>
> void update_clone_config_from_gitmodules(int *max_jobs)
> {
> - config_from_gitmodules(gitmodules_update_clone_config, the_repository, &max_jobs);
> + config_from_gitmodules(gitmodules_update_clone_config, the_repository, max_jobs);
> }
> diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh
> index 3adab12091..234a021fb3 100755
> --- a/t/t7406-submodule-update.sh
> +++ b/t/t7406-submodule-update.sh
> @@ -1055,6 +1055,16 @@ test_expect_success 'submodule update can be run in parallel' '
> )
> '
>
> +test_expect_success 'submodule update honors fetch jobs config from .gitmodules' '
> + test_when_finished "rm -rf super3" &&
> + git clone cloned super3 &&
> + (cd super3 &&
> + git config -f .gitmodules submodule.fetchJobs 67 &&
> + GIT_TRACE="$(pwd)/trace.out" git submodule update --init &&
> + grep "67 tasks" trace.out
> + )
> +'
> +
> test_expect_success 'git clone passes the parallel jobs config on to submodules' '
> test_when_finished "rm -rf super4" &&
> GIT_TRACE=$(pwd)/trace.out git clone --recurse-submodules --jobs 7 . super4 &&
>
> base-commit: 67ad42147a7acc2af6074753ebd03d904476118f
> --
> gitgitgadget
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v3] submodule-config: fix reading submodule.fetchJobs
2026-05-03 11:57 ` [PATCH v2] " Saagar Jha via GitGitGadget
2026-05-03 13:07 ` Pablo
@ 2026-05-03 13:52 ` Saagar Jha via GitGitGadget
2026-05-10 3:50 ` [PATCH v4] " Saagar Jha via GitGitGadget
1 sibling, 1 reply; 9+ messages in thread
From: Saagar Jha via GitGitGadget @ 2026-05-03 13:52 UTC (permalink / raw)
To: git; +Cc: Pablo, Saagar Jha, Saagar Jha
From: Saagar Jha <saagar@saagarjha.com>
update_clone_config_from_gitmodules() passes &max_jobs to
config_from_gitmodules(), but max_jobs is already a pointer. This causes
the config value to be written to the wrong address and get dropped.
Pass max_jobs directly.
Signed-off-by: Saagar Jha <saagar@saagarjha.com>
---
submodule-config: fix reading submodule.fetchJobs
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2287%2Fsaagarjha%2Fmaint-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2287/saagarjha/maint-v3
Pull-Request: https://github.com/git/git/pull/2287
Range-diff vs v2:
1: 868901f1a6 ! 1: 70fb2ede0f submodule-config: fix reading submodule.fetchJobs
@@ Metadata
## Commit message ##
submodule-config: fix reading submodule.fetchJobs
- The old code accidentally passed &max_jobs rather than max_jobs into
- config_from_gitmodules, which caused the setting to be written to the
- wrong place and dropped.
+ update_clone_config_from_gitmodules() passes &max_jobs to
+ config_from_gitmodules(), but max_jobs is already a pointer. This causes
+ the config value to be written to the wrong address and get dropped.
+
+ Pass max_jobs directly.
Signed-off-by: Saagar Jha <saagar@saagarjha.com>
submodule-config.c | 2 +-
t/t7406-submodule-update.sh | 10 ++++++++++
2 files changed, 11 insertions(+), 1 deletion(-)
diff --git a/submodule-config.c b/submodule-config.c
index 1f19fe2077..57b190678e 100644
--- a/submodule-config.c
+++ b/submodule-config.c
@@ -1037,5 +1037,5 @@ static int gitmodules_update_clone_config(const char *var, const char *value,
void update_clone_config_from_gitmodules(int *max_jobs)
{
- config_from_gitmodules(gitmodules_update_clone_config, the_repository, &max_jobs);
+ config_from_gitmodules(gitmodules_update_clone_config, the_repository, max_jobs);
}
diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh
index 3adab12091..234a021fb3 100755
--- a/t/t7406-submodule-update.sh
+++ b/t/t7406-submodule-update.sh
@@ -1055,6 +1055,16 @@ test_expect_success 'submodule update can be run in parallel' '
)
'
+test_expect_success 'submodule update honors fetch jobs config from .gitmodules' '
+ test_when_finished "rm -rf super3" &&
+ git clone cloned super3 &&
+ (cd super3 &&
+ git config -f .gitmodules submodule.fetchJobs 67 &&
+ GIT_TRACE="$(pwd)/trace.out" git submodule update --init &&
+ grep "67 tasks" trace.out
+ )
+'
+
test_expect_success 'git clone passes the parallel jobs config on to submodules' '
test_when_finished "rm -rf super4" &&
GIT_TRACE=$(pwd)/trace.out git clone --recurse-submodules --jobs 7 . super4 &&
base-commit: 67ad42147a7acc2af6074753ebd03d904476118f
--
gitgitgadget
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v2] submodule-config: fix reading submodule.fetchJobs
2026-05-03 13:07 ` Pablo
@ 2026-05-10 0:01 ` Junio C Hamano
0 siblings, 0 replies; 9+ messages in thread
From: Junio C Hamano @ 2026-05-10 0:01 UTC (permalink / raw)
To: Pablo; +Cc: Saagar Jha via GitGitGadget, git, Saagar Jha
Pablo <pabloosabaterr@gmail.com> writes:
> El dom, 3 may 2026 a las 13:57, Saagar Jha via GitGitGadget
> (<gitgitgadget@gmail.com>) escribió:
>>
>> From: Saagar Jha <saagar@saagarjha.com>
>>
>> The old code accidentally passed &max_jobs rather than max_jobs into
>> config_from_gitmodules, which caused the setting to be written to the
>> wrong place and dropped.
>
> Better, but we can improve this, following
> Documentation/SubmittingPatches [1], it is better to write in present
> tense something like:
>
> update_clone_config_from_gitmodules() passes &max_jobs to
> config_from_gitmodules(), but max_jobs is already a pointer.
> This causes the config value to be written to the wrong address
> and dropped.
>
> Pass max_jobs directly.
>
> I recommend reading Documentation/SubmittingPatches:
> [1]: https://github.com/git/git/blob/master/Documentation/SubmittingPatches
Very well said. And your version reads more natural in the context
of this project, to those who are used to "git log --no-merges".
The config_from_gitmodules() function takes (void *) as the third
parameter unfortunately, which forces the compiler to take any
pointer without complaints. Typically an "int" is the same size as
or narrower than a "(void *)", and has no stricter alignment
requirement than "(int *)", so passing the address of max_int (i.e.,
"&max_int") would have caused the callee to assign an "int" result
to the on-stack "max_int" variable, corrupting the pointer but
otherwise causing no crashes, since the pointer is not used after
the function returns. That is why we never noticed this at either
compile time or runtime. Only those who paid attention to the
actual return value, like the test added by this patch, would have
noticed it. Nicely done.
>> +test_expect_success 'submodule update honors fetch jobs config from .gitmodules' '
>> + test_when_finished "rm -rf super3" &&
>> + git clone cloned super3 &&
>> + (cd super3 &&
>> + git config -f .gitmodules submodule.fetchJobs 67 &&
>> + GIT_TRACE="$(pwd)/trace.out" git submodule update --init &&
>> + grep "67 tasks" trace.out
>> + )
>> +'
This inherited badness from existing tests in the same script, but
in the modern tests, the abouve would be written more like
test_expect_success 'submodule update honors fetch jobs config from .gitmodules' '
test_when_finished "rm -rf super3" &&
git clone cloned super3 &&
(
cd super3 &&
git config -f .gitmodules submodule.fetchJobs 67 &&
GIT_TRACE="$(pwd)/trace.out" git submodule update --init &&
test_grep "67 tasks" trace.out
)
'
in order to
(1) open-close parentheses for subshell on their own lines;
(2) test_grep to make the error stand out more when it fails
or avoiding a subshell altogether and do
test_expect_success 'submodule update honors fetch jobs config from .gitmodules' '
test_when_finished "rm -rf super3" &&
git clone cloned super3 &&
git -C super3 config -f .gitmodules submodule.fetchJobs 67 &&
GIT_TRACE="$(pwd)/trace.out" git -C super3 submodule update --init &&
test_grep "67 tasks" trace.out
'
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v4] submodule-config: fix reading submodule.fetchJobs
2026-05-03 13:52 ` [PATCH v3] " Saagar Jha via GitGitGadget
@ 2026-05-10 3:50 ` Saagar Jha via GitGitGadget
0 siblings, 0 replies; 9+ messages in thread
From: Saagar Jha via GitGitGadget @ 2026-05-10 3:50 UTC (permalink / raw)
To: git; +Cc: Pablo, Saagar Jha, Saagar Jha
From: Saagar Jha <saagar@saagarjha.com>
update_clone_config_from_gitmodules() passes &max_jobs to
config_from_gitmodules(), but max_jobs is already a pointer. This causes
the config value to be written to the wrong address and get dropped.
Pass max_jobs directly.
Signed-off-by: Saagar Jha <saagar@saagarjha.com>
---
submodule-config: fix reading submodule.fetchJobs
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2287%2Fsaagarjha%2Fmaint-v4
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2287/saagarjha/maint-v4
Pull-Request: https://github.com/git/git/pull/2287
Range-diff vs v3:
1: 70fb2ede0f ! 1: 415c17f4bb submodule-config: fix reading submodule.fetchJobs
@@ t/t7406-submodule-update.sh: test_expect_success 'submodule update can be run in
+test_expect_success 'submodule update honors fetch jobs config from .gitmodules' '
+ test_when_finished "rm -rf super3" &&
+ git clone cloned super3 &&
-+ (cd super3 &&
-+ git config -f .gitmodules submodule.fetchJobs 67 &&
-+ GIT_TRACE="$(pwd)/trace.out" git submodule update --init &&
-+ grep "67 tasks" trace.out
-+ )
++ git -C super3 config -f .gitmodules submodule.fetchJobs 67 &&
++ GIT_TRACE="$(pwd)/trace.out" git -C super3 submodule update --init &&
++ test_grep "67 tasks" trace.out
+'
+
test_expect_success 'git clone passes the parallel jobs config on to submodules' '
submodule-config.c | 2 +-
t/t7406-submodule-update.sh | 8 ++++++++
2 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/submodule-config.c b/submodule-config.c
index 1f19fe2077..57b190678e 100644
--- a/submodule-config.c
+++ b/submodule-config.c
@@ -1037,5 +1037,5 @@ static int gitmodules_update_clone_config(const char *var, const char *value,
void update_clone_config_from_gitmodules(int *max_jobs)
{
- config_from_gitmodules(gitmodules_update_clone_config, the_repository, &max_jobs);
+ config_from_gitmodules(gitmodules_update_clone_config, the_repository, max_jobs);
}
diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh
index 3adab12091..6abb00876a 100755
--- a/t/t7406-submodule-update.sh
+++ b/t/t7406-submodule-update.sh
@@ -1055,6 +1055,14 @@ test_expect_success 'submodule update can be run in parallel' '
)
'
+test_expect_success 'submodule update honors fetch jobs config from .gitmodules' '
+ test_when_finished "rm -rf super3" &&
+ git clone cloned super3 &&
+ git -C super3 config -f .gitmodules submodule.fetchJobs 67 &&
+ GIT_TRACE="$(pwd)/trace.out" git -C super3 submodule update --init &&
+ test_grep "67 tasks" trace.out
+'
+
test_expect_success 'git clone passes the parallel jobs config on to submodules' '
test_when_finished "rm -rf super4" &&
GIT_TRACE=$(pwd)/trace.out git clone --recurse-submodules --jobs 7 . super4 &&
base-commit: 67ad42147a7acc2af6074753ebd03d904476118f
--
gitgitgadget
^ permalink raw reply related [flat|nested] 9+ messages in thread
end of thread, other threads:[~2026-05-10 3:50 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-03 10:14 [PATCH] submodule-config: fix reading submodule.fetchJobs Saagar Jha via GitGitGadget
2026-05-03 11:07 ` Pablo
2026-05-03 11:24 ` Saagar Jha
2026-05-03 12:19 ` Pablo
2026-05-03 11:57 ` [PATCH v2] " Saagar Jha via GitGitGadget
2026-05-03 13:07 ` Pablo
2026-05-10 0:01 ` Junio C Hamano
2026-05-03 13:52 ` [PATCH v3] " Saagar Jha via GitGitGadget
2026-05-10 3:50 ` [PATCH v4] " Saagar Jha via GitGitGadget
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox