From: "Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: newren@gmail.com, delphij@google.com, peff@peff.net,
johannes.schindelin@gmx.de, me@ttaylorr.com,
Derrick Stolee <derrickstolee@github.com>
Subject: [PATCH v2 0/2] setup: warn if extensions exist on old format
Date: Tue, 14 Jul 2020 01:26:55 +0000 [thread overview]
Message-ID: <pull.674.v2.git.1594690017.gitgitgadget@gmail.com> (raw)
In-Reply-To: <pull.674.git.1594668051847.gitgitgadget@gmail.com>
This is based on xl/upgrade-repo-format.
Thanks, Taylor and Junio for jumping in with helpful review.
Updates in v2:
1. My initial patch is essentially dropped in its entirety, with Junio's
patch here instead. I added an extra test and kept some of my commit
message.
2. A second patch has joined the fray, hopefully answering the concerned
raise by Johannes [1].
[1]
https://lore.kernel.org/git/pull.675.git.1594677321039.gitgitgadget@gmail.com/
Thanks, -Stolee
Derrick Stolee (2):
setup: tweak upgrade policy to grandfather worktreeconfig
config: provide extra detail about worktree config
builtin/config.c | 5 +++--
cache.h | 2 +-
setup.c | 12 ++++++++----
t/t1091-sparse-checkout-builtin.sh | 12 ++++++++++++
t/t2404-worktree-config.sh | 12 ++++++++++++
5 files changed, 36 insertions(+), 7 deletions(-)
base-commit: 14c7fa269e42df4133edd9ae7763b678ed6594cd
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-674%2Fderrickstolee%2Fsparse-checkout-warning-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-674/derrickstolee/sparse-checkout-warning-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/674
Range-diff vs v1:
1: 113cb7cda2 ! 1: 1b26d9710a setup: warn if extensions exist on old format
@@ Metadata
Author: Derrick Stolee <dstolee@microsoft.com>
## Commit message ##
- setup: warn if extensions exist on old format
+ setup: tweak upgrade policy to grandfather worktreeconfig
Prior to 14c7fa269e4 (check_repository_format_gently(): refuse extensions
for old repositories, 2020-06-05), Git was honoring configured
@@ Commit message
The issue now is that users who relied on that previously bad behavior
will upgrade to the next version of Git and suddently be in a bad
- situation. In particular, users of the 'git sparse-checkout' builting
+ situation. In particular, users of the 'git sparse-checkout' command
will rely on the extensions.worktreeConfig for the core.sparseCheckout
and core.sparseCheckoutCone config options. Without that extension,
these users will suddenly have repositories stop acting like a sparse
@@ Commit message
This is because the logic in 14c7fa269e4 refuses to upgrae repos when
the version is unset but extensions exist.
- While it is important to correct this improper behavior, create a
- warning so users in this situation can correct themselves without too
- much guesswork. By creating a warning in
- check_repository_format_gently(), we can alert the user if they have a
- ocnfigured extension but not a configured repository version.
+ One possible way to alert a user of this issue is to warn them when Git
+ notices an extension exists, but core.repositoryFormatVersion is not a
+ correct value. However,
+ - it requires the end-user to read, understand and execute the
+ manual upgrade
+
+ - it encourages to follow the same procedure blindly, making the
+ protection even less useful
+
+ Let's instead keep failing hard without teaching how to bypass the
+ repository protection, but allow upgrading even when only the
+ worktreeconfig extension exists in an old repository, which is
+ likely to be set by a broke version of Git that did not update the
+ repository version when setting the extension.
+
+ This change of behavior is made visible by testing how 'git
+ sparse-checkout init' behaves to upgrade the repository format version
+ even if the extension.worktreeConfig is already set. This would
+ previously fail without a clear way forward.
+
+ Signed-off-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
+ ## cache.h ##
+@@ cache.h: 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;
+ };
+
+
## setup.c ##
-@@ setup.c: static int check_repository_format_gently(const char *gitdir, struct repository_
- }
+@@ setup.c: 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"))
+@@ setup.c: 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;
}
-+ if (candidate->version == 0 && candidate->has_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;
- }
+ return read_worktree_config(var, value, vdata);
+@@ setup.c: 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);
## t/t1091-sparse-checkout-builtin.sh ##
@@ t/t1091-sparse-checkout-builtin.sh: test_expect_success 'git sparse-checkout init' '
check_files repo a
'
-+test_expect_success 'warning about core.repositoryFormatVersion' '
++test_expect_success 'git sparse-checkout works if repository format is wrong' '
+ test_when_finished git -C repo config core.repositoryFormatVersion 1 &&
-+ git -C repo status 2>err &&
-+ test_must_be_empty err &&
-+ git -C repo config --local core.repositoryFormatVersion 0 &&
-+ git -C repo status 2>err &&
-+ test_i18ngrep "some extensions are enabled, but core.repositoryFormatVersion=0" err
++ git -C repo config --unset core.repositoryFormatVersion &&
++ git -C repo sparse-checkout init &&
++ git -C repo config core.repositoryFormatVersion >actual &&
++ echo 1 >expect &&
++ git -C repo config core.repositoryFormatVersion 0 &&
++ git -C repo sparse-checkout init &&
++ git -C repo config core.repositoryFormatVersion >actual &&
++ test_cmp expect actual
+'
+
test_expect_success 'git sparse-checkout list after init' '
-: ---------- > 2: e11e973c6f config: provide extra detail about worktree config
--
gitgitgadget
next prev parent reply other threads:[~2020-07-14 1:27 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
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 ` Derrick Stolee via GitGitGadget [this message]
2020-07-14 1:26 ` [PATCH v2 1/2] " 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
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=pull.674.v2.git.1594690017.gitgitgadget@gmail.com \
--to=gitgitgadget@gmail.com \
--cc=delphij@google.com \
--cc=derrickstolee@github.com \
--cc=git@vger.kernel.org \
--cc=johannes.schindelin@gmx.de \
--cc=me@ttaylorr.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.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
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 read-only IMAP folder(s) and NNTP newsgroup(s).