[PATCH] gc: add git maintenance list command

[PATCH] gc: add git maintenance list command

[Rémy Léone via GitGitGadget]

From: =?UTF-8?q?R=C3=A9my=20L=C3=A9one?= <rleone@scaleway.com>

List all repositories registered for background maintenance.

This displays the paths of all repositories that are configured in the
maintenance.repo config variable. By default, it reads from the global
config, but you can specify a different config file with the
--config-file option.

Signed-off-by: Rémy Léone <rleone@scaleway.com>
---
    gc: add git maintenance list command
    
    List all repositories registered for background maintenance. This
    displays the paths of all repositories that are configured in the
    maintenance.repo config variable. By default, it reads from the global
    config, but you can specify a different config file with the
    --config-file option.

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2201%2Fremyleone%2Fmaintenance_list-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2201/remyleone/maintenance_list-v1
Pull-Request: https://github.com/git/git/pull/2201

 Documentation/git-maintenance.adoc |  9 ++++-
 builtin/gc.c                       | 59 ++++++++++++++++++++++++++++++
 t/t7900-maintenance.sh             | 33 +++++++++++++++++
 3 files changed, 100 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-maintenance.adoc b/Documentation/git-maintenance.adoc
index bda616f14c..17cb52ea2d 100644
--- a/Documentation/git-maintenance.adoc
+++ b/Documentation/git-maintenance.adoc
@@ -11,7 +11,7 @@ SYNOPSIS
 [verse]
 'git maintenance' run [<options>]
 'git maintenance' start [--scheduler=<scheduler>]
-'git maintenance' (stop|register|unregister) [<options>]
+'git maintenance' (stop|register|unregister|list) [<options>]
 'git maintenance' is-needed [<options>]
 
 
@@ -85,6 +85,13 @@ The `unregister` subcommand will report an error if the current repository
 is not already registered. Use the `--force` option to return success even
 when the current repository is not registered.
 
+list::
+	List all repositories registered for background maintenance. This
+	displays the paths of all repositories that are configured in the
+	`maintenance.repo` config variable. By default, it reads from the
+	global config, but you can specify a different config file with
+	the `--config-file` option.
+
 is-needed::
     Check whether maintenance needs to be run without actually running it.
     Exits with a 0 status code if maintenance needs to be run, 1 otherwise.
diff --git a/builtin/gc.c b/builtin/gc.c
index 4390eee6ec..9b15e236b4 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -2239,6 +2239,64 @@ static int maintenance_unregister(int argc, const char **argv, const char *prefi
 	return 0;
 }
 
