setup: warn if extensions exist on old format

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/[email protected]/

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 <[email protected]>

## 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 <[email protected]>

+ ## 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