patch, 2 partsscalar: add --no-maintenance option

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.

Thanks, -Stolee

Derrick Stolee (2):
  scalar register: add --no-maintenance option
  scalar clone: add --no-maintenance option

 Documentation/scalar.adoc | 15 +++++++++++++--
 scalar.c                  | 23 ++++++++++++++---------
 t/t9210-scalar.sh         |  7 +++++++
 t/t9211-scalar-clone.sh   |  6 ++++++
 4 files changed, 40 insertions(+), 11 deletions(-)

base-commit: f65182a99e545d2f2bc22e6c1c2da192133b16a3
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1913%2Fderrickstolee%2Fscalar-no-maintenance-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1913/derrickstolee/scalar-no-maintenance-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1913

From: Derrick Stolee <stolee@gmail.com>

When registering a repository with Scalar to get the latest opinionated configuration, the 'scalar register' command will also set up background maintenance. This is a recommended feature for most user scenarios.

However, this is not always recommended in some scenarios where background modifications may interfere with foreground activities. Specifically, setting up a clone for use in automation may require doing certain maintenance steps in the foreground that could become blocked by concurrent background maintenance operations.

Allow the user to specify --no-maintenance to 'scalar register'. This requires updating the method prototype for register_dir(), so use the default of enabling this value when otherwise specified.

Signed-off-by: Derrick Stolee <stolee@gmail.com>
---
 Documentation/scalar.adoc |  8 +++++++-
 scalar.c                  | 17 ++++++++++-------
 t/t9210-scalar.sh         |  7 +++++++
 3 files changed, 24 insertions(+), 8 deletions(-)

On Wed, Apr 30, 2025 at 10:24:39AM +0000, Derrick Stolee via GitGitGadget wrote:

Isn't this change contrary to what the docs say? `toggle_maintenance(0)` would cause us to execute `git maintenance unregister --force`, which deregisters maintenance for us.

Maybe s/if/whether/? Might just be me not being a native speaker, though.

We should probably have a test that verifies that we don't deregister maintenance.

Patrick

On 5/2/2025 5:15 AM, Patrick Steinhardt wrote:

You are right. I've confirmed this using 'test_subcommand' in my test, so I'll plumb the ability to skip toggle_maintenance() in certain cases. I'll carefully document this in code as well.

Thanks for the careful eye. 

I like your choice here.

Yes, will do. The currently-passing test that confirms the unregister is happening would look like this:

test_expect_success 'scalar register --no-maintenance' '
	git init register-no-maint &&
	event_log="$(pwd)/no-maint.event" &&
	GIT_TEST_MAINT_SCHEDULER="crontab:false,launchctl:false,schtasks:false" \
	GIT_TRACE2_EVENT="$event_log" \
	GIT_TRACE2_EVENT_DEPTH=100 \
		scalar register --no-maintenance register-no-maint 2>err &&
	test_must_be_empty err &&
	test_subcommand git maintenance unregister --force <no-maint.event
'When using test_subcommand, it's really important to verify that this kindof test passes before changing the behavior and turning the test into a
negation, since it's easier to accidentally "pass" a negative test if the
test is malformed.

Thanks, -Stolee

From: Derrick Stolee <stolee@gmail.com>

When creating a new enlistment via 'scalar clone', the default is to set up situations that work for most user scenarios. Background maintenance is one of those highly-recommended options for most users.

However, when using 'scalar clone' to create an enlistment in a different situation, such as prepping a VM image, it may be valuable to disable background maintenance so the manual maintenance steps do not get blocked by concurrent background maintenance activities.

Add a new --no-maintenance option to 'scalar clone'.

Signed-off-by: Derrick Stolee <stolee@gmail.com>
---
 Documentation/scalar.adoc | 7 ++++++-
 scalar.c                  | 8 +++++---
 t/t9211-scalar-clone.sh   | 6 ++++++
 3 files changed, 17 insertions(+), 4 deletions(-)

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

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.

On 4/30/2025 4:28 PM, Junio C Hamano wrote:

I agree that this was a miss on my part. Thanks for the careful eye.
 

I will continue thinking about this while playing with different options for a v2. My gut reaction is that register_dir() is our centralized way to "set up recommended configuration" which includes config options and background maintenance. We're now introducing a way to customize this centralized operation based on decentralized options, hence a new option to the method.

