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

tilak-raaz <[email protected]>
Newsgroups org.kernel.vger.git
Message-ID <[email protected]>
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 <[email protected]>
---

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)
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.