builtin/maintenance: extend "maintenance.strategy" to manual maintenance

The "maintenance.strategy" configuration allows users to configure how
Git is supposed to perform repository maintenance. The idea is that we
provide a set of high-level strategies that may be useful in different
contexts, like for example when handling a large monorepo. Furthermore,
the strategy can be tweaked by the user by overriding specific tasks.

In its current form though, the strategy only applies to scheduled
maintenance. This creates something of a gap, as scheduled and manual
maintenance will now use _different_ strategies as the latter would
continue to use git-gc(1) by default. This makes the strategies way less
useful than they could be on the one hand. But even more importantly,
the two different strategies might clash with one another, where one of
the strategies performs maintenance in such a way that it discards
benefits from the other strategy.

So ideally, it should be possible to pick one strategy that then applies
globally to all the different ways that we perform maintenance. This
doesn't necessarily mean that the strategy always does the _same_ thing
for every maintenance type. But it means that the strategy can configure
the different types to work in tandem with each other.

Change the meaning of "maintenance.strategy" accordingly so that the
strategy is applied to both types, manual and scheduled. As preceding
commits have introduced logic to run maintenance tasks depending on this
type we can tweak strategies so that they perform those tasks depending
on the context.

Note that this raises the question of backwards compatibility: when the
user has configured the "incremental" strategy we would have ignored
that strategy beforehand. Instead, repository maintenance would have
continued to use git-gc(1) by default.

But luckily, we can match that behaviour by:

  - Keeping all current tasks of the incremental strategy as
    `MAINTENANCE_TYPE_SCHEDULED`. This ensures that those tasks will not
    run during manual maintenance.

  - Configuring the "gc" task so that it is invoked during manual
    maintenance.

Like this, the user shouldn't observe any difference in behaviour.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Acked-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/Documentation/config/maintenance.adoc b/Documentation/config/maintenance.adoc
index 45fdafc2c63cf41618714fbe946f80aed4252363..b7e90a71a3df4cac20a23f2969028a7220b6f11b 100644 (file)
--- a/Documentation/config/maintenance.adoc
+++ b/Documentation/config/maintenance.adoc
@@ -16,19 +16,25 @@ detach.
 
 maintenance.strategy::
        This string config option provides a way to specify one of a few
-       recommended schedules for background maintenance. This only affects
-       which tasks are run during `git maintenance run --schedule=X`
-       commands, provided no `--task=<task>` arguments are provided.
-       Further, if a `maintenance.<task>.schedule` config value is set,
-       then that value is used instead of the one provided by
-       `maintenance.strategy`. The possible strategy strings are:
+       recommended strategies for repository maintenance. This affects
+       which tasks are run during `git maintenance run`, provided no
+       `--task=<task>` arguments are provided. This setting impacts manual
+       maintenance, auto-maintenance as well as scheduled maintenance. The
+       tasks that run may be different depending on the maintenance type.
 +
-* `none`: This default setting implies no tasks are run at any schedule.
+The maintenance strategy can be further tweaked by setting
+`maintenance.<task>.enabled` and `maintenance.<task>.schedule`. If set, these
+values are used instead of the defaults provided by `maintenance.strategy`.
++
+The possible strategies are:
++
+* `none`: This strategy implies no tasks are run at all. This is the default
+  strategy for scheduled maintenance.
 * `incremental`: This setting optimizes for performing small maintenance
   activities that do not delete any data. This does not schedule the `gc`
   task, but runs the `prefetch` and `commit-graph` tasks hourly, the
   `loose-objects` and `incremental-repack` tasks daily, and the `pack-refs`
-  task weekly.
+  task weekly. Manual repository maintenance uses the `gc` task.
 
 maintenance.<task>.enabled::
        This boolean config option controls whether the maintenance task

