scalar: 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

gitgitgadget

From: Derrick Stolee <[email protected]>

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.

---
 Documentation/scalar.adoc |  8 +++++++-
 scalar.c                  | 17 ++++++++++-------
 t/t9210-scalar.sh         |  7 +++++++
 3 files changed, 24 insertions(+), 8 deletions(-)

index 7e4259c6743f..b2b244a86499 100644
@@ -11,7 +11,7 @@ SYNOPSIS
 scalar clone [--single-branch] [--branch <main-branch>] [--full-clone]
 	[--[no-]src] <url> [<enlistment>]
 scalar list
-scalar register [<enlistment>]
+scalar register [--[no-]maintenance] [<enlistment>]
 scalar unregister [<enlistment>]
 scalar run ( all | config | commit-graph | fetch | loose-objects | pack-files ) [<enlistment>]
 scalar reconfigure [ --all | <enlistment> ]

@@ -117,6 +117,12 @@ Note: when this subcommand is called in a worktree that is called `src/`, its
 parent directory is considered to be the Scalar enlistment. If the worktree is
 _not_ called `src/`, it itself will be considered to be the Scalar enlistment.
 
+--[no-]maintenance::
+	By default, `scalar register` configures the enlistment to use Git's
+	background maintenance feature. Use the `--no-maintenance` to skip
+	this configuration. This does not disable any maintenance that may
+	already be enabled in other ways.
+
 Unregister
 ~~~~~~~~~~
 

index d359f08bb8e2..2a21fd55f39b 100644
@@ -259,7 +259,7 @@ 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"));

@@ -267,7 +267,7 @@ 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()) {

@@ -550,7 +550,7 @@ 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);

@@ -597,11 +597,14 @@ static int cmd_list(int argc, const char **argv UNUSED)
 
 static int cmd_register(int argc, const char **argv)
 {
+	int maintenance = 1;
 	struct option options[] = {
+		OPT_BOOL(0, "maintenance", &maintenance,
+			 N_("specify if background maintenance should be enabled")),
 		OPT_END(),
 	};
 	const char * const usage[] = {
-		N_("scalar register [<enlistment>]"),
+		N_("scalar register [--[no-]maintenance] [<enlistment>]"),
 		NULL
 	};
 

@@ -610,7 +613,7 @@ static int cmd_register(int argc, const char **argv)
 
 	setup_enlistment_directory(argc, argv, usage, options, NULL);
 
-	return register_dir();
+	return register_dir(maintenance);
 }
 
 static int get_scalar_repos(const char *key, const char *value,

@@ -803,13 +806,13 @@ 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",

index a81662713eb8..a488f72de9fe 100755
@@ -129,6 +129,13 @@ test_expect_success 'scalar unregister' '
 	scalar unregister vanish
 '
 
+test_expect_success 'scalar register --no-maintenance' '
+	git init register-no-maint &&
+	GIT_TEST_MAINT_SCHEDULER="crontab:false,launchctl:false,schtasks:false" \
+		scalar register --no-maintenance register-no-maint 2>err &&
+	test_must_be_empty err
+'
+
 test_expect_success 'set up repository to clone' '
 	test_commit first &&
 	test_commit second &&

gitgitgadget

From: Derrick Stolee <[email protected]>

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'.

---
 Documentation/scalar.adoc | 7 ++++++-
 scalar.c                  | 8 +++++---
 t/t9211-scalar-clone.sh   | 6 ++++++
 3 files changed, 17 insertions(+), 4 deletions(-)

index b2b244a86499..7753df3b4352 100644
@@ -9,7 +9,7 @@ SYNOPSIS
 --------
 [verse]
 scalar clone [--single-branch] [--branch <main-branch>] [--full-clone]
-	[--[no-]src] <url> [<enlistment>]
+	[--[no-]src] [--[no-]tags] [--[no-]maintenance] <url> [<enlistment>]
 scalar list
 scalar register [--[no-]maintenance] [<enlistment>]
 scalar unregister [<enlistment>]

@@ -97,6 +97,11 @@ cloning. If the HEAD at the remote did not point at any branch when
 	A sparse-checkout is initialized by default. This behavior can be
 	turned off via `--full-clone`.
 
+--[no-]maintenance::
+	By default, `scalar clone` configures the enlistment to use Git's
+	background maintenance feature. Use the `--no-maintenance` to skip
+	this configuration.
+
 List
 ~~~~
 

index 2a21fd55f39b..90ea4da11b40 100644
@@ -411,7 +411,7 @@ static int cmd_clone(int argc, const char **argv)
 	const char *branch = NULL;
 	char *branch_to_free = NULL;
 	int full_clone = 0, single_branch = 0, show_progress = isatty(2);
-	int src = 1, tags = 1;
+	int src = 1, tags = 1, maintenance = 1;
 	struct option clone_options[] = {
 		OPT_STRING('b', "branch", &branch, N_("<branch>"),
 			   N_("branch to checkout after clone")),

@@ -424,11 +424,13 @@ static int cmd_clone(int argc, const char **argv)
 			 N_("create repository within 'src' directory")),
 		OPT_BOOL(0, "tags", &tags,
 			 N_("specify if tags should be fetched during clone")),
+		OPT_BOOL(0, "maintenance", &maintenance,
+			 N_("specify if background maintenance should be enabled")),
 		OPT_END(),
 	};
 	const char * const clone_usage[] = {
 		N_("scalar clone [--single-branch] [--branch <main-branch>] [--full-clone]\n"
-		   "\t[--[no-]src] [--[no-]tags] <url> [<enlistment>]"),
+		   "\t[--[no-]src] [--[no-]tags] [--[no-]maintenance] <url> [<enlistment>]"),
 		NULL
 	};
 	const char *url;

