From: Junio C Hamano <gitster@pobox.com>
To: "Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com>
Cc: git@vger.kernel.org, newren@gmail.com, delphij@google.com,
	peff@peff.net, Derrick Stolee <derrickstolee@github.com>,
	Derrick Stolee <dstolee@microsoft.com>
Subject: Re: [PATCH] setup: warn if extensions exist on old format
Date: Mon, 13 Jul 2020 13:00:35 -0700	[thread overview]
Message-ID: <xmqqy2nnmbh8.fsf@gitster.c.googlers.com> (raw)
In-Reply-To: <xmqqblkjnr5m.fsf@gitster.c.googlers.com> (Junio C. Hamano's message of "Mon, 13 Jul 2020 12:36:37 -0700")
Junio C Hamano <gitster@pobox.com> writes:
> An alternative may be to grandfather some extensions that were
> enabled by git by mistake without updating the format version, and
> we update the repository even if the repository has extensions that
> should not exist, but those offending extensions are limited only to
> those that we decide to special case.  That would make the end-user
> experience even smoother.
>
> Is extenions.worktreeCOnfig the only one that needs this escape
> hatch?
Assuming that worktreeconfig is the only thing, the change may look
like this.  With this change, we might want to drop the new warning
in hunk ll.542- to avoid encouraging people to muck with their
repository with random configuration variables that happen to share
extensions.* prefix with us.
 cache.h |  2 +-
 setup.c | 17 +++++++++++++----
 2 files changed, 14 insertions(+), 5 deletions(-)
diff --git a/cache.h b/cache.h
index e5885cc9ea..8ff46857f6 100644
--- a/cache.h
+++ b/cache.h
@@ -1042,8 +1042,8 @@ struct repository_format {
 	int worktree_config;
 	int is_bare;
 	int hash_algo;
-	int has_extensions;
 	char *work_tree;
+	int has_unallowed_extensions;
 	struct string_list unknown_extensions;
 };
 
diff --git a/setup.c b/setup.c
index eb066db6d8..5f4786d3b9 100644
--- a/setup.c
+++ b/setup.c
@@ -455,12 +455,13 @@ static int check_repo_format(const char *var, const char *value, void *vdata)
 	if (strcmp(var, "core.repositoryformatversion") == 0)
 		data->version = git_config_int(var, value);
 	else if (skip_prefix(var, "extensions.", &ext)) {
-		data->has_extensions = 1;
 		/*
 		 * record any known extensions here; otherwise,
 		 * we fall through to recording it as unknown, and
 		 * check_repository_format will complain
 		 */
+		int is_unallowed_extension = 1;
+
 		if (!strcmp(ext, "noop"))
 			;
 		else if (!strcmp(ext, "preciousobjects"))
@@ -469,10 +470,13 @@ static int check_repo_format(const char *var, const char *value, void *vdata)
 			if (!value)
 				return config_error_nonbool(var);
 			data->partial_clone = xstrdup(value);
-		} else if (!strcmp(ext, "worktreeconfig"))
+		} else if (!strcmp(ext, "worktreeconfig")) {
 			data->worktree_config = git_config_bool(var, value);
-		else
+			is_unallowed_extension = 0;
+		} else
 			string_list_append(&data->unknown_extensions, ext);
+
+		data->has_unallowed_extensions |= is_unallowed_extension;
 	}
 
 	return read_worktree_config(var, value, vdata);
@@ -542,6 +546,11 @@ static int check_repository_format_gently(const char *gitdir, struct repository_
 		}
 	}
 
+	if (candidate->version == 0 && candidate->has_unallowed_extensions) {
+		warning(_("some extensions are enabled, but core.repositoryFormatVersion=0"));
+		warning(_("if you intended to use extensions, run 'git config core.repositoryFormatVersion 1'"));
+	}
+
 	return 0;
 }
 
@@ -560,7 +569,7 @@ int upgrade_repository_format(int target_version)
 		return 0;
 
 	if (verify_repository_format(&repo_fmt, &err) < 0 ||
-	    (!repo_fmt.version && repo_fmt.has_extensions)) {
+	    (!repo_fmt.version && repo_fmt.has_unallowed_extensions)) {
 		warning("unable to upgrade repository format from %d to %d: %s",
 			repo_fmt.version, target_version, err.buf);
 		strbuf_release(&err);
next prev parent reply	other threads:[~2020-07-13 20:00 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-13 19:20 [PATCH] setup: warn if extensions exist on old format Derrick Stolee via GitGitGadget
2020-07-13 19:34 ` Taylor Blau
2020-07-13 19:41   ` Derrick Stolee
2020-07-13 19:36 ` Junio C Hamano
2020-07-13 20:00   ` Junio C Hamano [this message]
2020-07-13 20:18     ` [PATCH] setup: tweak upgrade policy to grandfather worktreeconfig Junio C Hamano
2020-07-13 20:46       ` Derrick Stolee
2020-07-13 21:45         ` Junio C Hamano
2020-07-14  4:06         ` Jonathan Nieder
2020-07-14  4:27           ` Junio C Hamano
2020-07-14  1:26 ` [PATCH v2 0/2] setup: warn if extensions exist on old format Derrick Stolee via GitGitGadget
2020-07-14  1:26   ` [PATCH v2 1/2] setup: tweak upgrade policy to grandfather worktreeconfig Derrick Stolee via GitGitGadget
2020-07-14 20:22     ` Johannes Schindelin
2020-07-14  1:26   ` [PATCH v2 2/2] config: provide extra detail about worktree config Derrick Stolee via GitGitGadget
2020-07-14 20:24   ` [PATCH v2 0/2] setup: warn if extensions exist on old format Johannes Schindelin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox
  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):
  git send-email \
    --in-reply-to=xmqqy2nnmbh8.fsf@gitster.c.googlers.com \
    --to=gitster@pobox.com \
    --cc=delphij@google.com \
    --cc=derrickstolee@github.com \
    --cc=dstolee@microsoft.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=newren@gmail.com \
    --cc=peff@peff.net \
    /path/to/YOUR_REPLY
  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
  Be sure your reply has a Subject: header at the top and a blank line
  before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).