Re: [PATCH 1/2] scalar register: add --no-maintenance option
- From
Patrick Steinhardt <ps@pks.im>
- Date
- May 2, 2025, 09:15 UTC
- Message-ID
- <aBSNN6Z3Vve6c6Bm@pks.im>
- In-Reply-To
- <4910bacd0524a29186e673a174a47cd46ac95b5e.1746008680.git.gitgitgadget@gmail.com>
On Wed, Apr 30, 2025 at 10:24:39AM +0000, Derrick Stolee via GitGitGadget wrote:
Show 48 quoted lines
> diff --git a/Documentation/scalar.adoc b/Documentation/scalar.adoc
> index 7e4259c6743f..b2b244a86499 100644
> --- a/Documentation/scalar.adoc
> +++ b/Documentation/scalar.adoc
> @@ -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
> ~~~~~~~~~~
>
> diff --git a/scalar.c b/scalar.c
> index d359f08bb8e2..2a21fd55f39b 100644
> --- a/scalar.c
> +++ b/scalar.c
> @@ -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()) {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.
Show 8 quoted lines
> @@ -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")),Maybe s/if/whether/? Might just be me not being a native speaker, though.
Show 15 quoted lines
> diff --git a/t/t9210-scalar.sh b/t/t9210-scalar.sh > index a81662713eb8..a488f72de9fe 100755 > --- a/t/t9210-scalar.sh > +++ b/t/t9210-scalar.sh > @@ -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 > +' > +
We should probably have a test that verifies that we don't deregister maintenance.
Patrick