{"thread":{"id":"66174","subject":"[GSoC PATCH] submodule: warn on valueless active config","startedAt":"2026-08-14T17:37:44Z","lastAt":"2026-08-14T22:04:30Z","messageCount":7,"participants":["Tilak Raaz","Weijie Yuan","D. Ben Knoble","Junio C Hamano","tilak-raaz"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"550629","messageId":"CABB4Jh3UUXvmAJpefaiP-xVRQfGRdTF2jW8GkdhbA1BXe6Okdw@mail.gmail.com","threadId":"66174","inReplyTo":null,"subject":"[GSoC PATCH] submodule: warn on valueless active config","fromName":"Tilak Raaz","fromEmail":"raaztilak07@gmail.com","sentAt":"2026-08-14T17:37:29Z","receivedAt":"2026-08-14T17:37:44Z","isPatch":true,"body":"Hi everyone,\n\nMy name is Tilak  (he/him), and I am a second-year Electronics and\nInstrumentation Engineering student at NIT Rourkela. I am preparing to\napply for GSoC 2027 and am starting my contributions to Git.\n\nRegarding my background with Git: I have built Git from source,\nsuccessfully\nnavigated the codebase, and tackled the NEEDSWORK comment regarding\nvalueless 'submodule.active' configurations in submodule.c.\n\nBelow is my microproject patch resolving this issue by switching from\nrepo_config_get_string_multi() to repo_config_get_value_multi() and\nadding an automated test case in t7400-submodule-basic.sh.\n\nI look forward to your feedback!\n\nThanks,\nTilak\n\n\nFrom 08a2f244efab6e4cf21638d87a721ca664ed9433 Mon Sep 17 00:00:00 2001\nFrom: tilak-raaz <raaztilak07@gmail.com>\nDate: Fri, 14 Aug 2026 22:50:11 +0530\nSubject: [GSoC PATCH] submodule: warn on valueless active config\n\nThe config parser previously threw a hard error if 'submodule.active'\nwas provided without a value, causing commands to abort.\n\nSwap repo_config_get_string_multi() to repo_config_get_value_multi()\nto parse valueless keys safely, and emit a warning to the user rather\nthan crashing.\n\nThis resolves a NEEDSWORK comment in submodule.c.\n\nSigned-off-by: tilak-raaz <raaztilak07@gmail.com>\n---\n submodule.c                | 16 ++++++++--------\n t/t7400-submodule-basic.sh | 11 +++++++++++\n 2 files changed, 19 insertions(+), 8 deletions(-)\n\ndiff --git a/submodule.c b/submodule.c\nindex 5c92575888..b709c429ba 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -231,11 +231,7 @@ int option_parse_recurse_submodules_worktree_updater(const struct option *opt,\n /*\n  * Determine if a submodule has been initialized at a given 'path'\n  */\n-/*\n- * NEEDSWORK: Emit a warning if submodule.active exists, but is valueless,\n- * ie, the config looks like: \"[submodule] active\\n\".\n- * Since that is an invalid pathspec, we should inform the user.\n- */\n+\n int is_tree_submodule_active(struct repository *repo,\n \t\t\t     const struct object_id *treeish_name,\n \t\t\t     const char *path)\n@@ -261,14 +257,18 @@ int is_tree_submodule_active(struct repository *repo,\n \tfree(key);\n \n \t/* submodule.active is set */\n-\tif (!repo_config_get_string_multi(repo, \"submodule.active\", &sl)) {\n+\tif (!repo_config_get_value_multi(repo, \"submodule.active\", &sl)) {\n \t\tstruct pathspec ps;\n \t\tstruct strvec args = STRVEC_INIT;\n \t\tconst struct string_list_item *item;\n \n \t\tfor_each_string_list_item(item, sl) {\n-\t\t\tstrvec_push(&args, item->string);\n-\t\t}\n+                if (!item->string) {\n+                        warning(_(\"submodule.active is present but has no value\"));\n+                        continue;\n+                }\n+                strvec_push(&args, item->string);\n+        }\n \n \t\tparse_pathspec(&ps, 0, 0, NULL, args.v);\n \t\tret = match_pathspec(repo->index, &ps, path, strlen(path), 0, NULL, 1);\ndiff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh\nindex eefdecb0bd..afc62ffa0b 100755\n--- a/t/t7400-submodule-basic.sh\n+++ b/t/t7400-submodule-basic.sh\n@@ -1549,4 +1549,15 @@ test_expect_success 'submodule add fails when name is reused' '\n \t)\n '\n \n+\n+test_expect_success 'warn on valueless submodule.active' '\n+        test_when_finished \"rm -rf empty-active\" &&\n+        git init empty-active &&\n+        test_commit -C empty-active initial &&\n+        git -c protocol.file.allow=always -C empty-active submodule add ../empty-active sub &&\n+        git -C empty-active config --unset submodule.sub.active &&\n+        printf \"[submodule]\\n\\tactive\\n\" >>empty-active/.git/config &&\n+        git -C empty-active submodule status 2>err &&\n+        grep \"submodule.active is present but has no value\" err\n+'\n test_done\n-- \n2.50.1 (Apple Git-155)\n\n"},{"id":"550632","messageId":"an9W4XwY8X4ZFHpA@wyuan.org","threadId":"66174","inReplyTo":"CABB4Jh3UUXvmAJpefaiP-xVRQfGRdTF2jW8GkdhbA1BXe6Okdw@mail.gmail.com","subject":"Re: [GSoC PATCH] submodule: warn on valueless active config","fromName":"Weijie Yuan","fromEmail":"wy@wyuan.org","sentAt":"2026-08-14T17:56:49Z","receivedAt":"2026-08-14T17:56:57Z","isPatch":true,"body":"On Fri, Aug 14, 2026 at 11:07:29PM +0530, Tilak Raaz wrote:\n> Hi everyone,\n> \n> My name is Tilak  (he/him), and I am a second-year Electronics and\n> Instrumentation Engineering student at NIT Rourkela. I am preparing to\n> apply for GSoC 2027 and am starting my contributions to Git.\n> \n> Regarding my background with Git: I have built Git from source,\n> successfully\n> navigated the codebase, and tackled the NEEDSWORK comment regarding\n> valueless 'submodule.active' configurations in submodule.c.\n> \n> Below is my microproject patch resolving this issue by switching from\n> repo_config_get_string_multi() to repo_config_get_value_multi() and\n> adding an automated test case in t7400-submodule-basic.sh.\n> \n> I look forward to your feedback!\n\nThanks!\n\nHowever, my suggestion is that it would be better to place your patch in\nthe main body of the email text rather than in the attachment.\nPlease take a look at Documentation/SubmittingPatches [[attachment]]\n\nAnd it also seems that the automated program 'b4' is unable to recognize\nyour patch, which may make the development process less convenient for\nthe developers and the maintainer.\n\n$ b4 am https://lore.kernel.org/git/CABB4Jh3UUXvmAJpefaiP-xVRQfGRdTF2jW8GkdhbA1BXe6Okdw@mail.gmail.com/\nLooking up CABB4Jh3UUXvmAJpefaiP-xVRQfGRdTF2jW8GkdhbA1BXe6Okdw@mail.gmail.com\nAnalyzing 1 messages in the thread\nNo patches found.\n\nPlease correct me if I'm wrong.\n\nThanks.\n"},{"id":"550633","messageId":"CABB4Jh1fUXKNn483FjD2S6U4cYVMEP6z+fjWMi8XRT+NQdNnYw@mail.gmail.com","threadId":"66174","inReplyTo":"an9W4XwY8X4ZFHpA@wyuan.org","subject":"Re: [GSoC PATCH] submodule: warn on valueless active config","fromName":"Tilak Raaz","fromEmail":"raaztilak07@gmail.com","sentAt":"2026-08-14T18:04:54Z","receivedAt":"2026-08-14T18:05:08Z","isPatch":true,"body":"On Fri, Aug 14, 2026 Weijie Yuan <wy@wyuan.org> wrote:\n> Thanks!\n>\n> However, my suggestion is that it would be better to place your patch in\n> the main body of the email text rather than in the attachment.\n> Please take a look at Documentation/SubmittingPatches\n>\n> And it also seems that the automated program 'b4' is unable to recognize\n> your patch, which may make the development process less convenient for\n> the developers and the maintainer.\n\nHi Weijie,\n\nThank you for the quick feedback and for pointing me to the documentation!\nI apologize for using an attachment; I am still getting my mailing list workflow\nconfigured.\n\nHere is the patch provided inline as plain text so that `b4` can parse\nit correctly:\n\nFrom 08a2f244efab6e4cf21638d87a721ca664ed9433 Mon Sep 17 00:00:00 2001\nFrom: tilak-raaz <raaztilak07@gmail.com>\nDate: Fri, 14 Aug 2026 22:50:11 +0530\nSubject: [GSoC PATCH] submodule: warn on valueless active config\n\nThe config parser previously threw a hard error if 'submodule.active'\nwas provided without a value, causing commands to abort.\n\nSwap repo_config_get_string_multi() to repo_config_get_value_multi()\nto parse valueless keys safely, and emit a warning to the user rather\nthan crashing.\n\nThis resolves a NEEDSWORK comment in submodule.c.\n\nSigned-off-by: tilak-raaz <raaztilak07@gmail.com>\n---\n submodule.c                | 16 ++++++++--------\n t/t7400-submodule-basic.sh | 11 +++++++++++\n 2 files changed, 19 insertions(+), 8 deletions(-)\n\ndiff --git a/submodule.c b/submodule.c\nindex 5c92575888..b709c429ba 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -231,11 +231,7 @@ int\noption_parse_recurse_submodules_worktree_updater(const struct option\n*opt,\n /*\n  * Determine if a submodule has been initialized at a given 'path'\n  */\n-/*\n- * NEEDSWORK: Emit a warning if submodule.active exists, but is valueless,\n- * ie, the config looks like: \"[submodule] active\\n\".\n- * Since that is an invalid pathspec, we should inform the user.\n- */\n+\n int is_tree_submodule_active(struct repository *repo,\n      const struct object_id *treeish_name,\n      const char *path)\n@@ -261,14 +257,18 @@ int is_tree_submodule_active(struct repository *repo,\n  free(key);\n\n  /* submodule.active is set */\n- if (!repo_config_get_string_multi(repo, \"submodule.active\", &sl)) {\n+ if (!repo_config_get_value_multi(repo, \"submodule.active\", &sl)) {\n  struct pathspec ps;\n  struct strvec args = STRVEC_INIT;\n  const struct string_list_item *item;\n\n  for_each_string_list_item(item, sl) {\n- strvec_push(&args, item->string);\n- }\n+                if (!item->string) {\n+                        warning(_(\"submodule.active is present but\nhas no value\"));\n+                        continue;\n+                }\n+                strvec_push(&args, item->string);\n+        }\n\n  parse_pathspec(&ps, 0, 0, NULL, args.v);\n  ret = match_pathspec(repo->index, &ps, path, strlen(path), 0, NULL, 1);\ndiff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh\nindex eefdecb0bd..afc62ffa0b 100755\n--- a/t/t7400-submodule-basic.sh\n+++ b/t/t7400-submodule-basic.sh\n@@ -1549,4 +1549,15 @@ test_expect_success 'submodule add fails when\nname is reused' '\n  )\n '\n\n+\n+test_expect_success 'warn on valueless submodule.active' '\n+        test_when_finished \"rm -rf empty-active\" &&\n+        git init empty-active &&\n+        test_commit -C empty-active initial &&\n+        git -c protocol.file.allow=always -C empty-active submodule\nadd ../empty-active sub &&\n+        git -C empty-active config --unset submodule.sub.active &&\n+        printf \"[submodule]\\n\\tactive\\n\" >>empty-active/.git/config &&\n+        git -C empty-active submodule status 2>err &&\n+        grep \"submodule.active is present but has no value\" err\n+'\n test_done\n-- \n2.50.1 (Apple Git-155)\n\n\nOn Fri, Aug 14, 2026 at 11:26 PM Weijie Yuan <wy@wyuan.org> wrote:\n>\n> On Fri, Aug 14, 2026 at 11:07:29PM +0530, Tilak Raaz wrote:\n> > Hi everyone,\n> >\n> > My name is Tilak  (he/him), and I am a second-year Electronics and\n> > Instrumentation Engineering student at NIT Rourkela. I am preparing to\n> > apply for GSoC 2027 and am starting my contributions to Git.\n> >\n> > Regarding my background with Git: I have built Git from source,\n> > successfully\n> > navigated the codebase, and tackled the NEEDSWORK comment regarding\n> > valueless 'submodule.active' configurations in submodule.c.\n> >\n> > Below is my microproject patch resolving this issue by switching from\n> > repo_config_get_string_multi() to repo_config_get_value_multi() and\n> > adding an automated test case in t7400-submodule-basic.sh.\n> >\n> > I look forward to your feedback!\n>\n> Thanks!\n>\n> However, my suggestion is that it would be better to place your patch in\n> the main body of the email text rather than in the attachment.\n> Please take a look at Documentation/SubmittingPatches [[attachment]]\n>\n> And it also seems that the automated program 'b4' is unable to recognize\n> your patch, which may make the development process less convenient for\n> the developers and the maintainer.\n>\n> $ b4 am https://lore.kernel.org/git/CABB4Jh3UUXvmAJpefaiP-xVRQfGRdTF2jW8GkdhbA1BXe6Okdw@mail.gmail.com/\n> Looking up CABB4Jh3UUXvmAJpefaiP-xVRQfGRdTF2jW8GkdhbA1BXe6Okdw@mail.gmail.com\n> Analyzing 1 messages in the thread\n> No patches found.\n>\n> Please correct me if I'm wrong.\n>\n> Thanks.\n"},{"id":"550640","messageId":"CALnO6CCDQBWS7dP7CZSbKE3f8rw4x=NAJhGyE7HCJRjJq_2dEA@mail.gmail.com","threadId":"66174","inReplyTo":"CABB4Jh1fUXKNn483FjD2S6U4cYVMEP6z+fjWMi8XRT+NQdNnYw@mail.gmail.com","subject":"Re: [GSoC PATCH] submodule: warn on valueless active config","fromName":"D. Ben Knoble","fromEmail":"ben.knoble@gmail.com","sentAt":"2026-08-14T19:07:39Z","receivedAt":"2026-08-14T19:07:51Z","isPatch":true,"body":"On Fri, Aug 14, 2026 at 2:05 PM Tilak Raaz <raaztilak07@gmail.com> wrote:\n>\n> On Fri, Aug 14, 2026 Weijie Yuan <wy@wyuan.org> wrote:\n> > Thanks!\n> >\n> > However, my suggestion is that it would be better to place your patch in\n> > the main body of the email text rather than in the attachment.\n> > Please take a look at Documentation/SubmittingPatches\n> >\n> > And it also seems that the automated program 'b4' is unable to recognize\n> > your patch, which may make the development process less convenient for\n> > the developers and the maintainer.\n>\n> Hi Weijie,\n>\n> Thank you for the quick feedback and for pointing me to the documentation!\n> I apologize for using an attachment; I am still getting my mailing list workflow\n> configured.\n>\n> Here is the patch provided inline as plain text so that `b4` can parse\n> it correctly:\n>\n> From 08a2f244efab6e4cf21638d87a721ca664ed9433 Mon Sep 17 00:00:00 2001\n> From: tilak-raaz <raaztilak07@gmail.com>\n> Date: Fri, 14 Aug 2026 22:50:11 +0530\n> Subject: [GSoC PATCH] submodule: warn on valueless active config\n>\n> The config parser previously threw a hard error if 'submodule.active'\n> was provided without a value, causing commands to abort.\n>\n> Swap repo_config_get_string_multi() to repo_config_get_value_multi()\n> to parse valueless keys safely, and emit a warning to the user rather\n> than crashing.\n>\n> This resolves a NEEDSWORK comment in submodule.c.\n>\n> Signed-off-by: tilak-raaz <raaztilak07@gmail.com>\n> ---\n>  submodule.c                | 16 ++++++++--------\n>  t/t7400-submodule-basic.sh | 11 +++++++++++\n>  2 files changed, 19 insertions(+), 8 deletions(-)\n>\n> diff --git a/submodule.c b/submodule.c\n> index 5c92575888..b709c429ba 100644\n> --- a/submodule.c\n> +++ b/submodule.c\n> @@ -231,11 +231,7 @@ int\n> option_parse_recurse_submodules_worktree_updater(const struct option\n> *opt,\n>  /*\n>   * Determine if a submodule has been initialized at a given 'path'\n>   */\n> -/*\n> - * NEEDSWORK: Emit a warning if submodule.active exists, but is valueless,\n> - * ie, the config looks like: \"[submodule] active\\n\".\n> - * Since that is an invalid pathspec, we should inform the user.\n> - */\n> +\n>  int is_tree_submodule_active(struct repository *repo,\n>       const struct object_id *treeish_name,\n>       const char *path)\n> @@ -261,14 +257,18 @@ int is_tree_submodule_active(struct repository *repo,\n>   free(key);\n>\n>   /* submodule.active is set */\n> - if (!repo_config_get_string_multi(repo, \"submodule.active\", &sl)) {\n> + if (!repo_config_get_value_multi(repo, \"submodule.active\", &sl)) {\n>   struct pathspec ps;\n>   struct strvec args = STRVEC_INIT;\n>   const struct string_list_item *item;\n>\n>   for_each_string_list_item(item, sl) {\n> - strvec_push(&args, item->string);\n> - }\n\nIt's hard to tell, but I think (depending on _how_ you sent this patch\nwith GMail) the indentation has become corrupted, and the patch won't\napply.\n\nGive the tips in git-send-email.io a try; especially with GMail, I've\nfound the safest way to send patches is with git-send-email. (I reply\nto conversations from just about any mail client, though.)\n"},{"id":"550641","messageId":"xmqqecg0ms5g.fsf@gitster.g","threadId":"66174","inReplyTo":"CABB4Jh1fUXKNn483FjD2S6U4cYVMEP6z+fjWMi8XRT+NQdNnYw@mail.gmail.com","subject":"Re: [GSoC PATCH] submodule: warn on valueless active config","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-08-14T19:14:03Z","receivedAt":"2026-08-14T19:14:06Z","isPatch":true,"body":"Tilak Raaz <raaztilak07@gmail.com> writes:\n\n> The config parser previously threw a hard error if 'submodule.active'\n> was provided without a value, causing commands to abort.\n\nThe standard helper to use is config_error_nonbool() when you need\nto report a section.variable defined this way\n\n\t[section]\n\t\tvariable\n\nwithout \"= value\", and section.variable cannot be a Boolean true.\n\nThe patch seems to be heavily whitespace damaged, and cannot be\nused, though.\n\nThanks.\n"},{"id":"550649","messageId":"20260814212431.43626-1-raaztilak07@gmail.com","threadId":"66174","inReplyTo":"CABB4Jh3UUXvmAJpefaiP-xVRQfGRdTF2jW8GkdhbA1BXe6Okdw@mail.gmail.com","subject":"[GSoC PATCH v2] submodule: warn on valueless active config","fromName":"tilak-raaz","fromEmail":"raaztilak07@gmail.com","sentAt":"2026-08-14T21:24:30Z","receivedAt":"2026-08-14T21:24:41Z","isPatch":true,"body":"The config parser previously threw a hard error if 'submodule.active'\nwas provided without a value, causing commands to abort.\n\nSwap repo_config_get_string_multi() to repo_config_get_value_multi()\nto parse valueless keys safely. Use the standard config_error_nonbool()\nhelper to emit a warning to the user rather than crashing.\n\nThis resolves a NEEDSWORK comment in submodule.c.\n\nSigned-off-by: tilak-raaz <raaztilak07@gmail.com>\n---\n\nThank you Ben and Weijie for the guidance on git-send-email. I have \nproperly configured my terminal to prevent the whitespace damage caused \nby the Gmail web client.\n\nJunio, thank you for pointing me to the correct helper function. \n\nChanges in v2:\n- Use config_error_nonbool() to report valueless submodule.active.\n- Add a regression test for the valueless configuration.\n- Fix whitespace/indentation issues from v1.\n submodule.c                | 12 ++++++------\n t/t7400-submodule-basic.sh | 11 +++++++++++\n 2 files changed, 17 insertions(+), 6 deletions(-)\n\ndiff --git a/submodule.c b/submodule.c\nindex 5c92575888..07d1fc63e9 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -231,11 +231,7 @@ int option_parse_recurse_submodules_worktree_updater(const struct option *opt,\n /*\n  * Determine if a submodule has been initialized at a given 'path'\n  */\n-/*\n- * NEEDSWORK: Emit a warning if submodule.active exists, but is valueless,\n- * ie, the config looks like: \"[submodule] active\\n\".\n- * Since that is an invalid pathspec, we should inform the user.\n- */\n+\n int is_tree_submodule_active(struct repository *repo,\n \t\t\t     const struct object_id *treeish_name,\n \t\t\t     const char *path)\n@@ -261,12 +257,16 @@ int is_tree_submodule_active(struct repository *repo,\n \tfree(key);\n \n \t/* submodule.active is set */\n-\tif (!repo_config_get_string_multi(repo, \"submodule.active\", &sl)) {\n+\tif (!repo_config_get_value_multi(repo, \"submodule.active\", &sl)) {\n \t\tstruct pathspec ps;\n \t\tstruct strvec args = STRVEC_INIT;\n \t\tconst struct string_list_item *item;\n \n \t\tfor_each_string_list_item(item, sl) {\n+\t\t\t if (!item->string) {\n+\t\t\t\tconfig_error_nonbool(\"submodule.active\");\n+\t\t\t\tcontinue;\n+\t\t\t}\n \t\t\tstrvec_push(&args, item->string);\n \t\t}\n \ndiff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh\nindex eefdecb0bd..74c26f6630 100755\n--- a/t/t7400-submodule-basic.sh\n+++ b/t/t7400-submodule-basic.sh\n@@ -1549,4 +1549,15 @@ test_expect_success 'submodule add fails when name is reused' '\n \t)\n '\n \n+\n+test_expect_success 'warn on valueless submodule.active' '\n+test_when_finished \"rm -rf empty-active\" &&\n+git init empty-active &&\n+test_commit -C empty-active initial &&\n+git -c protocol.file.allow=always -C empty-active submodule add ../empty-active sub &&\n+git -C empty-active config --unset submodule.sub.active &&\n+printf \"[submodule]\\n\\tactive\\n\" >>empty-active/.git/config &&\n+git -C empty-active submodule status 2>err &&\n+grep \"missing value for .submodule.active.\" err\n+'\n test_done\n-- \n2.50.1 (Apple Git-155)\n\n"},{"id":"550651","messageId":"xmqqqzk0l5oz.fsf@gitster.g","threadId":"66174","inReplyTo":"20260814212431.43626-1-raaztilak07@gmail.com","subject":"Re: [GSoC PATCH v2] submodule: warn on valueless active config","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-08-14T22:04:28Z","receivedAt":"2026-08-14T22:04:30Z","isPatch":true,"body":"tilak-raaz <raaztilak07@gmail.com> writes:\n\n> The config parser previously threw a hard error if 'submodule.active'\n> was provided without a value, causing commands to abort.\n\nAn exerpt from Documentation/SubmittingPatches:\n\n    [[present-tense]]\n    The problem statement that describes the status quo is written in the\n    present tense.  Write \"The code does X when it is given input Y\",\n    instead of \"The code used to do Y when given input X\".  You do not\n    have to say \"Currently\"---the status quo in the problem statement is\n    about the code _without_ your change, by project convention.\n\n> Swap repo_config_get_string_multi() to repo_config_get_value_multi()\n> to parse valueless keys safely. Use the standard config_error_nonbool()\n\n\"valueless true\", I think.\n\n> helper to emit a warning to the user rather than crashing.\n\nGood.\n\n> This resolves a NEEDSWORK comment in submodule.c.\n\nGood.  Resolving an existing NEEDSWORK is a two step process, (1) to\ndetermine if it still does make sense to do what it suggests to do,\nand then (2) do it.  The early part of the proposed log message\nsolves a half of step (1), in a sense that crashing is bad.  The\nother half is what we should do instead of crashing.\n\n> -/*\n> - * NEEDSWORK: Emit a warning if submodule.active exists, but is valueless,\n> - * ie, the config looks like: \"[submodule] active\\n\".\n> - * Since that is an invalid pathspec, we should inform the user.\n> - */\n> +\n>  int is_tree_submodule_active(struct repository *repo,\n>  \t\t\t     const struct object_id *treeish_name,\n>  \t\t\t     const char *path)\n> @@ -261,12 +257,16 @@ int is_tree_submodule_active(struct repository *repo,\n>  \tfree(key);\n>  \n>  \t/* submodule.active is set */\n> -\tif (!repo_config_get_string_multi(repo, \"submodule.active\", &sl)) {\n> +\tif (!repo_config_get_value_multi(repo, \"submodule.active\", &sl)) {\n>  \t\tstruct pathspec ps;\n>  \t\tstruct strvec args = STRVEC_INIT;\n>  \t\tconst struct string_list_item *item;\n>  \n>  \t\tfor_each_string_list_item(item, sl) {\n> +\t\t\t if (!item->string) {\n> +\t\t\t\tconfig_error_nonbool(\"submodule.active\");\n> +\t\t\t\tcontinue;\n> +\t\t\t}\n>  \t\t\tstrvec_push(&args, item->string);\n>  \t\t}\n\nAnd we do warn, but I am not sure if \"continue\" is sensible, though.\n\nSince we know that the configuration is broken, we should cause the\ncommand to fail (i.e., exit with a non-zero status), shouldn't we?\n\n\n>  \n> diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh\n> index eefdecb0bd..74c26f6630 100755\n> --- a/t/t7400-submodule-basic.sh\n> +++ b/t/t7400-submodule-basic.sh\n> @@ -1549,4 +1549,15 @@ test_expect_success 'submodule add fails when name is reused' '\n>  \t)\n>  '\n>  \n> +\n> +test_expect_success 'warn on valueless submodule.active' '\n> +test_when_finished \"rm -rf empty-active\" &&\n> +git init empty-active &&\n> +test_commit -C empty-active initial &&\n> +git -c protocol.file.allow=always -C empty-active submodule add ../empty-active sub &&\n> +git -C empty-active config --unset submodule.sub.active &&\n> +printf \"[submodule]\\n\\tactive\\n\" >>empty-active/.git/config &&\n> +git -C empty-active submodule status 2>err &&\n\nIn other words, shouldn't this say\n\n\ttest_must_fail git submodule status &&\n\n> +grep \"missing value for .submodule.active.\" err\n> +'\n\nCuriously, the test part of your patch is severely\nwhitespace-damaged, even though the C part looked OK.  This is quite\npuzzling.\n\n"}]}