Volume XXII, number 279Tuesday, October 6, 2026Latest message 15 minutes ago

The Git List

News and archive of git@vger.kernel.org, since April 2005

patchsubmodule: warn on valueless active config

7 messages between Aug 14, 2026 and Aug 14, 2026, from Tilak Raaz, Weijie Yuan, D. Ben Knoble, Junio C Hamano, tilak-raaz.

Plain Markdown or JSON for tools and agents. Diffs are folded; open one to read it.

Tilak RaazAug 14, 2026, 17:37 UTC on lore
Hi everyone,

My name is Tilak (he/him), and I am a second-year Electronics and Instrumentation Engineering student at NIT Rourkela. I am preparing to apply for GSoC 2027 and am starting my contributions to Git.

Regarding my background with Git: I have built Git from source, successfully navigated the codebase, and tackled the NEEDSWORK comment regarding valueless 'submodule.active' configurations in submodule.c.

Below is my microproject patch resolving this issue by switching from repo_config_get_string_multi() to repo_config_get_value_multi() and adding an automated test case in t7400-submodule-basic.sh.

I look forward to your feedback!

Thanks, Tilak

From 08a2f244efab6e4cf21638d87a721ca664ed9433 Mon Sep 17 00:00:00 2001
From: tilak-raaz <raaztilak07@gmail.com>
Date: Fri, 14 Aug 2026 22:50:11 +0530
Subject: [GSoC PATCH] submodule: warn on valueless active config

The config parser previously threw a hard error if 'submodule.active' was provided without a value, causing commands to abort.

Swap repo_config_get_string_multi() to repo_config_get_value_multi() to parse valueless keys safely, and emit a warning to the user rather than crashing.

This resolves a NEEDSWORK comment in submodule.c.
Signed-off-by: tilak-raaz <raaztilak07@gmail.com>
---
 submodule.c                | 16 ++++++++--------
 t/t7400-submodule-basic.sh | 11 +++++++++++
 2 files changed, 19 insertions(+), 8 deletions(-)
Show changes to 2 files +19 −8

submodule.c, t/t7400-submodule-basic.sh

