Re: [PATCH 3/3] scalar: only warn when background maintenance fails

[Victoria Dye <vdye@github.com>]

Derrick Stolee wrote:
> On 1/27/2023 7:32 PM, Junio C Hamano wrote:
>> Derrick Stolee <derrickstolee@github.com> writes:
>>
>>>>  The "maintain
>>>> their clone" certainly should include running periodic maintenance
>>>> tasks without them having to worry about it.  It feels like this is
>>>> calling for an explicit "disable periodic maintenance tasks in this
>>>> repository" option to help these esoteric environments that disable
>>>> cron-like system services, while keeping the default safer,
>>>> i.e. fail loudly when the periodic maintenance tasks that the users
>>>> expect to happen cannot be enabled, or something.
>>>>
>>>> Perhaps I am not the primary audience, but hmph, I have a feeling
>>>> that this is not exactly going into a healthy direction.
>>>
>>> Here, we are in an environment where background maintenance is
>>> unavailable in an unexpected way. If that feature is not available
>>> to the user, should they not get the benefits of the others?
>>
>> That is not what I was saying.  I just have expected to see a way
>> for the user to give scalar an explicit "I understand that periodic
>> maintenance does not happen in this repository" consent, instead of
>> demoting an error detection for everybody to a warning that users
>> will just ignore.
> 
> Ah, so you'd prefer a --no-maintenance option for users who have
> this problem instead of just a warning. I'll do that in v2.

I mentioned this earlier [1], but I want to reiterate that I really don't
think a dedicated '--no-maintenance' option is a good approach to this
problem. I understand wanting more active user acknowledgement that "I
understand that periodic maintenance does not happen in this repository";
without that, users may (rightfully) be confused when they find their
scalar-cloned repository full of loose objects. But, in the use case you've
presented (where no scheduler is available), the user would need to -
somewhat redundantly, I feel - acknowledge that for *every* repository they
clone. 

I'm also still worried about cluttering scalar's UX with options that toggle
use of its internally-configured options and features. One of the big
selling points for including scalar in the upstream project ([2], [3]) was
its ability to "intelligently" configure all of the settings a user would
need to optimize a large repository *without* a user needing to know what
any of those options are/what they mean. These settings are inherently
subject to change (due to use of experimental features); exposing a feature
toggle entrenches that setting permanently within scalar and makes a user
aware of implementation details that were intended to be hidden. At a high
level, it pushes scalar towards simply being an "opinionated" 'git
config'-configurator, which was a model I explicitly tried to move away from
while upstreaming last year.  

I still believe treating maintenance like FSMonitor - pre-determining
whether the feature is available and only enabling it if possible - is the
most consistent and user-friendly solution to the given problem within the
context of scalar. But, if you feel that user acknowledgement is absolutely
critical, I'd strongly prefer a config setting like 'maintenance.enabled';
it could be set globally (the appropriate scope in the case of a system that
has no scheduler), or with '-c' with Scalar clone if it really needs to be
per-repo.

[1] https://lore.kernel.org/git/3ade6d9f-8477-40c2-d683-d629e863c6ab@github.com/
[2] https://lore.kernel.org/git/pull.1005.git.1630359290.gitgitgadget@gmail.com/
[3] https://lore.kernel.org/git/pull.1275.git.1656521925.gitgitgadget@gmail.com/

> 
> This could be a good time for me to upstream the --no-src option
> while I'm messing with arguments in 'scalar clone'.

For what it's worth, my concerns about option clutter don't really apply to
'--no-src' (cloning directly into a given directory, rather than
'<directory>/src'). Unlike features like FSMonitor and maintenance, the
'src/' directory is a user-facing Scalar design decision. It's also
something that seems to exist primarily for backward-compatibility reasons
(if I'm interpreting your earlier comments [4] correctly). This could be a
step on a deprecation path to make '--no-src' the default and remove the
legacy enlistment structure? At the very least, it's sufficiently outside
scalar's "configure for a large repo" scope for me to not worry about it
setting a bad precedent.

[4] https://lore.kernel.org/git/82716e5b-3522-68f5-7479-1b39811e0cb2@github.com/

> 
> Thanks,
> -Stolee