Re: [PATCH] setup: warn about un-enabled extensions

[Junio C Hamano <gitster@pobox.com>]

Derrick Stolee <stolee@gmail.com> writes:

> Your previous diff had this comment, which I thought to be
> helpful: 
>
> +		/*
> +		 * Extensions are added by more "} else if (...) {"
> +		 * lines here, but do NOT mark them as allowed in v0
> +		 * by copy-pasting without thinking.
> +		 */

Yeah, but it felt somewhat strange to have it at the end of one
entry, like this:

+			unallowed_in_v0 = 0;
 		} else if (!strcmp(ext, "worktreeconfig")) {
 			data->worktree_config = git_config_bool(var, value);
+			unallowed_in_v0 = 0;
+		/*
+		 * Extensions are added by more "} else if (...) {"
+		 * lines here, but do NOT mark them as allowed in v0
+		 * by copy-pasting without thinking.
+		 */
+		} else {
 			string_list_append(&data->unknown_extensions, ext);


In any case, I updated the comment in front of the if/else if/
cascade to essentially say the same thing, and with test updates
this time.

Thanks.

-- >8 --
Subject: [PATCH] setup: grandfather other extensions that used to be honored by mistake

We special cased worktreeconfig extension to be OK to exist when the
repository gets converted from v0 to v1 in an earlier commit, but
other extenions were also honored by mistake in v0.  Mark them the
same way so that they won't interfere with the upgrading from v0 to
v1.

A test in t0410 used to expect that presence of the
extension.partialclone configuration variable to interfere, but that
no longer is true.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 setup.c                  | 30 +++++++++++++++++++-----------
 t/t0410-partial-clone.sh | 18 ++++++++++++++++--
 2 files changed, 35 insertions(+), 13 deletions(-)

diff --git a/setup.c b/setup.c
index 65270440a9..97292479d6 100644
--- a/setup.c
+++ b/setup.c
@@ -456,27 +456,35 @@ static int check_repo_format(const char *var, const char *value, void *vdata)
 		data->version = git_config_int(var, value);
 	else if (skip_prefix(var, "extensions.", &ext)) {
 		/*
-		 * record any known extensions here; otherwise,
-		 * we fall through to recording it as unknown, and
-		 * check_repository_format will complain
+		 * Grandfather extensions that were known in 2.27 and
+		 * were honored by mistake even in v0 repositories; it
+		 * shoudn't be an error to upgrade v0 to v1 with them
+		 * in the repository, as they couldn't have been used
+		 * for incompatible purposes by the end user.
+		 *
+		 * When adding new extensions support in this if/elseif/...
+		 * cascade, do not mark them as allowed in v0!
 		 */
-		int is_unallowed_extension = 1;
+		int unallowed_in_v0 = 1;
 
-		if (!strcmp(ext, "noop"))
-			;
-		else if (!strcmp(ext, "preciousobjects"))
+		if (!strcmp(ext, "noop")) {
+			unallowed_in_v0 = 0;
+		} else if (!strcmp(ext, "preciousobjects")) {
 			data->precious_objects = git_config_bool(var, value);
-		else if (!strcmp(ext, "partialclone")) {
+			unallowed_in_v0 = 0;
+		} else if (!strcmp(ext, "partialclone")) {
 			if (!value)
 				return config_error_nonbool(var);
 			data->partial_clone = xstrdup(value);
+			unallowed_in_v0 = 0;
 		} else if (!strcmp(ext, "worktreeconfig")) {
 			data->worktree_config = git_config_bool(var, value);
-			is_unallowed_extension = 0;
-		} else
+			unallowed_in_v0 = 0;
+		} else {
 			string_list_append(&data->unknown_extensions, ext);
+		}
 
-		data->has_unallowed_extensions |= is_unallowed_extension;
+		data->has_unallowed_extensions |= unallowed_in_v0;
 	}
 
 	return read_worktree_config(var, value, vdata);
diff --git a/t/t0410-partial-clone.sh b/t/t0410-partial-clone.sh
index 463dc3a8be..e9674fc257 100755
--- a/t/t0410-partial-clone.sh
+++ b/t/t0410-partial-clone.sh
@@ -42,7 +42,7 @@ test_expect_success 'convert shallow clone to partial clone' '
 	test_cmp_config -C client 1 core.repositoryformatversion
 '
 
-test_expect_success 'convert shallow clone to partial clone must fail with any extension' '
+test_expect_success 'convert shallow clone to partial clone is OK with a grandfathered extension' '
 	rm -fr server client &&
 	test_create_repo server &&
 	test_commit -C server my_commit 1 &&
@@ -50,7 +50,21 @@ test_expect_success 'convert shallow clone to partial clone must fail with any e
 	git clone --depth=1 "file://$(pwd)/server" client &&
 	test_cmp_config -C client 0 core.repositoryformatversion &&
 	git -C client config extensions.partialclone origin &&
-	test_must_fail git -C client fetch --unshallow --filter="blob:none"
+	git -C client fetch --unshallow --filter="blob:none" &&
+	test_cmp_config -C client 1 core.repositoryformatversion
+'
+
+test_expect_success 'convert shallow clone to partial clone must fail with an unknown extension' '
+	rm -fr server client &&
+	test_create_repo server &&
+	test_commit -C server my_commit 1 &&
+	test_commit -C server my_commit2 1 &&
+	git clone --depth=1 "file://$(pwd)/server" client &&
+	test_cmp_config -C client 0 core.repositoryformatversion &&
+	git -C client config extensions.partialclone origin &&
+	git -C client config extensions.unknown true &&
+	test_must_fail git -C client fetch --unshallow --filter="blob:none" &&
+	test_cmp_config -C client 0 core.repositoryformatversion
 '
 
 test_expect_success 'missing reflog object, but promised by a commit, passes fsck' '
-- 
2.28.0-rc0