git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v2] for-each-repo: do nothing on empty config

From
Derrick Stolee via GitGitGadget <gitgitgadget@gmail.com>
Date
Jan 6, 2021, 19:19 UTC
Message-ID
<pull.834.v2.git.1609960783988.gitgitgadget@gmail.com>
In-Reply-To
<pull.834.git.1609857770445.gitgitgadget@gmail.com>
From: Derrick Stolee <dstolee@microsoft.com>

'git for-each-repo --config=X' should return success without calling any subcommands when the config key 'X' has no value. The current implementation instead segfaults.

A user could run into this issue if they used 'git maintenance start' to initialize their cron schedule using 'git for-each-repo --config=maintenance.repo ...' but then using 'git maintenance unregister' to remove the config option. (Note: 'git maintenance stop' would remove the config _and_ remove the cron schedule.)

Add a simple test to ensure this works.
Reported-by: Andreas Bühmann <dev@uuml.de>
Helped-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
---
    for-each-repo: do nothing on empty config
    
    Thanks, Andreas, for drawing my attention to this bug.
    
    [1] https://github.com/gitgitgadget/git/issues/833
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-834%2Fderrickstolee%2Ffor-each-repo-empty-config-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-834/derrickstolee/for-each-repo-empty-config-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/834
Range-diff vs v1:
 1:  36dccbb8c20 ! 1:  a1f1300bacb for-each-repo: do nothing on empty config
     @@ Commit message
          Add a simple test to ensure this works.
      
          Reported-by: Andreas Bühmann <dev@uuml.de>
     +    Helped-by: Eric Sunshine <sunshine@sunshineco.com>
          Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
      
       ## builtin/for-each-repo.c ##
     @@ builtin/for-each-repo.c: int cmd_for_each_repo(int argc, const char **argv, cons
       	values = repo_config_get_value_multi(the_repository,
       					     config_key);
       
     ++	/*
     ++	 * Do nothing on an empty list, which is equivalent to the case
     ++	 * where the config variable does not exist at all.
     ++	 */
      +	if (!values)
     -+		return result;
     ++		return 0;
      +
       	for (i = 0; !result && i < values->nr; i++)
       		result = run_command_on_repo(values->items[i].string, &args);
     @@ t/t0068-for-each-repo.sh: test_expect_success 'run based on configured value' '
       '
       
      +test_expect_success 'do nothing on empty config' '
     -+	git for-each-repo --config=bogus.config -- these args would fail
     ++	# will fail if any subcommand is run
     ++	git for-each-repo --config=bogus.config -- false
      +'
      +
       test_done
 builtin/for-each-repo.c  | 7 +++++++
 t/t0068-for-each-repo.sh | 5 +++++
 2 files changed, 12 insertions(+)
diff --git a/builtin/for-each-repo.c b/builtin/for-each-repo.c
index 5bba623ff12..52be64a4373 100644
--- a/builtin/for-each-repo.c
+++ b/builtin/for-each-repo.c
@@ -51,6 +51,13 @@ int cmd_for_each_repo(int argc, const char **argv, const char *prefix)
 	values = repo_config_get_value_multi(the_repository,
 					     config_key);
 
+	/*
+	 * Do nothing on an empty list, which is equivalent to the case
+	 * where the config variable does not exist at all.
+	 */
+	if (!values)
+		return 0;
+
 	for (i = 0; !result && i < values->nr; i++)
 		result = run_command_on_repo(values->items[i].string, &args);
 
diff --git a/t/t0068-for-each-repo.sh b/t/t0068-for-each-repo.sh
index 136b4ec8392..c3efba4da0b 100755
--- a/t/t0068-for-each-repo.sh
+++ b/t/t0068-for-each-repo.sh
@@ -27,4 +27,9 @@ test_expect_success 'run based on configured value' '
 	grep again message
 '
 
+test_expect_success 'do nothing on empty config' '
+	# will fail if any subcommand is run
+	git for-each-repo --config=bogus.config -- false
+'
+
 test_done

base-commit: 4950b2a2b5c4731e4c9d5b803739a6979b23fed6
-- 
gitgitgadget
Previous: Derrick StoleeNext: Derrick Stolee via GitGitGadget
Message 14 of 15 in “for-each-repo: do nothing on empty config”
  1. for-each-repo: do nothing on empty configDerrick Stolee via GitGitGadget, Jan 5, 2021
  2. Eric SunshineJan 5, 2021
  3. Derrick StoleeJan 6, 2021
  4. Eric SunshineJan 6, 2021
  5. Derrick StoleeJan 6, 2021
  6. Eric SunshineJan 6, 2021
  7. Junio C HamanoJan 6, 2021
  8. Eric SunshineJan 7, 2021
  9. Junio C HamanoJan 6, 2021
  10. Derrick StoleeJan 6, 2021
  11. Junio C HamanoJan 6, 2021
  12. Junio C HamanoJan 6, 2021
  13. Derrick StoleeJan 7, 2021
  14. for-each-repo: do nothing on empty configDerrick Stolee via GitGitGadget, Jan 6, 2021
  15. for-each-repo: do nothing on empty configDerrick Stolee via GitGitGadget, Jan 8, 2021

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.