diff mbox series

[v4,11/15] scalar: allow reconfiguring an existing enlistment

Message ID ada242c7c8c1f530231f97e76c97fff18e368ff5.1631630356.git.gitgitgadget@gmail.com (mailing list archive)
State New, archived
Headers show
Series Upstreaming the Scalar command | expand

Commit Message

Johannes Schindelin Sept. 14, 2021, 2:39 p.m. UTC
From: Johannes Schindelin <johannes.schindelin@gmx.de>

This comes in handy during Scalar upgrades, or when config settings were
messed up by mistake.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 contrib/scalar/scalar.c          | 79 +++++++++++++++++++++-----------
 contrib/scalar/scalar.txt        |  8 ++++
 contrib/scalar/t/t9099-scalar.sh |  8 ++++
 3 files changed, 67 insertions(+), 28 deletions(-)

Comments

Elijah Newren Sept. 28, 2021, 5:24 a.m. UTC | #1
On Tue, Sep 14, 2021 at 7:39 AM Johannes Schindelin via GitGitGadget
<gitgitgadget@gmail.com> wrote:
>
> From: Johannes Schindelin <johannes.schindelin@gmx.de>
>
> This comes in handy during Scalar upgrades, or when config settings were
> messed up by mistake.
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>  contrib/scalar/scalar.c          | 79 +++++++++++++++++++++-----------
>  contrib/scalar/scalar.txt        |  8 ++++
>  contrib/scalar/t/t9099-scalar.sh |  8 ++++
>  3 files changed, 67 insertions(+), 28 deletions(-)
>
> diff --git a/contrib/scalar/scalar.c b/contrib/scalar/scalar.c
> index 8a11f390251..1fff7eb7c12 100644
> --- a/contrib/scalar/scalar.c
> +++ b/contrib/scalar/scalar.c
> @@ -115,18 +115,20 @@ static int run_git(const char *arg, ...)
>         return res;
>  }
>
> -static int set_recommended_config(void)
> +static int set_recommended_config(int reconfigure)
>  {
>         struct {
>                 const char *key;
>                 const char *value;
> +               int overwrite_on_reconfigure;
>         } config[] = {
> -               { "am.keepCR", "true" },
> -               { "core.FSCache", "true" },
> -               { "core.multiPackIndex", "true" },
> -               { "core.preloadIndex", "true" },
> +               /* Required */
> +               { "am.keepCR", "true", 1 },
> +               { "core.FSCache", "true", 1 },
> +               { "core.multiPackIndex", "true", 1 },
> +               { "core.preloadIndex", "true", 1 },
>  #ifndef WIN32
> -               { "core.untrackedCache", "true" },
> +               { "core.untrackedCache", "true", 1 },
>  #else
>                 /*
>                  * Unfortunately, Scalar's Functional Tests demonstrated
> @@ -140,28 +142,29 @@ static int set_recommended_config(void)
>                  * Therefore, with a sad heart, we disable this very useful
>                  * feature on Windows.
>                  */
> -               { "core.untrackedCache", "false" },
> +               { "core.untrackedCache", "false", 1 },
>  #endif
> -               { "core.logAllRefUpdates", "true" },
> -               { "credential.https://dev.azure.com.useHttpPath", "true" },
> -               { "credential.validate", "false" }, /* GCM4W-only */
> -               { "gc.auto", "0" },
> -               { "gui.GCWarning", "false" },
> -               { "index.threads", "true" },
> -               { "index.version", "4" },
> -               { "merge.stat", "false" },
> -               { "merge.renames", "false" },
> -               { "pack.useBitmaps", "false" },
> -               { "pack.useSparse", "true" },
> -               { "receive.autoGC", "false" },
> -               { "reset.quiet", "true" },
> -               { "feature.manyFiles", "false" },
> -               { "feature.experimental", "false" },
> -               { "fetch.unpackLimit", "1" },
> -               { "fetch.writeCommitGraph", "false" },
> +               { "core.logAllRefUpdates", "true", 1 },
> +               { "credential.https://dev.azure.com.useHttpPath", "true", 1 },
> +               { "credential.validate", "false", 1 }, /* GCM4W-only */
> +               { "gc.auto", "0", 1 },
> +               { "gui.GCWarning", "false", 1 },
> +               { "index.threads", "true", 1 },
> +               { "index.version", "4", 1 },
> +               { "merge.stat", "false", 1 },
> +               { "merge.renames", "false", 1 },
> +               { "pack.useBitmaps", "false", 1 },
> +               { "pack.useSparse", "true", 1 },
> +               { "receive.autoGC", "false", 1 },
> +               { "reset.quiet", "true", 1 },
> +               { "feature.manyFiles", "false", 1 },
> +               { "feature.experimental", "false", 1 },
> +               { "fetch.unpackLimit", "1", 1 },
> +               { "fetch.writeCommitGraph", "false", 1 },
>  #ifdef WIN32
> -               { "http.sslBackend", "schannel" },
> +               { "http.sslBackend", "schannel", 1 },
>  #endif
> +               /* Optional */
>                 { "status.aheadBehind", "false" },
>                 { "commitGraph.generationVersion", "1" },
>                 { "core.autoCRLF", "false" },

Now you have optional settings...but index.version and merge.renames
aren't among them??  Why are those required?  (...and to go a step
further; should merge.renames even be off in a merge-ort world?)
Johannes Schindelin Oct. 6, 2021, 8:43 p.m. UTC | #2
Hi Elijah,

On Mon, 27 Sep 2021, Elijah Newren wrote:

> On Tue, Sep 14, 2021 at 7:39 AM Johannes Schindelin via GitGitGadget
> <gitgitgadget@gmail.com> wrote:
> >
> > From: Johannes Schindelin <johannes.schindelin@gmx.de>
> >
> > This comes in handy during Scalar upgrades, or when config settings were
> > messed up by mistake.
> >
> > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> > ---
> >  contrib/scalar/scalar.c          | 79 +++++++++++++++++++++-----------
> >  contrib/scalar/scalar.txt        |  8 ++++
> >  contrib/scalar/t/t9099-scalar.sh |  8 ++++
> >  3 files changed, 67 insertions(+), 28 deletions(-)
> >
> > diff --git a/contrib/scalar/scalar.c b/contrib/scalar/scalar.c
> > index 8a11f390251..1fff7eb7c12 100644
> > --- a/contrib/scalar/scalar.c
> > +++ b/contrib/scalar/scalar.c
> > @@ -115,18 +115,20 @@ static int run_git(const char *arg, ...)
> >         return res;
> >  }
> >
> > -static int set_recommended_config(void)
> > +static int set_recommended_config(int reconfigure)
> >  {
> >         struct {
> >                 const char *key;
> >                 const char *value;
> > +               int overwrite_on_reconfigure;
> >         } config[] = {
> > -               { "am.keepCR", "true" },
> > -               { "core.FSCache", "true" },
> > -               { "core.multiPackIndex", "true" },
> > -               { "core.preloadIndex", "true" },
> > +               /* Required */
> > +               { "am.keepCR", "true", 1 },
> > +               { "core.FSCache", "true", 1 },
> > +               { "core.multiPackIndex", "true", 1 },
> > +               { "core.preloadIndex", "true", 1 },
> >  #ifndef WIN32
> > -               { "core.untrackedCache", "true" },
> > +               { "core.untrackedCache", "true", 1 },
> >  #else
> >                 /*
> >                  * Unfortunately, Scalar's Functional Tests demonstrated
> > @@ -140,28 +142,29 @@ static int set_recommended_config(void)
> >                  * Therefore, with a sad heart, we disable this very useful
> >                  * feature on Windows.
> >                  */
> > -               { "core.untrackedCache", "false" },
> > +               { "core.untrackedCache", "false", 1 },
> >  #endif
> > -               { "core.logAllRefUpdates", "true" },
> > -               { "credential.https://dev.azure.com.useHttpPath", "true" },
> > -               { "credential.validate", "false" }, /* GCM4W-only */
> > -               { "gc.auto", "0" },
> > -               { "gui.GCWarning", "false" },
> > -               { "index.threads", "true" },
> > -               { "index.version", "4" },
> > -               { "merge.stat", "false" },
> > -               { "merge.renames", "false" },
> > -               { "pack.useBitmaps", "false" },
> > -               { "pack.useSparse", "true" },
> > -               { "receive.autoGC", "false" },
> > -               { "reset.quiet", "true" },
> > -               { "feature.manyFiles", "false" },
> > -               { "feature.experimental", "false" },
> > -               { "fetch.unpackLimit", "1" },
> > -               { "fetch.writeCommitGraph", "false" },
> > +               { "core.logAllRefUpdates", "true", 1 },
> > +               { "credential.https://dev.azure.com.useHttpPath", "true", 1 },
> > +               { "credential.validate", "false", 1 }, /* GCM4W-only */
> > +               { "gc.auto", "0", 1 },
> > +               { "gui.GCWarning", "false", 1 },
> > +               { "index.threads", "true", 1 },
> > +               { "index.version", "4", 1 },
> > +               { "merge.stat", "false", 1 },
> > +               { "merge.renames", "false", 1 },
> > +               { "pack.useBitmaps", "false", 1 },
> > +               { "pack.useSparse", "true", 1 },
> > +               { "receive.autoGC", "false", 1 },
> > +               { "reset.quiet", "true", 1 },
> > +               { "feature.manyFiles", "false", 1 },
> > +               { "feature.experimental", "false", 1 },
> > +               { "fetch.unpackLimit", "1", 1 },
> > +               { "fetch.writeCommitGraph", "false", 1 },
> >  #ifdef WIN32
> > -               { "http.sslBackend", "schannel" },
> > +               { "http.sslBackend", "schannel", 1 },
> >  #endif
> > +               /* Optional */
> >                 { "status.aheadBehind", "false" },
> >                 { "commitGraph.generationVersion", "1" },
> >                 { "core.autoCRLF", "false" },
>
> Now you have optional settings...but index.version and merge.renames
> aren't among them??  Why are those required?  (...and to go a step
> further; should merge.renames even be off in a merge-ort world?)

I think the idea here is that they are required so that a `scalar
reconfigure` will set them, even if the current enlistment had been
created by a previous Scalar version that had _not_ set those.

And yes, in a merge-ort world, `merge.renames` should probably be forced
to `true`, again because it is in the "Required" section.

Ciao,
Dscho
diff mbox series

Patch

diff --git a/contrib/scalar/scalar.c b/contrib/scalar/scalar.c
index 8a11f390251..1fff7eb7c12 100644
--- a/contrib/scalar/scalar.c
+++ b/contrib/scalar/scalar.c
@@ -115,18 +115,20 @@  static int run_git(const char *arg, ...)
 	return res;
 }
 