Is the right solution to move the toggle_maintenance() out of register_dir()? If this is the only way we plan to customize the config, then yes. Otherwise, the second or third customization will start to lead to copied logic through these three locations.

Again, I'm mostly thinking out loud here before I go and dig into the code. I'll report back with v2.

Thanks, -Stolee

Derrick Stolee <stolee@gmail.com> writes:

It is mostly philosophical, I think, but I actually think the callers that are allowed to be different is a good thing. The callers can pass different parameters to register_dir(), but the distinction between these different callers would become more subtle and not immediately obvious to readers. With an explicit call to toggle_maintenance() at each callsite, it becomes more obvious to see who sets up the maintenance job and how.

If this is and will stay the only way, I would not care too much either way, but if we are planning to extend, then I would say that it is more important to allow callers to be more explicit.

Besides, you'd need to call toggle_maintenance() to disable it in a caller outside register_dir(), so it is not like you can hide the tweaks from readers of the code and make it appear to be simpler. They need to be aware of what goes on anyway.

Junio C Hamano <gitster@pobox.com> writes:

In other words, I think making each helper function do one thing and one thing well is a good thing, and "register" should focus on registering the new repository to the system.

By the way, it is excellent that the new option honors the positive form (i.e. "clone --maintenance" and "clone --no-maintenance"). I was re-reading the patches and was pleasantly surprised, as I would have forgotten to do so while focusing too much on the shiny new feature of being able to disable if I were doing these patches.

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.

Updates in v2 =============

 * The previous use of toggle_maintenance() in register_dir() would run the
   'git maintenance unregister --force' command. There is a new patch 1 that
   is explicit about whether this should or should not happen and new tests
   are added to verify this behavior in the later patches.
 * A new patch 4 adds the --[no-]maintenance option to scalar reconfigure.

Thanks, -Stolee

Derrick Stolee (4):
  scalar: customize register_dir()'s behavior
  scalar register: add --no-maintenance option
  scalar clone: add --no-maintenance option
  scalar reconfigure: add --no-maintenance option

 Documentation/scalar.adoc | 29 ++++++++++++++++++----
 scalar.c                  | 51 +++++++++++++++++++++++++++++----------
 t/t9210-scalar.sh         | 20 +++++++++++++--
 t/t9211-scalar-clone.sh   | 11 ++++++++-
 4 files changed, 90 insertions(+), 21 deletions(-)

base-commit: f65182a99e545d2f2bc22e6c1c2da192133b16a3
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1913%2Fderrickstolee%2Fscalar-no-maintenance-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1913/derrickstolee/scalar-no-maintenance-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/1913