+static const char * const builtin_maintenance_list_usage[] = {
+	"git maintenance list [--config-file <path>]",
+	NULL
+};
+
+static int maintenance_list(int argc, const char **argv, const char *prefix,
+		    struct repository *repo UNUSED)
+{
+	char *config_file = NULL;
+	struct option options[] = {
+		OPT_STRING(0, "config-file", &config_file, N_("file"), N_("use given config file")),
+		OPT_END(),
+	};
+	const char *key = "maintenance.repo";
+	const struct string_list *list;
+	struct config_set cs = { { 0 } };
+	char *global_config_file = NULL;
+
+	argc = parse_options(argc, argv, prefix, options,
+			     builtin_maintenance_list_usage, 0);
+	if (argc)
+		usage_with_options(builtin_maintenance_list_usage,
+				   options);
+
+	if (config_file) {
+		git_configset_init(&cs);
+		git_configset_add_file(&cs, config_file);
+		if (git_configset_get_string_multi(&cs, key, &list)) {
+			/* No repositories registered in custom config */
+			git_configset_clear(&cs);
+			return 0;
+		}
+	} else {
+		global_config_file = git_global_config();
+		if (!global_config_file)
+			die(_("$HOME not set"));
+		git_configset_init(&cs);
+		git_configset_add_file(&cs, global_config_file);
+		if (git_configset_get_string_multi(&cs, key, &list)) {
+			/* No repositories registered in global config */
+			free(global_config_file);
+			git_configset_clear(&cs);
+			return 0;
+		}
+	}
+
+	{
+		struct string_list_item *item;
+		for_each_string_list_item(item, list) {
+			printf("%s\n", item->string);
+		}
+	}
+
+	free(global_config_file);
+	git_configset_clear(&cs);
+	return 0;
+}
+
 static const char *get_frequency(enum schedule_priority schedule)
 {
 	switch (schedule) {
@@ -3535,6 +3593,7 @@ int cmd_maintenance(int argc,
 		OPT_SUBCOMMAND("stop", &fn, maintenance_stop),
 		OPT_SUBCOMMAND("register", &fn, maintenance_register),
 		OPT_SUBCOMMAND("unregister", &fn, maintenance_unregister),
+		OPT_SUBCOMMAND("list", &fn, maintenance_list),
 		OPT_SUBCOMMAND("is-needed", &fn, maintenance_is_needed),
 		OPT_END(),
 	};
diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh
index 7cc0ce57f8..9dc16e154e 100755
--- a/t/t7900-maintenance.sh
+++ b/t/t7900-maintenance.sh
@@ -1482,4 +1482,37 @@ test_expect_success 'maintenance aborts with existing lock file' '
 	test_grep "Another scheduled git-maintenance(1) process seems to be running" err
 '
 
+test_expect_success 'maintenance list shows registered repositories' '
+	test_when_finished "rm -rf repo1 repo2" &&
+	test_when_finished git config --global --unset-all maintenance.repo &&
+	git init repo1 &&
+	git init repo2 &&
+	(
+		cd repo1 &&
+		git maintenance register &&
+		cd ../repo2 &&
+		git maintenance register
+	) &&
+	git config --global --get-all maintenance.repo >expect &&
+	git maintenance list >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'maintenance list with --config-file' '
+	CUSTOM_CONFIG="./custom-maintenance-config" &&
+	test_when_finished "rm -rf repo3 repo4" &&
+	test_when_finished rm -f "$CUSTOM_CONFIG" &&
+	git init repo3 &&
+	git init repo4 &&
+	(
+		cd repo3 &&
+		git maintenance register --config-file "../$CUSTOM_CONFIG" &&
+		cd ../repo4 &&
+		git maintenance register --config-file "../$CUSTOM_CONFIG"
+	) &&
+	git config --file="$CUSTOM_CONFIG" --get-all maintenance.repo >expect &&
+	git maintenance list --config-file "$CUSTOM_CONFIG" >actual &&
+	test_cmp expect actual
+'
+
 test_done

base-commit: 852829b3dd2fe4e7c7fc4d8badde644cf1b66c74
-- 
gitgitgadget

Re: [PATCH] gc: add git maintenance list command

[Pablo Sabater]

> char *config_file = NULL;

>	if (config_file) {
>		git_configset_init(&cs);
>		git_configset_add_file(&cs, config_file);
>		if (git_configset_get_string_multi(&cs, key, &list)) {
>			/* No repositories registered in custom config */
>			git_configset_clear(&cs);
>			return 0;
>		}
>	} else {
>		global_config_file = git_global_config();
>		if (!global_config_file)
>			die(_("$HOME not set"));
>		git_configset_init(&cs);
>		git_configset_add_file(&cs, global_config_file);
>		if (git_configset_get_string_multi(&cs, key, &list)) {
>			/* No repositories registered in global config */
>			free(global_config_file);
>			git_configset_clear(&cs);
>			return 0;
>		}
>	}

Here the branches look too similar, after the third line at the else it becomes
exactly the same. If you notice, global_config_file is initialized as NULL
then in case of !config_file, its value comes from git_global_config().
Anyways, there's no need to separate the free logic because both NULL and
git_global_config() can be freed.

you can avoid this by extracting the diferent logic from the else

  if (!config_file) {
      config_file = git_global_config();
      if (!config_file)
          die(_("$HOME not set"));
      global_config_file = config_file;
  }

and then the rest of the code is common for both cases

  git_configset_init(&cs);
  git_configset_add_file(&cs, config_file);
  if (git_configset_get_string_multi(&cs, key, &list)) {
      free(global_config_file);
      git_configset_clear(&cs);
      return 0;
  }