@@ -550,7 +552,7 @@ static int cmd_clone(int argc, const char **argv)
 	if (res)
 		goto cleanup;
 
-	res = register_dir(1);
+	res = register_dir(maintenance);
 
 cleanup:
 	free(branch_to_free);

index 01f71910f533..b9c130db6056 100755
@@ -180,6 +180,12 @@ test_expect_success 'scalar clone warns when background maintenance fails' '
 	grep "could not turn on maintenance" err
 '
 
+test_expect_success 'scalar clone --no-maintenance' '
+	GIT_TEST_MAINT_SCHEDULER="crontab:false,launchctl:false,schtasks:false" \
+		scalar clone --no-maintenance "file://$(pwd)/to-clone" no-maint 2>err &&
+	! grep "could not turn on maintenance" err
+'
+
 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 &&

gitgitgadget

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.

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

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.

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.

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

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

gitgitgadget

From: Derrick Stolee <[email protected]>

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.

---
 scalar.c | 29 ++++++++++++++++++++++-------
 1 file changed, 22 insertions(+), 7 deletions(-)

index d359f08bb8e2..b20b063471a6 100644
@@ -209,6 +209,12 @@ static int set_recommended_config(int reconfigure)
 	return 0;
 }
 
+/**
+ * Enable or disable the maintenance mode for the current repository:
+ *
+ * * If 'enable' is nonzero, run 'git maintenance start'.
+ * * If 'enable' is zero, run 'git maintenance unregister --force'.
+ */
 static int toggle_maintenance(int enable)
 {
 	return run_git("maintenance",

@@ -259,7 +265,15 @@ static int stop_fsmonitor_daemon(void)
 	return 0;
 }
 
-static int register_dir(void)
+/**
+ * Register the current directory as a Scalar enlistment, and set the
+ * recommended configuration.
+ *
+ * * If 'maintenance' is non-zero, then enable background maintenance.
+ * * If 'maintenance' is zero, then leave background maintenance as it is
+ *   currently configured.
+ */
+static int register_dir(int maintenance)
 {
 	if (add_or_remove_enlistment(1))
 		return error(_("could not add enlistment"));

@@ -267,8 +281,9 @@ static int register_dir(void)
 	if (set_recommended_config(0))
 		return error(_("could not set recommended config"));
 
-	if (toggle_maintenance(1))
-		warning(_("could not turn on maintenance"));
+	if (maintenance &&
+	    toggle_maintenance(maintenance))
+		warning(_("could not toggle maintenance"));
 
 	if (have_fsmonitor_support() && start_fsmonitor_daemon()) {
 		return error(_("could not start the FSMonitor daemon"));

@@ -550,7 +565,7 @@ 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);

@@ -610,7 +625,7 @@ static int cmd_register(int argc, const char **argv)
 
 	setup_enlistment_directory(argc, argv, usage, options, NULL);
 
-	return register_dir();
+	return register_dir(1);
 }
 
 static int get_scalar_repos(const char *key, const char *value,

@@ -803,13 +818,13 @@ 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",

gitgitgadget

From: Derrick Stolee <[email protected]>

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.

---
 Documentation/scalar.adoc |  8 +++++++-
 scalar.c                  |  8 ++++++--
 t/t9210-scalar.sh         | 13 ++++++++++++-
 3 files changed, 25 insertions(+), 4 deletions(-)

index 7e4259c6743f..b2b244a86499 100644
@@ -11,7 +11,7 @@ SYNOPSIS
 scalar clone [--single-branch] [--branch <main-branch>] [--full-clone]
 	[--[no-]src] <url> [<enlistment>]
 scalar list
-scalar register [<enlistment>]
+scalar register [--[no-]maintenance] [<enlistment>]
 scalar unregister [<enlistment>]
 scalar run ( all | config | commit-graph | fetch | loose-objects | pack-files ) [<enlistment>]
 scalar reconfigure [ --all | <enlistment> ]

@@ -117,6 +117,12 @@ Note: when this subcommand is called in a worktree that is called `src/`, its
 parent directory is considered to be the Scalar enlistment. If the worktree is
 _not_ called `src/`, it itself will be considered to be the Scalar enlistment.
 
+--[no-]maintenance::
+	By default, `scalar register` configures the enlistment to use Git's
+	background maintenance feature. Use the `--no-maintenance` to skip
+	this configuration. This does not disable any maintenance that may
+	already be enabled in other ways.
+
 Unregister
 ~~~~~~~~~~
 

index b20b063471a6..da0c46bc96cc 100644
@@ -612,11 +612,14 @@ static int cmd_list(int argc, const char **argv UNUSED)
 
 static int cmd_register(int argc, const char **argv)
 {
+	int maintenance = 1;
 	struct option options[] = {
+		OPT_BOOL(0, "maintenance", &maintenance,
+			 N_("specify if background maintenance should be enabled")),
 		OPT_END(),
 	};
 	const char * const usage[] = {
-		N_("scalar register [<enlistment>]"),
+		N_("scalar register [--[no-]maintenance] [<enlistment>]"),
 		NULL
 	};
 

@@ -625,7 +628,8 @@ static int cmd_register(int argc, const char **argv)
 
 	setup_enlistment_directory(argc, argv, usage, options, NULL);
 
-	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,

index a81662713eb8..89a6a2a24d8b 100755
@@ -108,7 +108,7 @@ 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' '

@@ -129,6 +129,17 @@ 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_subcommand ! git maintenance unregister --force <no-maint.event
+'
+
 test_expect_success 'set up repository to clone' '
 	test_commit first &&
 	test_commit second &&

gitgitgadget

From: Derrick Stolee <[email protected]>

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'.

---
 Documentation/scalar.adoc |  7 ++++++-
 scalar.c                  |  9 ++++++---
 t/t9211-scalar-clone.sh   | 11 ++++++++++-
 3 files changed, 22 insertions(+), 5 deletions(-)

index b2b244a86499..7753df3b4352 100644
@@ -9,7 +9,7 @@ SYNOPSIS
 --------
 [verse]
 scalar clone [--single-branch] [--branch <main-branch>] [--full-clone]
-	[--[no-]src] <url> [<enlistment>]
+	[--[no-]src] [--[no-]tags] [--[no-]maintenance] <url> [<enlistment>]
 scalar list
 scalar register [--[no-]maintenance] [<enlistment>]
 scalar unregister [<enlistment>]

@@ -97,6 +97,11 @@ cloning. If the HEAD at the remote did not point at any branch when
 	A sparse-checkout is initialized by default. This behavior can be
 	turned off via `--full-clone`.
 
+--[no-]maintenance::
+	By default, `scalar clone` configures the enlistment to use Git's
+	background maintenance feature. Use the `--no-maintenance` to skip
+	this configuration.
+
 List
 ~~~~
 

index da0c46bc96cc..dd6e1447e086 100644
@@ -426,7 +426,7 @@ static int cmd_clone(int argc, const char **argv)
 	const char *branch = NULL;
 	char *branch_to_free = NULL;
 	int full_clone = 0, single_branch = 0, show_progress = isatty(2);
-	int src = 1, tags = 1;
+	int src = 1, tags = 1, maintenance = 1;
 	struct option clone_options[] = {
 		OPT_STRING('b', "branch", &branch, N_("<branch>"),
 			   N_("branch to checkout after clone")),

@@ -439,11 +439,13 @@ static int cmd_clone(int argc, const char **argv)
 			 N_("create repository within 'src' directory")),
 		OPT_BOOL(0, "tags", &tags,
 			 N_("specify if tags should be fetched during clone")),
+		OPT_BOOL(0, "maintenance", &maintenance,
+			 N_("specify if background maintenance should be enabled")),
 		OPT_END(),
 	};
 	const char * const clone_usage[] = {
 		N_("scalar clone [--single-branch] [--branch <main-branch>] [--full-clone]\n"
-		   "\t[--[no-]src] [--[no-]tags] <url> [<enlistment>]"),
+		   "\t[--[no-]src] [--[no-]tags] [--[no-]maintenance] <url> [<enlistment>]"),
 		NULL
 	};
 	const char *url;

@@ -565,7 +567,8 @@ static int cmd_clone(int argc, const char **argv)
 	if (res)
 		goto cleanup;
 
-	res = register_dir(1);
+	/* If --no-maintenance, then skip maintenance command entirely. */
+	res = register_dir(maintenance);
 
 cleanup:
 	free(branch_to_free);

index 01f71910f533..bfbf22a46218 100755
@@ -177,7 +177,16 @@ 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 toggle maintenance" err &&
+	test_subcommand ! git maintenance unregister --force <no-maint.event
 '
 
 test_expect_success '`scalar clone --no-src`' '

gitgitgadget

From: Derrick Stolee <[email protected]>

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.

---
 Documentation/scalar.adoc | 14 +++++++++++---
 scalar.c                  |  9 ++++++---
 t/t9210-scalar.sh         |  7 ++++++-
 3 files changed, 23 insertions(+), 7 deletions(-)

index 7753df3b4352..2868b01988e4 100644
@@ -14,7 +14,7 @@ scalar list
 scalar register [--[no-]maintenance] [<enlistment>]
 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 diagnose [<enlistment>]
 scalar delete <enlistment>
 

@@ -160,8 +160,16 @@ After a Scalar upgrade, or when the configuration of a Scalar enlistment
 was somehow corrupted or changed by mistake, this subcommand allows to
 reconfigure the enlistment.
 
-With the `--all` option, all enlistments currently registered with Scalar
-will be reconfigured. Use this option after each Scalar upgrade.
+--all::
+	When `--all` is specified, reconfigure all enlistments currently
+	registered with Scalar by the `scalar.repo` config key. Use this
+	option after each upgrade to get the latest features.
+
+--[no-]maintenance::
+	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.
 
 Diagnose
 ~~~~~~~~

index dd6e1447e086..0b8a63f6e5a6 100644
@@ -667,14 +667,16 @@ 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;
 	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_END(),
 	};
 	const char * const usage[] = {
-		N_("scalar reconfigure [--all | <enlistment>]"),
+		N_("scalar reconfigure [--[no-]maintenance] [--all | <enlistment>]"),
 		NULL
 	};
 	struct string_list scalar_repos = STRING_LIST_INIT_DUP;

