[PATCH v3 0/7] builtin/maintenance: fix auto-detach with non-standard tasks

Patrick Steinhardt
2024-08-16T10:44:59Z

Hi,

this is the second version of my patch series that fixes how
git-maintenance(1) detaches: instead of letting its child process
git-gc(1) detach, we now optionally ask git-maintenance(1) itself to
detach when running via our auto maintenance mechanism. This fixes
behaviour of git-maintenance(1) when configured to run non-standard
tasks like the "incremental" task.

Changes compared to v2:

- Fix leaking git-gc(1) process in t6500.

- Add missing documentation for `maintenance.autoDetach`.

Thanks!

Patrick

Patrick Steinhardt (7):
config: fix constness of out parameter for `git_config_get_expiry()`
builtin/gc: refactor to read config into structure
builtin/gc: fix leaking config values
builtin/gc: stop processing log file on signal
builtin/gc: add a `--detach` flag
builtin/maintenance: add a `--detach` flag
run-command: fix detaching when running auto maintenance

Documentation/config/gc.txt | 3 +-
Documentation/config/maintenance.txt | 11 +
Documentation/git-gc.txt | 5 +-
builtin/gc.c | 384 +++++++++++++++++----------
config.c | 4 +-
config.h | 2 +-
read-cache.c | 12 +-
run-command.c | 12 +-
t/t5304-prune.sh | 1 +
t/t5616-partial-clone.sh | 6 +-
t/t6500-gc.sh | 45 +++-
t/t7900-maintenance.sh | 82 +++++-
12 files changed, 396 insertions(+), 171 deletions(-)

Range-diff against v2:
1: 040453f27f = 1: 040453f27f config: fix constness of out parameter for `git_config_get_expiry()`
2: ff6aa9d7ba = 2: ff6aa9d7ba builtin/gc: refactor to read config into structure
3: 310e361371 = 3: 310e361371 builtin/gc: fix leaking config values
4: 812c61c9b6 = 4: 812c61c9b6 builtin/gc: stop processing log file on signal
5: ca78d3dc7c ! 5: b934b23889 builtin/gc: add a `--detach` flag
@@ builtin/gc.c: static int maintenance_run(int argc, const char **argv, const char
for (i = 0; i < TASK__COUNT; i++)

## t/t6500-gc.sh ##
+@@ t/t6500-gc.sh: test_expect_success 'gc.maxCruftSize sets appropriate repack options' '
+ test_subcommand $cruft_max_size_opts --max-cruft-size=3145728 <trace2.txt
+ '
+
+-run_and_wait_for_auto_gc () {
++run_and_wait_for_gc () {
+ # We read stdout from gc for the side effect of waiting until the
+ # background gc process exits, closing its fd 9. Furthermore, the
+ # variable assignment from a command substitution preserves the
+ # exit status of the main gc process.
+ # Note: this fd trickery doesn't work on Windows, but there is no
+ # need to, because on Win the auto gc always runs in the foreground.
+- doesnt_matter=$(git gc --auto 9>&1)
++ doesnt_matter=$(git gc "$@" 9>&1)
+ }
+
+ test_expect_success 'background auto gc does not run if gc.log is present and recent but does if it is old' '
+@@ t/t6500-gc.sh: test_expect_success 'background auto gc does not run if gc.log is present and re
+ test-tool chmtime =-345600 .git/gc.log &&
+ git gc --auto &&
+ test_config gc.logexpiry 2.days &&
+- run_and_wait_for_auto_gc &&
++ run_and_wait_for_gc --auto &&
+ ls .git/objects/pack/pack-*.pack >packs &&
+ test_line_count = 1 packs
+ '
@@ t/t6500-gc.sh: test_expect_success 'background auto gc respects lock for all operations' '
+ printf "%d %s" "$shell_pid" "$hostname" >.git/gc.pid &&
+
+ # our gc should exit zero without doing anything
+- run_and_wait_for_auto_gc &&
++ run_and_wait_for_gc --auto &&
+ (ls -1 .git/refs/heads .git/reftable >actual || true) &&
test_cmp expect actual
'

@@ t/t6500-gc.sh: test_expect_success 'background auto gc respects lock for all ope
+ git config gc.autodetach false &&
+ git config gc.auto 2 &&
+
-+ cat >expect <<-EOF &&
-+ Auto packing the repository in background for optimum performance.
-+ See "git help gc" for manual housekeeping.
-+ EOF
-+ GIT_PROGRESS_DELAY=0 git gc --auto --detach 2>actual &&
-+ test_cmp expect actual
++ # Note that we cannot use `test_cmp` here to compare stderr
++ # because it may contain output from `set -x`.
++ run_and_wait_for_gc --auto --detach 2>actual &&
++ test_grep "Auto packing the repository in background for optimum performance." actual
+ )
+'
+
6: 06dbb73425 = 6: 347d0a2002 builtin/maintenance: add a `--detach` flag
7: 6bc170ff05 ! 7: 9befef7c1f run-command: fix detaching when running auto maintenance
@@ Commit message

Signed-off-by: Patrick Steinhardt <ps@pks.im>

+ ## Documentation/config/gc.txt ##
+@@ Documentation/config/gc.txt: use, it'll affect how the auto pack limit works.
+
+ gc.autoDetach::
+ Make `git gc --auto` return immediately and run in the background
+- if the system supports it. Default is true.
++ if the system supports it. Default is true. This config variable acts
++ as a fallback in case `maintenance.autoDetach` is not set.
+
+ gc.bigPackThreshold::
+ If non-zero, all non-cruft packs larger than this limit are kept
+
+ ## Documentation/config/maintenance.txt ##
+@@ Documentation/config/maintenance.txt: maintenance.auto::
+ `git maintenance run --auto` after doing their normal work. Defaults
+ to true.
+
++maintenance.autoDetach::
++ Many Git commands trigger automatic maintenance after they have
++ written data into the repository. This boolean config option
++ controls whether this automatic maintenance shall happen in the
++ foreground or whether the maintenance process shall detach and
++ continue to run in the background.
+++
++If unset, the value of `gc.autoDetach` is used as a fallback. Defaults
++to true if both are unset, meaning that the maintenance process will
++detach.
++
+ maintenance.strategy::
+ This string config option provides a way to specify one of a few
+ recommended schedules for background maintenance. This only affects
+
## builtin/gc.c ##
@@ builtin/gc.c: static int maintenance_task_gc(struct maintenance_run_opts *opts,
strvec_push(&child.args, "--quiet");
--
2.46.0.46.g406f326d27.dirty