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

From: Victoria Dye <vdye@github.com>
Date: Jan 30, 2023, 17:42 UTC
Message-ID: <db04e31d-681f-5809-f51a-37b9c734b45a@github.com>
In-Reply-To: <b63611dc-a889-8900-403a-ec7c42a89705@github.com>

Derrick Stolee wrote:

Show 27 quoted lines

> 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