Re: [PATCH 01/11] odb: run "pre-auto-gc" hook for all maintenance tasks

[Junio C Hamano <gitster@pobox.com>]

Patrick Steinhardt <ps@pks.im> writes:

> While the former makes sense, the latter is somewhat off. While the hook
> is indeed strongly tied to gc'ing a repository, the original intent of
> the hook is rather to inhibit any kind of automated garbage collection.
> That noticeably also includes all the other maintenance tasks that our
> new infrastructure may run, but those aren't getting intercepted at all.

If we want to halt object collection right now for some reason, it
is likely that for the same reason we may want automated pruning of
old reflog entries, for example.  So I can buy the above reasoning.

> diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh
> index d7f82e1bec..1212b306b6 100755
> --- a/t/t7900-maintenance.sh
> +++ b/t/t7900-maintenance.sh
> @@ -740,6 +740,127 @@ test_expect_success 'geometric repacking honors configured split factor' '
>  	)
>  '
>  
> +test_expect_success 'pre-auto-gc hook runs exactly once' '
> +	test_when_finished "rm -rf repo" &&
> +	git init repo &&
> +	(
> +		cd repo &&
> +		write_script .git/hooks/pre-auto-gc <<-\EOF &&
> +		echo hook >>hook.log
> +		EOF
> +
> +		# Satisfy the auto condition for multiple tasks, both in the
> +		# foreground and in the background phase.
> +		git config set maintenance.reflog-expire.auto -1 &&
> +		git config set maintenance.geometric-repack.auto -1 &&
> +		git config set maintenance.rerere-gc.auto -1 &&
> +
> +		GIT_TRACE2_EVENT="$(pwd)/trace2.txt" \
> +			git maintenance run --auto 2>/dev/null &&
> +
> +		# The successful hook does not inhibit any of the tasks...
> +		test_subcommand git reflog expire --all <trace2.txt &&
> +		test_subcommand_flex git repack <trace2.txt &&
> +		test_subcommand git rerere gc <trace2.txt &&
> +		# ... but it must only have been executed a single time.
> +		test_line_count = 1 hook.log
> +	)
> +'

Somehow I'd feel better if the hook used a full path to the append
only log file, but it is reasonably clear that these three commands
are unlikely to chdir around, so it may be OK.

Obviously not in scope of this topic, but I wonder if we have a
better way to test these three "housekeeping tasks" have run, than
casting in stone the current implementation that spawns these three
external command as subprocesses.