git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Junio C Hamano <gitster@pobox.com>
Date
Jul 15, 2020, 18:09 UTC
Message-ID
<xmqqh7u8k5va.fsf@gitster.c.googlers.com>
In-Reply-To
<31f52913-8745-18b4-63fc-37d2a9aea8d0@gmail.com>
Derrick Stolee <stolee@gmail.com> writes:
Show 8 quoted lines
> 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
Previous: Derrick StoleeNext: Derrick Stolee
Message 12 of 39 in “setup: warn about un-enabled extensions”
  1. setup: warn about un-enabled extensionsJohannes Schindelin via GitGitGadget, Jul 13, 2020
  2. Junio C HamanoJul 13, 2020
  3. Derrick StoleeJul 14, 2020
  4. Johannes SchindelinJul 14, 2020
  5. Junio C HamanoJul 14, 2020
  6. Derrick StoleeJul 14, 2020
  7. Johannes SchindelinJul 14, 2020
  8. Junio C HamanoJul 14, 2020
  9. Junio C HamanoJul 15, 2020
  10. Junio C HamanoJul 15, 2020
  11. Derrick StoleeJul 15, 2020
  12. Junio C HamanoJul 15, 2020
  13. Derrick StoleeJul 15, 2020
  14. Johannes SchindelinJul 15, 2020
  15. Junio C HamanoJul 15, 2020
  16. Johannes SchindelinJul 15, 2020
  17. Jonathan NiederJul 15, 2020
  18. Junio C HamanoJul 16, 2020
  19. 0/2 extensions.* fixes for 2.28 (Re: [PATCH] setup: warn about un-enabled extensions)Jonathan Nieder, Jul 16, 2020
  20. 1/2 Revert "check_repository_format_gently(): refuse extensions for old repositories"Jonathan Nieder, Jul 16, 2020
  21. Jeff KingJul 16, 2020
  22. 2/2 repository: allow repository format upgrade with extensionsJonathan Nieder, Jul 16, 2020
  23. Junio C HamanoJul 16, 2020
  24. Jeff KingJul 16, 2020
  25. Jeff KingJul 16, 2020
  26. Derrick StoleeJul 16, 2020
  27. Junio C HamanoJul 16, 2020
  28. Jeff KingJul 16, 2020
  29. Junio C HamanoJul 16, 2020
  30. Junio C HamanoJul 16, 2020
  31. Jeff KingJul 16, 2020
  32. Junio C HamanoJul 16, 2020
  33. Jonathan NiederJul 16, 2020
  34. Junio C HamanoJul 16, 2020
  35. Jeff KingJul 17, 2020
  36. Junio C HamanoJul 17, 2020
  37. Jeff KingJul 17, 2020
  38. Johannes SchindelinJul 16, 2020
  39. Derrick StoleeJul 16, 2020

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.