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

[tilak-raaz <raaztilak07@gmail.com>]

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(-)

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)