@matheusbsbs / matheus-bagliano / commits / 1bb6bdb646

builtin/maintenance: centralize configuration of explicit tasks

Users of git-maintenance(1) can explicitly ask it to run specific tasks by passing the `--task=` command line option. This option can be passed multiple times, which causes us to execute tasks in the same order as the tasks have been provided by the user. The order in which tasks are run is computed in `task_option_parse()`: every time we parse such a command line argument, we modify the global array of tasks by seting the selected index for that specific task. This has two downsides: - We modify global state, which makes it hard to follow the logic. - The configuration of tasks is split across multiple different functions, so it is not easy to figure out the different factors that play a role in selecting tasks. Refactor the logic so that `task_option_parse()` does not modify global state anymore. Instead, this function now only collects the list of configured tasks. The logic to configure ordering of the respective tasks is then deferred to `initialize_task_config()`. This refactoring solves the second problem, that the configuration of tasks is spread across multiple different locations. The first problem, that we modify global state, will be fixed in a subsequent commit. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Patrick Steinhardt committed Jun 3, 2025 at 16:01 UTC 1bb6bdb646583a2fc2e0e6436f5cdabdd4d14189

1 file changed +24 -23

builtin/gc.c

+24 -23

@@ -1690,15 +1690,22 @@ static void initialize_maintenance_strategy(void)

1690 }

1691 }

1692

1693 -static void initialize_task_config(int schedule)

1693 +static void initialize_task_config(const struct string_list *selected_tasks,

1694 + int schedule)

1695 {

1695 - int i;

1696 struct strbuf config_name = STRBUF_INIT;

1697

1698 + for (size_t i = 0; i < TASK__COUNT; i++)

1699 + tasks[i].selected_order = -1;

1700 + for (size_t i = 0; i < selected_tasks->nr; i++) {

1701 + struct maintenance_task *task = selected_tasks->items[i].util;

1702 + task->selected_order = i;

1703 + }

1704 +

1705 if (schedule)

1706 initialize_maintenance_strategy();

1707

1701 - for (i = 0; i < TASK__COUNT; i++) {

1708 + for (size_t i = 0; i < TASK__COUNT; i++) {

1709 int config_value;

1710 char *config_str;

1711

@@ -1722,33 +1729,28 @@ static void initialize_task_config(int schedule)

1729 strbuf_release(&config_name);

1730 }

1731

1725 -static int task_option_parse(const struct option *opt UNUSED,

1732 +static int task_option_parse(const struct option *opt,

1733 const char *arg, int unset)

1734 {

1728 - int i, num_selected = 0;

1729 - struct maintenance_task *task = NULL;

1735 + struct string_list *selected_tasks = opt->value;

1736 + size_t i;

1737

1738 BUG_ON_OPT_NEG(unset);

1739

1733 - for (i = 0; i < TASK__COUNT; i++) {

1734 - if (tasks[i].selected_order >= 0)

1735 - num_selected++;

1736 - if (!strcasecmp(tasks[i].name, arg)) {

1737 - task = &tasks[i];

1738 - }

1739 - }

1740 -

1741 - if (!task) {

1740 + for (i = 0; i < TASK__COUNT; i++)

1741 + if (!strcasecmp(tasks[i].name, arg))

1742 + break;

1743 + if (i >= TASK__COUNT) {

1744 error(_("'%s' is not a valid task"), arg);

1745 return 1;

1746 }

1747

1746 - if (task->selected_order >= 0) {

1748 + if (unsorted_string_list_has_string(selected_tasks, arg)) {

1749 error(_("task '%s' cannot be selected multiple times"), arg);

1750 return 1;

1751 }

1752

1751 - task->selected_order = num_selected + 1;

1753 + string_list_append(selected_tasks, arg)->util = &tasks[i];

1754

1755 return 0;

1756 }

@@ -1756,8 +1758,8 @@ static int task_option_parse(const struct option *opt UNUSED,

1758 static int maintenance_run(int argc, const char **argv, const char *prefix,

1759 struct repository *repo UNUSED)

1760 {

1759 - int i;

1761 struct maintenance_run_opts opts = MAINTENANCE_RUN_OPTS_INIT;

1762 + struct string_list selected_tasks = STRING_LIST_INIT_DUP;

1763 struct gc_config cfg = GC_CONFIG_INIT;

1764 struct option builtin_maintenance_run_options[] = {

1765 OPT_BOOL(0, "auto", &opts.auto_flag,

@@ -1769,7 +1771,7 @@ static int maintenance_run(int argc, const char **argv, const char *prefix,

1771 maintenance_opt_schedule),

1772 OPT_BOOL(0, "quiet", &opts.quiet,

1773 N_("do not report progress or other information over stderr")),

1772 - OPT_CALLBACK_F(0, "task", NULL, N_("task"),

1774 + OPT_CALLBACK_F(0, "task", &selected_tasks, N_("task"),

1775 N_("run a specific task"),

1776 PARSE_OPT_NONEG, task_option_parse),

1777 OPT_END()

@@ -1778,9 +1780,6 @@ static int maintenance_run(int argc, const char **argv, const char *prefix,

1780

1781 opts.quiet = !isatty(2);

1782

1781 - for (i = 0; i < TASK__COUNT; i++)

1782 - tasks[i].selected_order = -1;

1783 -

1783 argc = parse_options(argc, argv, prefix,

1784 builtin_maintenance_run_options,

1785 builtin_maintenance_run_usage,

@@ -1790,13 +1789,15 @@ static int maintenance_run(int argc, const char **argv, const char *prefix,

1789 die(_("use at most one of --auto and --schedule=<frequency>"));

1790

1791 gc_config(&cfg);

1793 - initialize_task_config(opts.schedule);

1792 + initialize_task_config(&selected_tasks, opts.schedule);

1793

1794 if (argc != 0)

1795 usage_with_options(builtin_maintenance_run_usage,

1796 builtin_maintenance_run_options);

1797

1798 ret = maintenance_run_tasks(&opts, &cfg);

1799 +

1800 + string_list_clear(&selected_tasks, 0);

1801 gc_config_release(&cfg);

1802 return ret;

1803 }