-static int set_recommended_config(void)
+static int set_recommended_config(int reconfigure)
 {
 	struct {
 		const char *key;
 		const char *value;
+		int overwrite_on_reconfigure;
 	} config[] = {
-		{ "am.keepCR", "true" },
-		{ "core.FSCache", "true" },
-		{ "core.multiPackIndex", "true" },
-		{ "core.preloadIndex", "true" },
+		/* Required */
+		{ "am.keepCR", "true", 1 },
+		{ "core.FSCache", "true", 1 },
+		{ "core.multiPackIndex", "true", 1 },
+		{ "core.preloadIndex", "true", 1 },
 #ifndef WIN32
-		{ "core.untrackedCache", "true" },
+		{ "core.untrackedCache", "true", 1 },
 #else
 		/*
 		 * Unfortunately, Scalar's Functional Tests demonstrated
@@ -140,28 +142,29 @@  static int set_recommended_config(void)
 		 * Therefore, with a sad heart, we disable this very useful
 		 * feature on Windows.
 		 */
-		{ "core.untrackedCache", "false" },
+		{ "core.untrackedCache", "false", 1 },
 #endif
-		{ "core.logAllRefUpdates", "true" },
-		{ "credential.https://dev.azure.com.useHttpPath", "true" },
-		{ "credential.validate", "false" }, /* GCM4W-only */
-		{ "gc.auto", "0" },
-		{ "gui.GCWarning", "false" },
-		{ "index.threads", "true" },
-		{ "index.version", "4" },
-		{ "merge.stat", "false" },
-		{ "merge.renames", "false" },
-		{ "pack.useBitmaps", "false" },
-		{ "pack.useSparse", "true" },
-		{ "receive.autoGC", "false" },
-		{ "reset.quiet", "true" },
-		{ "feature.manyFiles", "false" },
-		{ "feature.experimental", "false" },
-		{ "fetch.unpackLimit", "1" },
-		{ "fetch.writeCommitGraph", "false" },
+		{ "core.logAllRefUpdates", "true", 1 },
+		{ "credential.https://dev.azure.com.useHttpPath", "true", 1 },
+		{ "credential.validate", "false", 1 }, /* GCM4W-only */
+		{ "gc.auto", "0", 1 },
+		{ "gui.GCWarning", "false", 1 },
+		{ "index.threads", "true", 1 },
+		{ "index.version", "4", 1 },
+		{ "merge.stat", "false", 1 },
+		{ "merge.renames", "false", 1 },
+		{ "pack.useBitmaps", "false", 1 },
+		{ "pack.useSparse", "true", 1 },
+		{ "receive.autoGC", "false", 1 },
+		{ "reset.quiet", "true", 1 },
+		{ "feature.manyFiles", "false", 1 },
+		{ "feature.experimental", "false", 1 },
+		{ "fetch.unpackLimit", "1", 1 },
+		{ "fetch.writeCommitGraph", "false", 1 },
 #ifdef WIN32
-		{ "http.sslBackend", "schannel" },
+		{ "http.sslBackend", "schannel", 1 },
 #endif
+		/* Optional */
 		{ "status.aheadBehind", "false" },
 		{ "commitGraph.generationVersion", "1" },
 		{ "core.autoCRLF", "false" },
@@ -172,7 +175,8 @@  static int set_recommended_config(void)
 	char *value;
 
 	for (i = 0; config[i].key; i++) {
-		if (git_config_get_string(config[i].key, &value)) {
+		if ((reconfigure && config[i].overwrite_on_reconfigure) ||
+		    git_config_get_string(config[i].key, &value)) {
 			trace2_data_string("scalar", the_repository, config[i].key, "created");
 			if (git_config_set_gently(config[i].key,
 						  config[i].value) < 0)
@@ -237,7 +241,7 @@  static int register_dir(void)
 	int res = add_or_remove_enlistment(1);
 
 	if (!res)
-		res = set_recommended_config();
+		res = set_recommended_config(0);
 
 	if (!res)
 		res = toggle_maintenance(1);
@@ -425,7 +429,7 @@  static int cmd_clone(int argc, const char **argv)
 	    (res = run_git("sparse-checkout", "init", "--cone", NULL)))
 		goto cleanup;
 
-	if (set_recommended_config())
+	if (set_recommended_config(0))
 		return error(_("could not configure '%s'"), dir);
 
 	if ((res = run_git("fetch", "--quiet", "origin", NULL))) {
@@ -490,6 +494,24 @@  static int cmd_register(int argc, const char **argv)
 	return register_dir();
 }
 
+static int cmd_reconfigure(int argc, const char **argv)
+{
+	struct option options[] = {
+		OPT_END(),
+	};
+	const char * const usage[] = {
+		N_("scalar reconfigure [<enlistment>]"),
+		NULL
+	};
+
+	argc = parse_options(argc, argv, NULL, options,
+			     usage, 0);
+
+	setup_enlistment_directory(argc, argv, usage, options, NULL);
+
+	return set_recommended_config(1);
+}
+
 static int cmd_run(int argc, const char **argv)
 {
 	struct option options[] = {
@@ -626,6 +648,7 @@  static struct {
 	{ "register", cmd_register },
 	{ "unregister", cmd_unregister },
 	{ "run", cmd_run },
+	{ "reconfigure", cmd_reconfigure },
 	{ NULL, NULL},
 };
 
diff --git a/contrib/scalar/scalar.txt b/contrib/scalar/scalar.txt
index f139a14445d..f4e4686e8c8 100644
--- a/contrib/scalar/scalar.txt
+++ b/contrib/scalar/scalar.txt
@@ -13,6 +13,7 @@  scalar list
 scalar register [<enlistment>]
 scalar unregister [<enlistment>]
 scalar run ( all | config | commit-graph | fetch | loose-objects | pack-files ) [<enlistment>]
+scalar reconfigure <enlistment>
 
 DESCRIPTION
 -----------
@@ -116,6 +117,13 @@  opinionated default settings that make Git work more efficiently with
 large repositories. As this task is run as part of `scalar clone`
 automatically, explicit invocations of this task are rarely needed.
 
+Reconfigure
+~~~~~~~~~~~
+
+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.
+
 SEE ALSO
 --------
 linkgit:git-clone[1], linkgit:git-maintenance[1].
diff --git a/contrib/scalar/t/t9099-scalar.sh b/contrib/scalar/t/t9099-scalar.sh
index 9a35ab4fde6..e6d74a06ca0 100755
--- a/contrib/scalar/t/t9099-scalar.sh
+++ b/contrib/scalar/t/t9099-scalar.sh
@@ -65,4 +65,12 @@  test_expect_success 'scalar clone' '
 	)
 '
 
+test_expect_success 'scalar reconfigure' '
+	git init one/src &&
+	scalar register one &&
+	git -C one/src config core.preloadIndex false &&
+	scalar reconfigure one &&
+	test true = "$(git -C one/src config core.preloadIndex)"
+'
+
 test_done