diff --git a/submodule.c b/submodule.c
index 5c92575888..b709c429ba 100644
--- a/submodule.c
+++ b/submodule.c
@@ -231,11 +231,7 @@ int option_parse_recurse_submodules_worktree_updater(const struct option *opt,
 /*
  * Determine if a submodule has been initialized at a given 'path'
  */
-/*
- * NEEDSWORK: Emit a warning if submodule.active exists, but is valueless,
- * ie, the config looks like: "[submodule] active\n".
- * Since that is an invalid pathspec, we should inform the user.
- */
+
 int is_tree_submodule_active(struct repository *repo,
 			     const struct object_id *treeish_name,
 			     const char *path)
@@ -261,14 +257,18 @@ int is_tree_submodule_active(struct repository *repo,
 	free(key);
 
 	/* submodule.active is set */
-	if (!repo_config_get_string_multi(repo, "submodule.active", &sl)) {
+	if (!repo_config_get_value_multi(repo, "submodule.active", &sl)) {
 		struct pathspec ps;
 		struct strvec args = STRVEC_INIT;
 		const struct string_list_item *item;
 
 		for_each_string_list_item(item, sl) {
-			strvec_push(&args, item->string);
-		}
+                if (!item->string) {
+                        warning(_("submodule.active is present but has no value"));
+                        continue;
+                }
+                strvec_push(&args, item->string);
+        }
 
 		parse_pathspec(&ps, 0, 0, NULL, args.v);
 		ret = match_pathspec(repo->index, &ps, path, strlen(path), 0, NULL, 1);
diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
index eefdecb0bd..afc62ffa0b 100755
--- a/t/t7400-submodule-basic.sh
+++ b/t/t7400-submodule-basic.sh
@@ -1549,4 +1549,15 @@ test_expect_success 'submodule add fails when name is reused' '
 	)
 '
 
+
+test_expect_success 'warn on valueless submodule.active' '
+        test_when_finished "rm -rf empty-active" &&
+        git init empty-active &&
+        test_commit -C empty-active initial &&
+        git -c protocol.file.allow=always -C empty-active submodule add ../empty-active sub &&
+        git -C empty-active config --unset submodule.sub.active &&
+        printf "[submodule]\n\tactive\n" >>empty-active/.git/config &&
+        git -C empty-active submodule status 2>err &&
+        grep "submodule.active is present but has no value" err
+'
 test_done

-- 
2.50.1 (Apple Git-155)
Weijie YuanAug 14, 2026, 17:56 UTC in reply to Tilak Raaz on lore

Re: [GSoC PATCH] submodule: warn on valueless active config

On Fri, Aug 14, 2026 at 11:07:29PM +0530, Tilak Raaz wrote:
Show 16 quoted lines
> Hi everyone,
> 
> My name is Tilak  (he/him), and I am a second-year Electronics and
> Instrumentation Engineering student at NIT Rourkela. I am preparing to
> apply for GSoC 2027 and am starting my contributions to Git.
> 
> Regarding my background with Git: I have built Git from source,
> successfully
> navigated the codebase, and tackled the NEEDSWORK comment regarding
> valueless 'submodule.active' configurations in submodule.c.
> 
> Below is my microproject patch resolving this issue by switching from
> repo_config_get_string_multi() to repo_config_get_value_multi() and
> adding an automated test case in t7400-submodule-basic.sh.
> 
> I look forward to your feedback!
Thanks!

However, my suggestion is that it would be better to place your patch in the main body of the email text rather than in the attachment. Please take a look at Documentation/SubmittingPatches [[attachment]]

And it also seems that the automated program 'b4' is unable to recognize your patch, which may make the development process less convenient for the developers and the maintainer.

$ b4 am https://lore.kernel.org/git/CABB4Jh3UUXvmAJpefaiP-xVRQfGRdTF2jW8GkdhbA1BXe6Okdw@mail.gmail.com/ Looking up CABB4Jh3UUXvmAJpefaiP-xVRQfGRdTF2jW8GkdhbA1BXe6Okdw@mail.gmail.com Analyzing 1 messages in the thread No patches found.

Please correct me if I'm wrong.
Thanks.
Tilak RaazAug 14, 2026, 18:04 UTC in reply to Weijie Yuan on lore

Re: [GSoC PATCH] submodule: warn on valueless active config

On Fri, Aug 14, 2026 Weijie Yuan <wy@wyuan.org> wrote:
Show 9 quoted lines
> Thanks!
>
> However, my suggestion is that it would be better to place your patch in
> the main body of the email text rather than in the attachment.
> Please take a look at Documentation/SubmittingPatches
>
> And it also seems that the automated program 'b4' is unable to recognize
> your patch, which may make the development process less convenient for
> the developers and the maintainer.
Hi Weijie,

Thank you for the quick feedback and for pointing me to the documentation! I apologize for using an attachment; I am still getting my mailing list workflow configured.

Here is the patch provided inline as plain text so that `b4` can parse it correctly:

From 08a2f244efab6e4cf21638d87a721ca664ed9433 Mon Sep 17 00:00:00 2001
From: tilak-raaz <raaztilak07@gmail.com>
Date: Fri, 14 Aug 2026 22:50:11 +0530
Subject: [GSoC PATCH] submodule: warn on valueless active config

The config parser previously threw a hard error if 'submodule.active' was provided without a value, causing commands to abort.

Swap repo_config_get_string_multi() to repo_config_get_value_multi() to parse valueless keys safely, and emit a warning to the user rather than crashing.

This resolves a NEEDSWORK comment in submodule.c.
Signed-off-by: tilak-raaz <raaztilak07@gmail.com>
---
 submodule.c                | 16 ++++++++--------
 t/t7400-submodule-basic.sh | 11 +++++++++++
 2 files changed, 19 insertions(+), 8 deletions(-)
Show changes to 2 files +19 −8

submodule.c, t/t7400-submodule-basic.sh

diff --git a/submodule.c b/submodule.c
index 5c92575888..b709c429ba 100644
--- a/submodule.c
+++ b/submodule.c
@@ -231,11 +231,7 @@ int
option_parse_recurse_submodules_worktree_updater(const struct option
*opt,
 /*
  * Determine if a submodule has been initialized at a given 'path'
  */
-/*
- * NEEDSWORK: Emit a warning if submodule.active exists, but is valueless,
- * ie, the config looks like: "[submodule] active\n".
- * Since that is an invalid pathspec, we should inform the user.
- */
+
 int is_tree_submodule_active(struct repository *repo,
      const struct object_id *treeish_name,
      const char *path)
@@ -261,14 +257,18 @@ int is_tree_submodule_active(struct repository *repo,
  free(key);

  /* submodule.active is set */
- if (!repo_config_get_string_multi(repo, "submodule.active", &sl)) {
+ if (!repo_config_get_value_multi(repo, "submodule.active", &sl)) {
  struct pathspec ps;
  struct strvec args = STRVEC_INIT;
  const struct string_list_item *item;

  for_each_string_list_item(item, sl) {
- strvec_push(&args, item->string);
- }
+                if (!item->string) {
+                        warning(_("submodule.active is present but
has no value"));
+                        continue;
+                }
+                strvec_push(&args, item->string);
+        }

  parse_pathspec(&ps, 0, 0, NULL, args.v);
  ret = match_pathspec(repo->index, &ps, path, strlen(path), 0, NULL, 1);
diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
index eefdecb0bd..afc62ffa0b 100755
--- a/t/t7400-submodule-basic.sh
+++ b/t/t7400-submodule-basic.sh
@@ -1549,4 +1549,15 @@ test_expect_success 'submodule add fails when
name is reused' '
  )
 '

+
+test_expect_success 'warn on valueless submodule.active' '
+        test_when_finished "rm -rf empty-active" &&
+        git init empty-active &&
+        test_commit -C empty-active initial &&
+        git -c protocol.file.allow=always -C empty-active submodule
add ../empty-active sub &&
+        git -C empty-active config --unset submodule.sub.active &&
+        printf "[submodule]\n\tactive\n" >>empty-active/.git/config &&
+        git -C empty-active submodule status 2>err &&
+        grep "submodule.active is present but has no value" err
+'
 test_done

-- 
2.50.1 (Apple Git-155)


On Fri, Aug 14, 2026 at 11:26 PM Weijie Yuan <wy@wyuan.org> wrote:
>
> On Fri, Aug 14, 2026 at 11:07:29PM +0530, Tilak Raaz wrote:
> > Hi everyone,
> >
> > My name is Tilak  (he/him), and I am a second-year Electronics and
> > Instrumentation Engineering student at NIT Rourkela. I am preparing to
> > apply for GSoC 2027 and am starting my contributions to Git.
> >
> > Regarding my background with Git: I have built Git from source,
> > successfully
> > navigated the codebase, and tackled the NEEDSWORK comment regarding
> > valueless 'submodule.active' configurations in submodule.c.
> >
> > Below is my microproject patch resolving this issue by switching from
> > repo_config_get_string_multi() to repo_config_get_value_multi() and
> > adding an automated test case in t7400-submodule-basic.sh.
> >
> > I look forward to your feedback!
>
> Thanks!
>
> However, my suggestion is that it would be better to place your patch in
> the main body of the email text rather than in the attachment.
> Please take a look at Documentation/SubmittingPatches [[attachment]]
>
> And it also seems that the automated program 'b4' is unable to recognize
> your patch, which may make the development process less convenient for
> the developers and the maintainer.
>
> $ b4 am https://lore.kernel.org/git/CABB4Jh3UUXvmAJpefaiP-xVRQfGRdTF2jW8GkdhbA1BXe6Okdw@mail.gmail.com/
> Looking up CABB4Jh3UUXvmAJpefaiP-xVRQfGRdTF2jW8GkdhbA1BXe6Okdw@mail.gmail.com
> Analyzing 1 messages in the thread
> No patches found.
>
> Please correct me if I'm wrong.
>
> Thanks.
D. Ben KnobleAug 14, 2026, 19:07 UTC in reply to Tilak Raaz on lore

Re: [GSoC PATCH] submodule: warn on valueless active config

On Fri, Aug 14, 2026 at 2:05 PM Tilak Raaz <raaztilak07@gmail.com> wrote:
Show 73 quoted lines
>
> On Fri, Aug 14, 2026 Weijie Yuan <wy@wyuan.org> wrote:
> > Thanks!
> >
> > However, my suggestion is that it would be better to place your patch in
> > the main body of the email text rather than in the attachment.
> > Please take a look at Documentation/SubmittingPatches
> >
> > And it also seems that the automated program 'b4' is unable to recognize
> > your patch, which may make the development process less convenient for
> > the developers and the maintainer.
>
> Hi Weijie,
>
> Thank you for the quick feedback and for pointing me to the documentation!
> I apologize for using an attachment; I am still getting my mailing list workflow
> configured.
>
> Here is the patch provided inline as plain text so that `b4` can parse
> it correctly:
>
> From 08a2f244efab6e4cf21638d87a721ca664ed9433 Mon Sep 17 00:00:00 2001
> From: tilak-raaz <raaztilak07@gmail.com>
> Date: Fri, 14 Aug 2026 22:50:11 +0530
> Subject: [GSoC PATCH] submodule: warn on valueless active config
>
> The config parser previously threw a hard error if 'submodule.active'
> was provided without a value, causing commands to abort.
>
> Swap repo_config_get_string_multi() to repo_config_get_value_multi()
> to parse valueless keys safely, and emit a warning to the user rather
> than crashing.
>
> This resolves a NEEDSWORK comment in submodule.c.
>
> Signed-off-by: tilak-raaz <raaztilak07@gmail.com>
> ---
>  submodule.c                | 16 ++++++++--------
>  t/t7400-submodule-basic.sh | 11 +++++++++++
>  2 files changed, 19 insertions(+), 8 deletions(-)
>
> diff --git a/submodule.c b/submodule.c
> index 5c92575888..b709c429ba 100644
> --- a/submodule.c
> +++ b/submodule.c
> @@ -231,11 +231,7 @@ int
> option_parse_recurse_submodules_worktree_updater(const struct option
> *opt,
>  /*
>   * Determine if a submodule has been initialized at a given 'path'
>   */
> -/*
> - * NEEDSWORK: Emit a warning if submodule.active exists, but is valueless,
> - * ie, the config looks like: "[submodule] active\n".
> - * Since that is an invalid pathspec, we should inform the user.
> - */
> +
>  int is_tree_submodule_active(struct repository *repo,
>       const struct object_id *treeish_name,
>       const char *path)
> @@ -261,14 +257,18 @@ int is_tree_submodule_active(struct repository *repo,
>   free(key);
>
>   /* submodule.active is set */
> - if (!repo_config_get_string_multi(repo, "submodule.active", &sl)) {
> + if (!repo_config_get_value_multi(repo, "submodule.active", &sl)) {
>   struct pathspec ps;
>   struct strvec args = STRVEC_INIT;
>   const struct string_list_item *item;
>
>   for_each_string_list_item(item, sl) {
> - strvec_push(&args, item->string);
> - }

It's hard to tell, but I think (depending on _how_ you sent this patch with GMail) the indentation has become corrupted, and the patch won't apply.

Give the tips in git-send-email.io a try; especially with GMail, I've found the safest way to send patches is with git-send-email. (I reply to conversations from just about any mail client, though.)

Junio C HamanoAug 14, 2026, 19:14 UTC in reply to Tilak Raaz on lore

Re: [GSoC PATCH] submodule: warn on valueless active config

Tilak Raaz <raaztilak07@gmail.com> writes:
> The config parser previously threw a hard error if 'submodule.active'
> was provided without a value, causing commands to abort.

The standard helper to use is config_error_nonbool() when you need to report a section.variable defined this way

	[section]
		variable
without "= value", and section.variable cannot be a Boolean true.

The patch seems to be heavily whitespace damaged, and cannot be used, though.

Thanks.
tilak-raazAug 14, 2026, 21:24 UTC in reply to Tilak Raaz on lore

[GSoC PATCH v2] submodule: warn on valueless active config

The config parser previously threw a hard error if 'submodule.active' was provided without a value, causing commands to abort.

Swap repo_config_get_string_multi() to repo_config_get_value_multi() to parse valueless keys safely. Use the standard config_error_nonbool() helper to emit a warning to the user rather than crashing.

This resolves a NEEDSWORK comment in submodule.c.
Signed-off-by: tilak-raaz <raaztilak07@gmail.com>
---

Thank you Ben and Weijie for the guidance on git-send-email. I have properly configured my terminal to prevent the whitespace damage caused by the Gmail web client.

Junio, thank you for pointing me to the correct helper function. 
Changes in v2:
- Use config_error_nonbool() to report valueless submodule.active.
- Add a regression test for the valueless configuration.
- Fix whitespace/indentation issues from v1.
 submodule.c                | 12 ++++++------
 t/t7400-submodule-basic.sh | 11 +++++++++++
 2 files changed, 17 insertions(+), 6 deletions(-)
Show changes to 2 files +17 −6

submodule.c, t/t7400-submodule-basic.sh

diff --git a/submodule.c b/submodule.c
index 5c92575888..07d1fc63e9 100644
--- a/submodule.c
+++ b/submodule.c
@@ -231,11 +231,7 @@ int option_parse_recurse_submodules_worktree_updater(const struct option *opt,
 /*
  * Determine if a submodule has been initialized at a given 'path'
  */
-/*
- * NEEDSWORK: Emit a warning if submodule.active exists, but is valueless,
- * ie, the config looks like: "[submodule] active\n".
- * Since that is an invalid pathspec, we should inform the user.
- */
+
 int is_tree_submodule_active(struct repository *repo,
 			     const struct object_id *treeish_name,
 			     const char *path)
@@ -261,12 +257,16 @@ int is_tree_submodule_active(struct repository *repo,
 	free(key);
 
 	/* submodule.active is set */
-	if (!repo_config_get_string_multi(repo, "submodule.active", &sl)) {
+	if (!repo_config_get_value_multi(repo, "submodule.active", &sl)) {
 		struct pathspec ps;
 		struct strvec args = STRVEC_INIT;
 		const struct string_list_item *item;
 
 		for_each_string_list_item(item, sl) {
+			 if (!item->string) {
+				config_error_nonbool("submodule.active");
+				continue;
+			}
 			strvec_push(&args, item->string);
 		}
 
diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
index eefdecb0bd..74c26f6630 100755
--- a/t/t7400-submodule-basic.sh
+++ b/t/t7400-submodule-basic.sh
@@ -1549,4 +1549,15 @@ test_expect_success 'submodule add fails when name is reused' '
 	)
 '
 
+
+test_expect_success 'warn on valueless submodule.active' '
+test_when_finished "rm -rf empty-active" &&
+git init empty-active &&
+test_commit -C empty-active initial &&
+git -c protocol.file.allow=always -C empty-active submodule add ../empty-active sub &&
+git -C empty-active config --unset submodule.sub.active &&
+printf "[submodule]\n\tactive\n" >>empty-active/.git/config &&
+git -C empty-active submodule status 2>err &&
+grep "missing value for .submodule.active." err
+'
 test_done

-- 
2.50.1 (Apple Git-155)
Junio C HamanoAug 14, 2026, 22:04 UTC in reply to tilak-raaz on lore

Re: [GSoC PATCH v2] submodule: warn on valueless active config

tilak-raaz <raaztilak07@gmail.com> writes:
> The config parser previously threw a hard error if 'submodule.active'
> was provided without a value, causing commands to abort.
An exerpt from Documentation/SubmittingPatches:
    [[present-tense]]
    The problem statement that describes the status quo is written in the
    present tense.  Write "The code does X when it is given input Y",
    instead of "The code used to do Y when given input X".  You do not
    have to say "Currently"---the status quo in the problem statement is
    about the code _without_ your change, by project convention.
> Swap repo_config_get_string_multi() to repo_config_get_value_multi()
> to parse valueless keys safely. Use the standard config_error_nonbool()
"valueless true", I think.
> helper to emit a warning to the user rather than crashing.
Good.
> This resolves a NEEDSWORK comment in submodule.c.

Good. Resolving an existing NEEDSWORK is a two step process, (1) to determine if it still does make sense to do what it suggests to do, and then (2) do it. The early part of the proposed log message solves a half of step (1), in a sense that crashing is bad. The other half is what we should do instead of crashing.

Show 26 quoted lines
> -/*
> - * NEEDSWORK: Emit a warning if submodule.active exists, but is valueless,
> - * ie, the config looks like: "[submodule] active\n".
> - * Since that is an invalid pathspec, we should inform the user.
> - */
> +
>  int is_tree_submodule_active(struct repository *repo,
>  			     const struct object_id *treeish_name,
>  			     const char *path)
> @@ -261,12 +257,16 @@ int is_tree_submodule_active(struct repository *repo,
>  	free(key);
>  
>  	/* submodule.active is set */
> -	if (!repo_config_get_string_multi(repo, "submodule.active", &sl)) {
> +	if (!repo_config_get_value_multi(repo, "submodule.active", &sl)) {
>  		struct pathspec ps;
>  		struct strvec args = STRVEC_INIT;
>  		const struct string_list_item *item;
>  
>  		for_each_string_list_item(item, sl) {
> +			 if (!item->string) {
> +				config_error_nonbool("submodule.active");
> +				continue;
> +			}
>  			strvec_push(&args, item->string);
>  		}
And we do warn, but I am not sure if "continue" is sensible, though.

Since we know that the configuration is broken, we should cause the command to fail (i.e., exit with a non-zero status), shouldn't we?

Show 18 quoted lines
>  
> diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
> index eefdecb0bd..74c26f6630 100755
> --- a/t/t7400-submodule-basic.sh
> +++ b/t/t7400-submodule-basic.sh
> @@ -1549,4 +1549,15 @@ test_expect_success 'submodule add fails when name is reused' '
>  	)
>  '
>  
> +
> +test_expect_success 'warn on valueless submodule.active' '
> +test_when_finished "rm -rf empty-active" &&
> +git init empty-active &&
> +test_commit -C empty-active initial &&
> +git -c protocol.file.allow=always -C empty-active submodule add ../empty-active sub &&
> +git -C empty-active config --unset submodule.sub.active &&
> +printf "[submodule]\n\tactive\n" >>empty-active/.git/config &&
> +git -C empty-active submodule status 2>err &&
In other words, shouldn't this say
	test_must_fail git submodule status &&
> +grep "missing value for .submodule.active." err
> +'

Curiously, the test part of your patch is severely whitespace-damaged, even though the C part looked OK. This is quite puzzling.

Back to recent threads