Subject: [PATCH v3 0/3] scalar: make unregister idempotent

From: "Derrick Stolee via GitGitGadget" <[email-masked]>
Date: Mon, 26 Sep 2022 18:48:03 +0000

I noticed this while we were updating the microsoft/git fork to include
v2.38.0-rc0. I don't think 'git maintenance unregister' was idempotent
before, but instead some change in 'scalar unregister' led to it relying on
the return code of 'git maintenance unregister'. Our functional tests expect
'scalar unregister' to be idempotent, and I think that's a good pattern for
'git maintenance unregister', so I'm fixing it at that layer.

Despite finding this during the 2.38.0-rc0 integration, this isn't critical
to the release.

Perhaps an argument could be made that "failure means it wasn't registered
before", but I think that isn't terribly helpful.

Our functional tests are running the unregister subcommand to disable
maintenance in order to run tests on the object store (such as running
maintenance commands in the foreground and checking the object store
afterwards). This is a form of automation using 'unregister' as a check that
maintenance will not run at the same time, and it doesn't care if
maintenance was already disabled. I can imagine other scripting scenarios
wanting that kind of guarantee.

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

 * The --force option now uses OPT_FORCE and is hidden from autocomplete.
 * A new commit is added that removes the use of Git subprocesses in favor
   of git_config_set_multivar_in_file_gently().

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

 * This is now a two-patch series.
 * I rebased onto v2.38.0-rc1 for two reasons: Scalar is now merged, and the
   usage for 'git maintenance unregister' removed its translation markers.
 * Instead of making git maintenance unregister idempotent, add a --force
   option for those who do not want to require that the repository is
   already registered.
 * Make scalar unregister idempotent, with reasons argued in patch 2.

Thanks, -Stolee

Derrick Stolee (3):
  maintenance: add 'unregister --force'
  scalar: make 'unregister' idempotent
  gc: replace config subprocesses with API calls

 Documentation/git-maintenance.txt |  6 +-
 builtin/gc.c                      | 95 +++++++++++++++++++++----------
 scalar.c                          |  5 +-
 t/t7900-maintenance.sh            |  6 +-
 t/t9210-scalar.sh                 |  5 +-
 5 files changed, 82 insertions(+), 35 deletions(-)

base-commit: 1b3d6e17fe83eb6f79ffbac2f2c61bbf1eaef5f8
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1358%2Fderrickstolee%2Fmaintenance-unregister-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1358/derrickstolee/maintenance-unregister-v3
Pull-Request: https://github.com/gitgitgadget/git/pull/1358

Range-diff vs v2:

 1:  69c74f52eef ! 1:  8a8bffaec89 maintenance: add 'unregister --force'
     @@ builtin/gc.c: done:
       {
      +	int force = 0;
       	struct option options[] = {
     -+		OPT_BOOL(0, "force", &force,
     -+			 N_("return success even if repository was not registered")),
     ++		OPT__FORCE(&force,
     ++			   N_("return success even if repository was not registered"),
     ++			   PARSE_OPT_NOCOMPLETE),
       		OPT_END(),
       	};
      -	int rc;
 2:  f5d8d6e4901 = 2:  06d5ef3fc57 scalar: make 'unregister' idempotent
 -:  ----------- > 3:  260d7bee36e gc: replace config subprocesses with API calls

-- 
gitgitgadget