Range-diff vs v1:

 -:  ----------- > 1:  f3a3cfe3ef1 scalar: customize register_dir()'s behavior
 1:  4910bacd052 ! 2:  1b99a559520 scalar register: add --no-maintenance option
     @@ Documentation/scalar.adoc: Note: when this subcommand is called in a worktree th
       
      
       ## scalar.c ##
     -@@ scalar.c: static int stop_fsmonitor_daemon(void)
     - 	return 0;
     - }
     - 
     --static int register_dir(void)
     -+static int register_dir(int maintenance)
     - {
     - 	if (add_or_remove_enlistment(1))
     - 		return error(_("could not add enlistment"));
     -@@ scalar.c: static int register_dir(void)
     - 	if (set_recommended_config(0))
     - 		return error(_("could not set recommended config"));
     - 
     --	if (toggle_maintenance(1))
     -+	if (toggle_maintenance(maintenance))
     - 		warning(_("could not turn on maintenance"));
     - 
     - 	if (have_fsmonitor_support() && start_fsmonitor_daemon()) {
     -@@ scalar.c: static int cmd_clone(int argc, const char **argv)
     - 	if (res)
     - 		goto cleanup;
     - 
     --	res = register_dir();
     -+	res = register_dir(1);
     - 
     - cleanup:
     - 	free(branch_to_free);
      @@ scalar.c: static int cmd_list(int argc, const char **argv UNUSED)
       
       static int cmd_register(int argc, const char **argv)
     @@ scalar.c: static int cmd_register(int argc, const char **argv)
       
       	setup_enlistment_directory(argc, argv, usage, options, NULL);
       
     --	return register_dir();
     +-	return register_dir(1);
     ++	/* If --no-maintenance, then leave maintenance as-is. */
      +	return register_dir(maintenance);
       }
       
       static int get_scalar_repos(const char *key, const char *value,
     -@@ scalar.c: static int cmd_run(int argc, const char **argv)
     - 	strbuf_release(&buf);
     - 
     - 	if (i == 0)
     --		return register_dir();
     -+		return register_dir(1);
     - 
     - 	if (i > 0)
     - 		return run_git("maintenance", "run",
     - 			       "--task", tasks[i].task, NULL);
     - 
     --	if (register_dir())
     -+	if (register_dir(1))
     - 		return -1;
     - 	for (i = 1; tasks[i].arg; i++)
     - 		if (run_git("maintenance", "run",
      
       ## t/t9210-scalar.sh ##
     +@@ t/t9210-scalar.sh: test_expect_success 'scalar register warns when background maintenance fails' '
     + 	git init register-repo &&
     + 	GIT_TEST_MAINT_SCHEDULER="crontab:false,launchctl:false,schtasks:false" \
     + 		scalar register register-repo 2>err &&
     +-	grep "could not turn on maintenance" err
     ++	grep "could not toggle maintenance" err
     + '
     + 
     + test_expect_success 'scalar unregister' '
      @@ t/t9210-scalar.sh: test_expect_success 'scalar unregister' '
       	scalar unregister vanish
       '
       
      +test_expect_success 'scalar register --no-maintenance' '
      +	git init register-no-maint &&
     ++	event_log="$(pwd)/no-maint.event" &&
      +	GIT_TEST_MAINT_SCHEDULER="crontab:false,launchctl:false,schtasks:false" \
     ++	GIT_TRACE2_EVENT="$event_log" \
     ++	GIT_TRACE2_EVENT_DEPTH=100 \
      +		scalar register --no-maintenance register-no-maint 2>err &&
     -+	test_must_be_empty err
     ++	test_must_be_empty err &&
     ++	test_subcommand ! git maintenance unregister --force <no-maint.event
      +'
      +
       test_expect_success 'set up repository to clone' '
 2:  7ab1914b830 ! 3:  e52b1282d93 scalar clone: add --no-maintenance option
     @@ scalar.c: static int cmd_clone(int argc, const char **argv)
       		goto cleanup;
       
      -	res = register_dir(1);
     ++	/* If --no-maintenance, then skip maintenance command entirely. */
      +	res = register_dir(maintenance);
       
       cleanup:
       	free(branch_to_free);
      
       ## t/t9211-scalar-clone.sh ##
     -@@ t/t9211-scalar-clone.sh: test_expect_success 'scalar clone warns when background maintenance fails' '
     - 	grep "could not turn on maintenance" err
     - '
     - 
     +@@ t/t9211-scalar-clone.sh: test_expect_success 'progress without tty' '
     + test_expect_success 'scalar clone warns when background maintenance fails' '
     + 	GIT_TEST_MAINT_SCHEDULER="crontab:false,launchctl:false,schtasks:false" \
     + 		scalar clone "file://$(pwd)/to-clone" maint-fail 2>err &&
     +-	grep "could not turn on maintenance" err
     ++	grep "could not toggle maintenance" err
     ++'
     ++
      +test_expect_success 'scalar clone --no-maintenance' '
      +	GIT_TEST_MAINT_SCHEDULER="crontab:false,launchctl:false,schtasks:false" \
     ++	GIT_TRACE2_EVENT="$(pwd)/no-maint.event" \
     ++	GIT_TRACE2_EVENT_DEPTH=100 \
      +		scalar clone --no-maintenance "file://$(pwd)/to-clone" no-maint 2>err &&
     -+	! grep "could not turn on maintenance" err
     -+'
     -+
     ++	! grep "could not toggle maintenance" err &&
     ++	test_subcommand ! git maintenance unregister --force <no-maint.event
     + '
     + 
       test_expect_success '`scalar clone --no-src`' '
     - 	scalar clone --src "file://$(pwd)/to-clone" with-src &&
     - 	scalar clone --no-src "file://$(pwd)/to-clone" without-src &&
 -:  ----------- > 4:  6fac9c4c394 scalar reconfigure: add --no-maintenance option

From: Derrick Stolee <stolee@gmail.com>

In advance of adding a --[no-]maintenance option to several 'scalar' subcommands, extend the register_dir() method to include an option for how it should handle background maintenance.

It's important that we understand the context of toggle_maintenance() that will enable _or disable_ maintenance depending on its input value. Add a doc comment with this information.

Similarly, update register_dir() to either enable maintenance or leave it alone.

Signed-off-by: Derrick Stolee <stolee@gmail.com>
---
 scalar.c | 29 ++++++++++++++++++++++-------
 1 file changed, 22 insertions(+), 7 deletions(-)

From: Derrick Stolee <stolee@gmail.com>

When registering a repository with Scalar to get the latest opinionated configuration, the 'scalar register' command will also set up background maintenance. This is a recommended feature for most user scenarios.

However, this is not always recommended in some scenarios where background modifications may interfere with foreground activities. Specifically, setting up a clone for use in automation may require doing certain maintenance steps in the foreground that could become blocked by concurrent background maintenance operations.

Allow the user to specify --no-maintenance to 'scalar register'. This requires updating the method prototype for register_dir(), so use the default of enabling this value when otherwise specified.

Signed-off-by: Derrick Stolee <stolee@gmail.com>
---
 Documentation/scalar.adoc |  8 +++++++-
 scalar.c                  |  8 ++++++--
 t/t9210-scalar.sh         | 13 ++++++++++++-
 3 files changed, 25 insertions(+), 4 deletions(-)

From: Derrick Stolee <stolee@gmail.com>

When creating a new enlistment via 'scalar clone', the default is to set up situations that work for most user scenarios. Background maintenance is one of those highly-recommended options for most users.

However, when using 'scalar clone' to create an enlistment in a different situation, such as prepping a VM image, it may be valuable to disable background maintenance so the manual maintenance steps do not get blocked by concurrent background maintenance activities.

Add a new --no-maintenance option to 'scalar clone'.

Signed-off-by: Derrick Stolee <stolee@gmail.com>
---
 Documentation/scalar.adoc |  7 ++++++-
 scalar.c                  |  9 ++++++---
 t/t9211-scalar-clone.sh   | 11 ++++++++++-
 3 files changed, 22 insertions(+), 5 deletions(-)

From: Derrick Stolee <stolee@gmail.com>

When users want to enable the latest and greatest configuration options recommended by Scalar after a Git upgrade, 'scalar reconfigure --all' is a great option that iterates over all repos in the multi-valued 'scalar.repos' config key.

However, this feature previously forced users to enable background maintenance. In some environments this is not preferred.

Add a new --[no-]maintenance option to 'scalar reconfigure' that avoids running 'git maintenance start' on these enlistments.

Signed-off-by: Derrick Stolee <stolee@gmail.com>
---
 Documentation/scalar.adoc | 14 +++++++++++---
 scalar.c                  |  9 ++++++---
 t/t9210-scalar.sh         |  7 ++++++-
 3 files changed, 23 insertions(+), 7 deletions(-)

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

It makes sense for --maintenance option to be between enable and disable when registering a new directory to the system, and when cloning somebody else's repository that causes a new directory to be created and enlisting the resulting new directory to the system.

But wouldn't users want "leave maintenance-enrollment status alone" option when reconfiguring an existing already enlisted directory?

As written, the design easily allows enabling of maintenance as part of reconfiguring, but disabling cannot be done the same way (i.e. individual enlistments need to be visited and their maintenance disabled manually).

IOW, it is a bit counter-intuitive

that for clone and register, --maintenance means "enable" and "--no-maintenance" means "disable", but when reconfiguring an already registered directory, it would be natural to expect that "--no-maintenance" would explicitly tell the command to disable scheduled maintenance.

A 3-way approach would make this part something like ...

	switch (maintenance) {
	default:	BUG("..."); break;
	case ENABLE:	res = toggle_maintenance(1); break;
	case DISABLE:	res = toggle_maintenance(0); break;
	case ASIS:	res = 0; break;
	}
	if (res >= 0)
		succeeded = 1;

... which would allow people to easily say "leave the existing maintenance state alone".

I dunno.

On 5/5/2025 5:47 PM, Junio C Hamano wrote:

I can see how this command is different from the other two, and thus a three-way flipper can actually result in three different behaviors:

This does mean that we'd need to have a different toggle from the typical OPT_BOOL().

What do you think about something of the form --maintenance=<option> where <option> is one of these:

 * "enable" (default) runs 'git maintenance start'
 * "disable" runs 'git maintenance unregister'
 * "keep" does not mess with maintenance config.

Does this sort of option seem to make sense? I'll wait to see if any further adjustments are recommended before I start rolling a new version.

Thanks, -Stolee

Derrick Stolee <stolee@gmail.com> writes:

I think it makes superb sense.  

Certainly much less ambiguous than "--[no-]maintenance" given that we need to handle "reconfigure". Without the need to deal with "reconfigure", it certainly is attractive if we can treat it as a simple Boolean, though.

It is also tempting to just initialize the internal variable to -1 and keep using OPT_BOOL() though. Then after config and command line parsing is done, clone and register would turn -1 the user did not touch into 1 (i.e. enable is default for these two operations), while reconfigure treats -1 as "leave it as-is". It would make it very cumbersome if we ever change our mind and give a default other than "leave it as-is" to "reconfigure", but other than that minor downside, it may be easier to use from end-user's point of view.

I have no strong opinion between the two.

Thanks.

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.

Updates in v2 =============

 * The previous use of toggle_maintenance() in register_dir() would run the
   'git maintenance unregister --force' command. There is a new patch 1 that
   is explicit about whether this should or should not happen and new tests
   are added to verify this behavior in the later patches.
 * A new patch 4 adds the --[no-]maintenance option to scalar reconfigure.

Updates in v3 =============

 * Patch 4 converts the --[no-]maintenance option of scalar reconfigure to
   --maintenance=<mode> to keep the default behavior the same (enable
   maintenance) but also allow two other modes: disable maintenance and
   leave maintenance as configured.

Thanks, -Stolee

Derrick Stolee (4):
  scalar: customize register_dir()'s behavior
  scalar register: add --no-maintenance option
  scalar clone: add --no-maintenance option
  scalar reconfigure: add --maintenance=<mode> option

 Documentation/scalar.adoc | 32 ++++++++++++++++---
 scalar.c                  | 65 +++++++++++++++++++++++++++++++--------
 t/t9210-scalar.sh         | 26 ++++++++++++++--
 t/t9211-scalar-clone.sh   | 11 ++++++-
 4 files changed, 114 insertions(+), 20 deletions(-)

base-commit: f65182a99e545d2f2bc22e6c1c2da192133b16a3
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1913%2Fderrickstolee%2Fscalar-no-maintenance-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1913/derrickstolee/scalar-no-maintenance-v3
Pull-Request: https://github.com/gitgitgadget/git/pull/1913

Range-diff vs v2:

 1:  f3a3cfe3ef1 = 1:  f3a3cfe3ef1 scalar: customize register_dir()'s behavior
 2:  1b99a559520 = 2:  1b99a559520 scalar register: add --no-maintenance option
 3:  e52b1282d93 = 3:  e52b1282d93 scalar clone: add --no-maintenance option
 4:  6fac9c4c394 ! 4:  684f04aaf7e scalar reconfigure: add --no-maintenance option
     @@ Metadata
      Author: Derrick Stolee <dstolee@microsoft.com>
      
       ## Commit message ##
     -    scalar reconfigure: add --no-maintenance option
     +    scalar reconfigure: add --maintenance=<mode> option
      
          When users want to enable the latest and greatest configuration options
          recommended by Scalar after a Git upgrade, 'scalar reconfigure --all' is
     @@ Commit message
          However, this feature previously forced users to enable background
          maintenance. In some environments this is not preferred.
      
     -    Add a new --[no-]maintenance option to 'scalar reconfigure' that avoids
     -    running 'git maintenance start' on these enlistments.
     +    Add a new --maintenance=<mode> option to 'scalar reconfigure' that
     +    provides options for enabling (default), disabling, or leaving
     +    background maintenance config as-is.
      
     +    Helped-by: Junio C Hamano <gitster@pobox.com>
          Signed-off-by: Derrick Stolee <stolee@gmail.com>
      
       ## Documentation/scalar.adoc ##
     @@ Documentation/scalar.adoc: scalar list
       scalar unregister [<enlistment>]
       scalar run ( all | config | commit-graph | fetch | loose-objects | pack-files ) [<enlistment>]
      -scalar reconfigure [ --all | <enlistment> ]
     -+scalar reconfigure [--[no-]maintenance] [ --all | <enlistment> ]
     ++scalar reconfigure [--maintenance=<mode>] [ --all | <enlistment> ]
       scalar diagnose [<enlistment>]
       scalar delete <enlistment>
       
     @@ Documentation/scalar.adoc: After a Scalar upgrade, or when the configuration of
      +	registered with Scalar by the `scalar.repo` config key. Use this
      +	option after each upgrade to get the latest features.
      +
     -+--[no-]maintenance::
     ++--maintenance=<mode>::
      +	By default, Scalar configures the enlistment to use Git's
     -+	background maintenance feature. Use the `--no-maintenance` to skip
     -+	this configuration and leave the repositories in whatever state is
     -+	currently configured.
     ++	background maintenance feature; this is the same as using the
     ++	`--maintenance=enable` value for this option. Use the
     ++	`--maintenance=disable` to remove each considered enlistment
     ++	from background maintenance. Use `--maitnenance=keep' to leave
     ++	the background maintenance configuration untouched for These
     ++	repositories.
       
       Diagnose
       ~~~~~~~~
      
       ## scalar.c ##
      @@ scalar.c: static int remove_deleted_enlistment(struct strbuf *path)
     - 
       static int cmd_reconfigure(int argc, const char **argv)
       {
     --	int all = 0;
     -+	int all = 0, maintenance = 1;
     + 	int all = 0;
     ++	const char *maintenance_str = NULL;
     ++	int maintenance = 1; /* Enable maintenance by default. */
     ++
       	struct option options[] = {
       		OPT_BOOL('a', "all", &all,
       			 N_("reconfigure all registered enlistments")),
     -+		OPT_BOOL(0, "maintenance", &maintenance,
     -+			 N_("specify if background maintenance should be enabled")),
     ++		OPT_STRING(0, "maintenance", &maintenance_str,
     ++			 N_("<mode>"),
     ++			 N_("signal how to adjust background maintenance")),
       		OPT_END(),
       	};
       	const char * const usage[] = {
      -		N_("scalar reconfigure [--all | <enlistment>]"),
     -+		N_("scalar reconfigure [--[no-]maintenance] [--all | <enlistment>]"),
     ++		N_("scalar reconfigure [--maintenance=<mode>] [--all | <enlistment>]"),
       		NULL
       	};
       	struct string_list scalar_repos = STRING_LIST_INIT_DUP;
     +@@ scalar.c: static int cmd_reconfigure(int argc, const char **argv)
     + 		usage_msg_opt(_("--all or <enlistment>, but not both"),
     + 			      usage, options);
     + 
     ++	if (maintenance_str) {
     ++		if (!strcmp(maintenance_str, "enable"))
     ++			maintenance = 1;
     ++		else if (!strcmp(maintenance_str, "disable"))
     ++			maintenance = 0;
     ++		else if (!strcmp(maintenance_str, "keep"))
     ++			maintenance = -1;
     ++		else
     ++			die(_("unknown mode for --maintenance option: %s"),
     ++			    maintenance_str);
     ++	}
     ++
     + 	git_config(get_scalar_repos, &scalar_repos);
     + 
     + 	for (size_t i = 0; i < scalar_repos.nr; i++) {
      @@ scalar.c: static int cmd_reconfigure(int argc, const char **argv)
       		the_repository = old_repo;
       		repo_clear(&r);
       
      -		if (toggle_maintenance(1) >= 0)
     -+		if (maintenance &&
     -+		    toggle_maintenance(1) >= 0)
     ++		if (maintenance >= 0 &&
     ++		    toggle_maintenance(maintenance) >= 0)
       			succeeded = 1;
       
       loop_end:
     @@ t/t9210-scalar.sh: test_expect_success 'scalar reconfigure' '
      +	test_subcommand git maintenance start <reconfigure &&
      +	test_subcommand ! git maintenance unregister --force <reconfigure &&
      +
     -+	GIT_TRACE2_EVENT="$(pwd)/reconfigure-maint" scalar reconfigure --no-maintenance -a &&
     -+	test_subcommand ! git maintenance start <reconfigure-maint &&
     -+	test_subcommand ! git maintenance unregister --force <reconfigure-maint
     ++	GIT_TRACE2_EVENT="$(pwd)/reconfigure-maint-disable" \
     ++		scalar reconfigure -a --maintenance=disable &&
     ++	test_subcommand ! git maintenance start <reconfigure-maint-disable &&
     ++	test_subcommand git maintenance unregister --force <reconfigure-maint-disable &&
     ++
     ++	GIT_TRACE2_EVENT="$(pwd)/reconfigure-maint-keep" \
     ++		scalar reconfigure --maintenance=keep -a &&
     ++	test_subcommand ! git maintenance start <reconfigure-maint-keep &&
     ++	test_subcommand ! git maintenance unregister --force <reconfigure-maint-keep
       '
       
       test_expect_success 'scalar reconfigure --all with includeIf.onbranch' '

From: Derrick Stolee <stolee@gmail.com>

In advance of adding a --[no-]maintenance option to several 'scalar' subcommands, extend the register_dir() method to include an option for how it should handle background maintenance.

It's important that we understand the context of toggle_maintenance() that will enable _or disable_ maintenance depending on its input value. Add a doc comment with this information.

Similarly, update register_dir() to either enable maintenance or leave it alone.

Signed-off-by: Derrick Stolee <stolee@gmail.com>
---
 scalar.c | 29 ++++++++++++++++++++++-------
 1 file changed, 22 insertions(+), 7 deletions(-)

From: Derrick Stolee <stolee@gmail.com>

When registering a repository with Scalar to get the latest opinionated configuration, the 'scalar register' command will also set up background maintenance. This is a recommended feature for most user scenarios.

However, this is not always recommended in some scenarios where background modifications may interfere with foreground activities. Specifically, setting up a clone for use in automation may require doing certain maintenance steps in the foreground that could become blocked by concurrent background maintenance operations.

Allow the user to specify --no-maintenance to 'scalar register'. This requires updating the method prototype for register_dir(), so use the default of enabling this value when otherwise specified.

Signed-off-by: Derrick Stolee <stolee@gmail.com>
---
 Documentation/scalar.adoc |  8 +++++++-
 scalar.c                  |  8 ++++++--
 t/t9210-scalar.sh         | 13 ++++++++++++-
 3 files changed, 25 insertions(+), 4 deletions(-)

From: Derrick Stolee <stolee@gmail.com>

When creating a new enlistment via 'scalar clone', the default is to set up situations that work for most user scenarios. Background maintenance is one of those highly-recommended options for most users.

However, when using 'scalar clone' to create an enlistment in a different situation, such as prepping a VM image, it may be valuable to disable background maintenance so the manual maintenance steps do not get blocked by concurrent background maintenance activities.

Add a new --no-maintenance option to 'scalar clone'.

Signed-off-by: Derrick Stolee <stolee@gmail.com>
---
 Documentation/scalar.adoc |  7 ++++++-
 scalar.c                  |  9 ++++++---
 t/t9211-scalar-clone.sh   | 11 ++++++++++-
 3 files changed, 22 insertions(+), 5 deletions(-)

From: Derrick Stolee <stolee@gmail.com>

When users want to enable the latest and greatest configuration options recommended by Scalar after a Git upgrade, 'scalar reconfigure --all' is a great option that iterates over all repos in the multi-valued 'scalar.repos' config key.

However, this feature previously forced users to enable background maintenance. In some environments this is not preferred.

Add a new --maintenance=<mode> option to 'scalar reconfigure' that provides options for enabling (default), disabling, or leaving background maintenance config as-is.

Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Derrick Stolee <stolee@gmail.com>
---
 Documentation/scalar.adoc | 17 ++++++++++++++---
 scalar.c                  | 23 +++++++++++++++++++++--
 t/t9210-scalar.sh         | 13 ++++++++++++-
 3 files changed, 47 insertions(+), 6 deletions(-)

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

Hmph, this is a bit unexpected.

If I understand it correctly, here is the only place that the users can learn what the valid choices are, and it is not even an enumeration. They are forced to read the entire paragraph to learn what the choices are.

And there is no hint what are the right <mode> strings are.

So "scalar reconfigure -h" would not tell readers what the right choices are, either.

Those who say "scalar reconfigure --maintenance=yes" gets this message that tells 'yes' is not a known mode, without saying that they meant 'enable'.

The --opt=<mode> interface is good when we expect the vocabulary for <mode> to grow, but I am not sure if it is warranted in this case. Is there a strong reason why 'reconfigure' MUST enable the maintenance by default, even if it were originally disabled in the enlistment? If there isn't, initializing maintenance to -1 and setting it with OPT_BOOL() would make the UI consistent with the register and clone subcommands, and also we can lose the above block to parse out a string. Also the code below ...

... which does make perfect sense, would still be applicable.

I dunno. I just feel that 3-way "mode" interface is too much hassle to get right (e.g., give hints to guide the users who forgot what modes are available and how they are spelled) for this code path.

Anyway, will replace the previous round with this one.

Thanks.

On 5/7/25 5:46 PM, Junio C Hamano wrote:

I suppose this could be fixed by changing the `<mode>` to be of the form "[enable|disable|keep]".

My intention was to bend over backwards to prevent a behavior change in the default case. However, I'm coming around to understand that we don't need this background maintenance to be redone every time and can become a no-op by default. (Other new configuration will still happen.)

In the case where we're fine changing the default behavior, then the standard --[no-]maintenance option will work, though it is a three-way switch where the lack of its existence means "don't do either mode".

I've got a new version of this patch doing what you asked for in the first place.

Thanks, -Stolee

Derrick Stolee <stolee@gmail.com> writes:

Ahh, OK.  I misread your intention.

If it is common for existing users to disable maintenance, perhaps by mistake, together with configuration changes that are not quite right, perhaps also by mistake, and if they used reconfigure to recover from such mistakes, it indeed may make sense to nuke the current setting and enable maintenance unconditionally.

As you suggested in a part of your response I omitted, we can annotate <mode> to give hints on the valid choices to help users, without changing the default behaviour. I am personally fine either way, as long as we clearly document the reasoning behind our design.

Thanks.

On 5/12/2025 1:44 PM, Junio C Hamano wrote:

The original intent of updating background maintenance in the "reconfigure" command was primarily about updating the schedule, if needed.

The recent bug in the macOS scheduler is a good example of this. Users will need to rerun 'git maintenance start' somewhere in order to get the "correct" schedule. This bug only needs this run for any single repo, which makes the 'scalar reconfigure -a' command a somewhat strange place to get the fix.

I'll create a new patch on top of the current series version that does this, calling it out as an intentional pattern. It's previously been used by these examples:

 * --fixup=[(amend|reword):]<commit>
 * --diff-filter=[(A|C|D|M|R|T|U|X|B)...[*]]
 * --tool=[g,n,]vimdiff
 * --exclude-hidden=[fetch|receive|uploadpack]

One place where this kind of notation could be helpful, but appears to be absent, is the '-L(<n>:<m>)|(:<method>):<file>' argument for 'git log' and 'git rev-list'. Perhaps this is too dense, though, so it would be better split into '-L<n>:<m>:<file>' and '-L:<method>:<file>'.

Thanks, -Stolee

Derrick Stolee <stolee@gmail.com> writes:

Yup, these are good things to have in "git cmd -h" to help users jog their memory what the available choices are. We do not have to always verbosely explain what these mean everywhere, of course, but if we said in "git commit -h" something like

    --fixup=<choice>

that would be almost hostile to the users. And in documentation pages, of course we can describe what each of the available choices mean.

Yup.

 From 0f5dc1cb6d697c7d8d3c126f3640c2f58fcfda43 Mon Sep 17 00:00:00 2001
From: Derrick Stolee <stolee@gmail.com>
Date: Wed, 14 May 2025 09:50:32 -0400
Subject: [PATCH 5/4] scalar reconfigure: improve --maintenance docs

The --maintenance option for 'scalar reconfigure' has three possible values. Improve the documentation by specifying the option in the -h help menu and usage information.

Signed-off-by: Derrick Stolee <stolee@gmail.com>
---

Adding this extra patch on top to improve the docs. I could resend as a full v4 if needed.

Thanks, -Stolee

  Documentation/scalar.adoc | 13 ++++++-------
  scalar.c                  |  4 ++--
  2 files changed, 8 insertions(+), 9 deletions(-)

Derrick Stolee <stolee@gmail.com> writes:

Nah, the other four patches have been beaten to death, I think. Please double check the result when I push it out later today, as I've got the following when running "git am".

    warning: Patch sent with format=flowed; space at the end of lines might be lost.
    Applying: scalar reconfigure: improve --maintenance docs

Thanks.

On 5/14/2025 6:16 PM, Junio C Hamano wrote:

Thank you for fixing up this whitespace issue.

I'm happy with your copy of ds/scalar-no-maintenance.

Thanks, -Stolee