diff --git a/builtin/gc.c b/builtin/gc.c
index 6cc4f98c7aa33c808c009215de461083f7d1529d..3c0a9a2e5df64e90156baa1a4a7c35b89563eb97 100644 (file)
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -1873,6 +1873,20 @@ static const struct maintenance_strategy incremental_strategy = {
                        .type = MAINTENANCE_TYPE_SCHEDULED,
                        .schedule = SCHEDULE_WEEKLY,
                },
+               /*
+                * Historically, the "incremental" strategy was only available
+                * in the context of scheduled maintenance when set up via
+                * "maintenance.strategy". We have later expanded that config
+                * to also cover manual maintenance.
+                *
+                * To retain backwards compatibility with the previous status
+                * quo we thus run git-gc(1) in case manual maintenance was
+                * requested. This is the same as the default strategy, which
+                * would have been in use beforehand.
+                */
+               [TASK_GC] = {
+                       .type = MAINTENANCE_TYPE_MANUAL,
+               },
        },
 };
 
@@ -1916,19 +1930,20 @@ static void initialize_task_config(struct maintenance_run_opts *opts,
         *   - Unscheduled maintenance uses our default strategy.
         *
         * Both of these are affected by the gitconfig though, which may
-        * override specific aspects of our strategy.
+        * override specific aspects of our strategy. Furthermore, both
+        * strategies can be overridden by setting "maintenance.strategy".
         */
        if (opts->schedule) {
-               if (!repo_config_get_string_tmp(the_repository, "maintenance.strategy", &config_str))
-                       strategy = parse_maintenance_strategy(config_str);
-               else
-                       strategy = none_strategy;
+               strategy = none_strategy;
                type = MAINTENANCE_TYPE_SCHEDULED;
        } else {
                strategy = default_strategy;
                type = MAINTENANCE_TYPE_MANUAL;
        }
 
+       if (!repo_config_get_string_tmp(the_repository, "maintenance.strategy", &config_str))
+               strategy = parse_maintenance_strategy(config_str);
+
        for (size_t i = 0; i < TASK__COUNT; i++) {
                int config_value;
 

diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh
index 0fb917dd7b7e35377e637a874b8ed62691325f16..5219bc17a69a0aefb3c3ab292147124b60ffe15e 100755 (executable)
--- a/t/t7900-maintenance.sh
+++ b/t/t7900-maintenance.sh
@@ -886,6 +886,46 @@ test_expect_success 'maintenance.strategy inheritance' '
                <modified-daily.txt
 '
 
+test_strategy () {
+       STRATEGY="$1"
+       shift
+
+       cat >expect &&
+       rm -f trace2.txt &&
+       GIT_TRACE2_EVENT="$(pwd)/trace2.txt" \
+               git -c maintenance.strategy=$STRATEGY maintenance run --quiet "$@" &&
+       sed -n 's/{"event":"child_start","sid":"[^/"]*",.*,"argv":\["\(.*\)\"]}/\1/p' <trace2.txt |
+               sed 's/","/ /g'  >actual
+       test_cmp expect actual
+}
+
+test_expect_success 'maintenance.strategy is respected' '
+       test_when_finished "rm -rf repo" &&
+       git init repo &&
+       (
+               cd repo &&
+               test_commit initial &&
+
+               test_must_fail git -c maintenance.strategy=unknown maintenance run 2>err &&
+               test_grep "unknown maintenance strategy: .unknown." err &&
+
+               test_strategy incremental <<-\EOF &&
+               git pack-refs --all --prune
+               git reflog expire --all
+               git gc --quiet --no-detach --skip-foreground-tasks
+               EOF
+
+               test_strategy incremental --schedule=weekly <<-\EOF
+               git pack-refs --all --prune
+               git prune-packed --quiet
+               git multi-pack-index write --no-progress
+               git multi-pack-index expire --no-progress
+               git multi-pack-index repack --no-progress --batch-size=1
+               git commit-graph write --split --reachable --no-progress
+               EOF
+       )
+'
+
 test_expect_success 'register and unregister' '
        test_when_finished git config --global --unset-all maintenance.repo &&