Re: [PATCH 0/2] scalar: add --no-maintenance option

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

"Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:

> These patches add a new --no-maintenance option to the scalar register and
> scalar clone commands. My motivation is based on setting up Scalar clones in
> automated environments that set up a repo onto a disk image for use later.
> If background maintenance runs during later setup steps, then this
> introduces a variable that is unexpected at minimum and disruptive at worst.
> The disruption comes in if the automation has steps to run git maintenance
> run --task=<X> commands but those commands are blocked due to the
> maintenance.lock file.
>
> Functionally, these leave the default behavior as-is but allow disabling the
> git maintenance start step when users opt-in to this difference. The idea of
> Scalar is to recommend the best practices for a typical user, but allowing
> customization for expert users.

The feature itself I do not have any objections to and I found the
reasoning given above very sound.

With these two patches, we still have an unconditional call to
toggle_maintenance(1) in cmd_reconfigure().  Shoudln't the call be
at least removed, which would mean that reconfiguring would not
change the auto-maintenance states, or made controllable from the
command line of "maintenance reconfigure"?

It somehow looks to me that the real culprit of making the result of
applying two patches still unsatisfactory is the original design to
have the toggle_maintenance() call made from inside register_dir()
in the first place.  Shouldn't a much higher layer entry points like
cmd_register() and cmd_clone() be where the decision is made if
maintenance task should be set up (or not set up) by calling
toggle_maintenance(), leaving the register_dir() responsible only
for "enlist the directory to the system"?

IOW it feels to me that enabling (and now optionally disabling)
maintenance is tied too deeply into the act of enlisting a
directory; if we need to disable maintenance (and a mode to add
enlistment without enabling maintenance), it is a sign that it
shouldn't be a parameter into the register_dir() function that
controls what register_dir() does, and rather it should be done by
letting the caller who calls register_dir() decide to call (or not)
toggle_maintenance().

Thanks.