@@ -758,7 +760,8 @@ 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)
 			succeeded = 1;
 
 loop_end:

index 89a6a2a24d8b..34765a49fd07 100755
@@ -210,7 +210,12 @@ test_expect_success 'scalar reconfigure' '
 	GIT_TRACE2_EVENT="$(pwd)/reconfigure" scalar reconfigure -a &&
 	test_path_is_file one/src/cron.txt &&
 	test true = "$(git -C one/src config core.preloadIndex)" &&
-	test_subcommand git maintenance start <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
 '
 
 test_expect_success 'scalar reconfigure --all with includeIf.onbranch' '

gitgitgadget

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

	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;

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

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 <[email protected]>
      
       ## 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 <[email protected]>
          Signed-off-by: Derrick Stolee <[email protected]>
      
       ## 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' '

gitgitgadget

From: Derrick Stolee <[email protected]>

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.

---
 scalar.c | 29 ++++++++++++++++++++++-------
 1 file changed, 22 insertions(+), 7 deletions(-)

index d359f08bb8e2..b20b063471a6 100644
@@ -209,6 +209,12 @@ static int set_recommended_config(int reconfigure)
 	return 0;
 }
 
+/**
+ * Enable or disable the maintenance mode for the current repository:
+ *
+ * * If 'enable' is nonzero, run 'git maintenance start'.
+ * * If 'enable' is zero, run 'git maintenance unregister --force'.
+ */
 static int toggle_maintenance(int enable)
 {
 	return run_git("maintenance",

@@ -259,7 +265,15 @@ static int stop_fsmonitor_daemon(void)
 	return 0;
 }
 
-static int register_dir(void)
+/**
+ * Register the current directory as a Scalar enlistment, and set the
+ * recommended configuration.
+ *
+ * * If 'maintenance' is non-zero, then enable background maintenance.
+ * * If 'maintenance' is zero, then leave background maintenance as it is
+ *   currently configured.
+ */
+static int register_dir(int maintenance)
 {
 	if (add_or_remove_enlistment(1))
 		return error(_("could not add enlistment"));

@@ -267,8 +281,9 @@ static int register_dir(void)
 	if (set_recommended_config(0))
 		return error(_("could not set recommended config"));
 
-	if (toggle_maintenance(1))
-		warning(_("could not turn on maintenance"));
+	if (maintenance &&
+	    toggle_maintenance(maintenance))
+		warning(_("could not toggle maintenance"));
 
 	if (have_fsmonitor_support() && start_fsmonitor_daemon()) {
 		return error(_("could not start the FSMonitor daemon"));

@@ -550,7 +565,7 @@ 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);

@@ -610,7 +625,7 @@ static int cmd_register(int argc, const char **argv)
 
 	setup_enlistment_directory(argc, argv, usage, options, NULL);
 
-	return register_dir();
+	return register_dir(1);
 }
 
 static int get_scalar_repos(const char *key, const char *value,

@@ -803,13 +818,13 @@ 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",

gitgitgadget

From: Derrick Stolee <[email protected]>

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.

---
 Documentation/scalar.adoc |  8 +++++++-
 scalar.c                  |  8 ++++++--
 t/t9210-scalar.sh         | 13 ++++++++++++-
 3 files changed, 25 insertions(+), 4 deletions(-)

index 7e4259c6743f..b2b244a86499 100644
@@ -11,7 +11,7 @@ SYNOPSIS
 scalar clone [--single-branch] [--branch <main-branch>] [--full-clone]
 	[--[no-]src] <url> [<enlistment>]
 scalar list
-scalar register [<enlistment>]
+scalar register [--[no-]maintenance] [<enlistment>]
 scalar unregister [<enlistment>]
 scalar run ( all | config | commit-graph | fetch | loose-objects | pack-files ) [<enlistment>]
 scalar reconfigure [ --all | <enlistment> ]

@@ -117,6 +117,12 @@ Note: when this subcommand is called in a worktree that is called `src/`, its
 parent directory is considered to be the Scalar enlistment. If the worktree is
 _not_ called `src/`, it itself will be considered to be the Scalar enlistment.
 
+--[no-]maintenance::
+	By default, `scalar register` configures the enlistment to use Git's
+	background maintenance feature. Use the `--no-maintenance` to skip
+	this configuration. This does not disable any maintenance that may
+	already be enabled in other ways.
+
 Unregister
 ~~~~~~~~~~
 

index b20b063471a6..da0c46bc96cc 100644
@@ -612,11 +612,14 @@ static int cmd_list(int argc, const char **argv UNUSED)
 
 static int cmd_register(int argc, const char **argv)
 {
+	int maintenance = 1;
 	struct option options[] = {
+		OPT_BOOL(0, "maintenance", &maintenance,
+			 N_("specify if background maintenance should be enabled")),
 		OPT_END(),
 	};
 	const char * const usage[] = {
-		N_("scalar register [<enlistment>]"),
+		N_("scalar register [--[no-]maintenance] [<enlistment>]"),
 		NULL
 	};
 

@@ -625,7 +628,8 @@ static int cmd_register(int argc, const char **argv)
 
 	setup_enlistment_directory(argc, argv, usage, options, NULL);
 
-	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,

index a81662713eb8..89a6a2a24d8b 100755
@@ -108,7 +108,7 @@ 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' '

@@ -129,6 +129,17 @@ 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_subcommand ! git maintenance unregister --force <no-maint.event
+'
+
 test_expect_success 'set up repository to clone' '
 	test_commit first &&
 	test_commit second &&

gitgitgadget

From: Derrick Stolee <[email protected]>

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'.

---
 Documentation/scalar.adoc |  7 ++++++-
 scalar.c                  |  9 ++++++---
 t/t9211-scalar-clone.sh   | 11 ++++++++++-
 3 files changed, 22 insertions(+), 5 deletions(-)

index b2b244a86499..7753df3b4352 100644
@@ -9,7 +9,7 @@ SYNOPSIS
 --------
 [verse]
 scalar clone [--single-branch] [--branch <main-branch>] [--full-clone]
-	[--[no-]src] <url> [<enlistment>]
+	[--[no-]src] [--[no-]tags] [--[no-]maintenance] <url> [<enlistment>]
 scalar list
 scalar register [--[no-]maintenance] [<enlistment>]
 scalar unregister [<enlistment>]

@@ -97,6 +97,11 @@ cloning. If the HEAD at the remote did not point at any branch when
 	A sparse-checkout is initialized by default. This behavior can be
 	turned off via `--full-clone`.
 
+--[no-]maintenance::
+	By default, `scalar clone` configures the enlistment to use Git's
+	background maintenance feature. Use the `--no-maintenance` to skip
+	this configuration.
+
 List
 ~~~~
 

index da0c46bc96cc..dd6e1447e086 100644
@@ -426,7 +426,7 @@ static int cmd_clone(int argc, const char **argv)
 	const char *branch = NULL;
 	char *branch_to_free = NULL;
 	int full_clone = 0, single_branch = 0, show_progress = isatty(2);
-	int src = 1, tags = 1;
+	int src = 1, tags = 1, maintenance = 1;
 	struct option clone_options[] = {
 		OPT_STRING('b', "branch", &branch, N_("<branch>"),
 			   N_("branch to checkout after clone")),

@@ -439,11 +439,13 @@ static int cmd_clone(int argc, const char **argv)
 			 N_("create repository within 'src' directory")),
 		OPT_BOOL(0, "tags", &tags,
 			 N_("specify if tags should be fetched during clone")),
+		OPT_BOOL(0, "maintenance", &maintenance,
+			 N_("specify if background maintenance should be enabled")),
 		OPT_END(),
 	};
 	const char * const clone_usage[] = {
 		N_("scalar clone [--single-branch] [--branch <main-branch>] [--full-clone]\n"
-		   "\t[--[no-]src] [--[no-]tags] <url> [<enlistment>]"),
+		   "\t[--[no-]src] [--[no-]tags] [--[no-]maintenance] <url> [<enlistment>]"),
 		NULL
 	};
 	const char *url;

@@ -565,7 +567,8 @@ static int cmd_clone(int argc, const char **argv)
 	if (res)
 		goto cleanup;
 
-	res = register_dir(1);
+	/* If --no-maintenance, then skip maintenance command entirely. */
+	res = register_dir(maintenance);
 
 cleanup:
 	free(branch_to_free);

index 01f71910f533..bfbf22a46218 100755
@@ -177,7 +177,16 @@ 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 toggle maintenance" err &&
+	test_subcommand ! git maintenance unregister --force <no-maint.event
 '
 
 test_expect_success '`scalar clone --no-src`' '

gitgitgadget

From: Derrick Stolee <[email protected]>

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.

---
 Documentation/scalar.adoc | 17 ++++++++++++++---
 scalar.c                  | 23 +++++++++++++++++++++--
 t/t9210-scalar.sh         | 13 ++++++++++++-
 3 files changed, 47 insertions(+), 6 deletions(-)

index 7753df3b4352..387527be1ea5 100644
@@ -14,7 +14,7 @@ scalar list
 scalar register [--[no-]maintenance] [<enlistment>]
 scalar unregister [<enlistment>]
 scalar run ( all | config | commit-graph | fetch | loose-objects | pack-files ) [<enlistment>]
-scalar reconfigure [ --all | <enlistment> ]
+scalar reconfigure [--maintenance=<mode>] [ --all | <enlistment> ]
 scalar diagnose [<enlistment>]
 scalar delete <enlistment>
 

@@ -160,8 +160,19 @@ After a Scalar upgrade, or when the configuration of a Scalar enlistment
 was somehow corrupted or changed by mistake, this subcommand allows to
 reconfigure the enlistment.
 
-With the `--all` option, all enlistments currently registered with Scalar
-will be reconfigured. Use this option after each Scalar upgrade.
+--all::
+	When `--all` is specified, reconfigure all enlistments currently
+	registered with Scalar by the `scalar.repo` config key. Use this
+	option after each upgrade to get the latest features.
+
+--maintenance=<mode>::
+	By default, Scalar configures the enlistment to use Git's
+	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
 ~~~~~~~~

index dd6e1447e086..847d2dd2f58a 100644
@@ -668,13 +668,19 @@ static int remove_deleted_enlistment(struct strbuf *path)
 static int cmd_reconfigure(int argc, const char **argv)
 {
 	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_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 [--maintenance=<mode>] [--all | <enlistment>]"),
 		NULL
 	};
 	struct string_list scalar_repos = STRING_LIST_INIT_DUP;

@@ -694,6 +700,18 @@ 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++) {

@@ -758,7 +776,8 @@ static int cmd_reconfigure(int argc, const char **argv)
 		the_repository = old_repo;
 		repo_clear(&r);
 
-		if (toggle_maintenance(1) >= 0)
+		if (maintenance >= 0 &&
+		    toggle_maintenance(maintenance) >= 0)
 			succeeded = 1;
 
 loop_end:

index 89a6a2a24d8b..bd6f0c40d229 100755
@@ -210,7 +210,18 @@ test_expect_success 'scalar reconfigure' '
 	GIT_TRACE2_EVENT="$(pwd)/reconfigure" scalar reconfigure -a &&
 	test_path_is_file one/src/cron.txt &&
 	test true = "$(git -C one/src config core.preloadIndex)" &&
-	test_subcommand git maintenance start <reconfigure
+	test_subcommand git maintenance start <reconfigure &&
+	test_subcommand ! git maintenance unregister --force <reconfigure &&
+
+	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' '

gitgitgadget

Hmph, this is a bit unexpected.

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

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.

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

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 <[email protected]>
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.

---

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(-)

index 387527be1ea..4bd5b150e8e 100644
@@ -14,7 +14,7 @@ scalar list
 scalar register [--[no-]maintenance] [<enlistment>]
 scalar unregister [<enlistment>]
 scalar run ( all | config | commit-graph | fetch | loose-objects | pack-files 

) [<enlistment>]
-scalar reconfigure [--maintenance=<mode>] [ --all | <enlistment> ] +scalar reconfigure [--maintenance=(enable|disable|keep)] [ --all | <enlistment> ]
scalar diagnose [<enlistment>]
scalar delete <enlistment>

@@ -165,14 +165,13 @@ reconfigure the enlistment.
registered with Scalar by the scalar.repo config key. Use this option after each upgrade to get the latest features.

---maintenance=<mode>::
+--maintenance=(enable|disable|keep)::
By default, Scalar configures the enlistment to use Git's 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.
+ enable value for this option. Use the disable value to
+ remove each considered enlistment from background maintenance.
+ Use `keep' to leave the background maintenance configuration
+ untouched for these repositories.

Diagnose
~~~~~~~~

index 847d2dd2f58..355baf75e49 100644
@@ -675,12 +675,12 @@ static int cmd_reconfigure(int argc, const char **argv)
 		OPT_BOOL('a', "all", &all,
 			 N_("reconfigure all registered enlistments")),
 		OPT_STRING(0, "maintenance", &maintenance_str,
-			 N_("<mode>"),
+			 N_("(enable|disable|keep)"),
 			 N_("signal how to adjust background maintenance")),
 		OPT_END(),
 	};
 	const char * const usage[] = {
-		N_("scalar reconfigure [--maintenance=<mode>] [--all | <enlistment>]"),
+		N_("scalar reconfigure [--maintenance=(enable|disable|keep)] [--all | <enlistment>]"),
 		NULL
 	};
 	struct string_list scalar_repos = STRING_LIST_INIT_DUP;

2.47.2.vfs.0.2

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.

Thank you for fixing up this whitespace issue.

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

Thanks,
-Stolee