From 11664196ac15b0382793ec0923e56a103e973bc9 Mon Sep 17 00:00:00 2001 From: Jonathan Nieder Date: Wed, 15 Jul 2020 23:24:29 -0700 Subject: [PATCH 1/2] Revert "check_repository_format_gently(): refuse extensions for old repositories" This reverts commit 14c7fa269e42df4133edd9ae7763b678ed6594cd. The core.repositoryFormatVersion field was introduced in ab9cb76f661 (Repository format version check., 2005-11-25), providing a welcome bit of forward compatibility, thanks to some welcome analysis by Martin Atukunda. The semantics are simple: a repository with core.repositoryFormatVersion set to 0 should be comprehensible by all Git implementations in active use; and Git implementations should error out early instead of trying to act on Git repositories with higher core.repositoryFormatVersion values representing new formats that they do not understand. A new repository format did not need to be defined until 00a09d57eb8 (introduce "extensions" form of core.repositoryformatversion, 2015-06-23). This provided a finer-grained extension mechanism for Git repositories. In a repository with core.repositoryFormatVersion set to 1, Git implementations can act on "extensions.*" settings that modify how a repository is interpreted. In repository format version 1, unrecognized extensions settings cause Git to error out. What happens if a user sets an extension setting but forgets to increase the repository format version to 1? The extension settings were still recognized in that case; worse, unrecognized extensions settings do *not* cause Git to error out. So combining repository format version 0 with extensions settings produces in some sense the worst of both worlds. To improve that situation, since 14c7fa269e4 (check_repository_format_gently(): refuse extensions for old repositories, 2020-06-05) Git instead ignores extensions in v0 mode. This way, v0 repositories get the historical (pre-2015) behavior and maintain compatibility with Git implementations that do not know about the v1 format. Unfortunately, users had been using this sort of configuration and this behavior change came to many as a surprise: - users of "git config --worktree" that had followed its advice to enable extensions.worktreeConfig (without also increasing the repository format version) would find their worktree configuration no longer taking effect - tools such as copybara[*] that had set extensions.partialClone in existing repositories (without also increasing the repository format version) would find that setting no longer taking effect The behavior introduced in 14c7fa269e4 might be a good behavior if we were traveling back in time to 2015, but we're far too late. For some reason I thought that it was what had been originally implemented and that it had regressed. Apologies for not doing my research when 14c7fa269e4 was under development. Let's return to the behavior we've had since 2015: always act on extensions.* settings, regardless of repository format version. While we're here, include some tests to describe the effect on the "upgrade repository version" code path. [*] https://github.com/google/copybara/commit/ca76c0b1e13c4e36448d12c2aba4a5d9d98fb6e7 Reported-by: Johannes Schindelin Signed-off-by: Jonathan Nieder Signed-off-by: Junio C Hamano --- setup.c | 12 +++--------- t/t0410-partial-clone.sh | 15 +++++++++++++-- 2 files changed, 16 insertions(+), 11 deletions(-) diff --git a/setup.c b/setup.c index eb066db6d8c..597b41b822c 100644 --- a/setup.c +++ b/setup.c @@ -507,15 +507,9 @@ static int check_repository_format_gently(const char *gitdir, struct repository_ die("%s", err.buf); } - if (candidate->version >= 1) { - repository_format_precious_objects = candidate->precious_objects; - set_repository_format_partial_clone(candidate->partial_clone); - repository_format_worktree_config = candidate->worktree_config; - } else { - repository_format_precious_objects = 0; - set_repository_format_partial_clone(NULL); - repository_format_worktree_config = 0; - } + repository_format_precious_objects = candidate->precious_objects; + set_repository_format_partial_clone(candidate->partial_clone); + repository_format_worktree_config = candidate->worktree_config; string_list_clear(&candidate->unknown_extensions, 0); if (repository_format_worktree_config) { diff --git a/t/t0410-partial-clone.sh b/t/t0410-partial-clone.sh index 463dc3a8be0..51d1eba6050 100755 --- a/t/t0410-partial-clone.sh +++ b/t/t0410-partial-clone.sh @@ -42,14 +42,25 @@ 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 'converting to partial clone fails with noop 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.noop true && + test_must_fail git -C client fetch --unshallow --filter="blob:none" +' + +test_expect_success 'converting to partial clone fails with unrecognized 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.nonsense true && test_must_fail git -C client fetch --unshallow --filter="blob:none" ' From 62f2eca6065d0c27b0ea3b7806a590466ea04960 Mon Sep 17 00:00:00 2001 From: Jonathan Nieder Date: Wed, 15 Jul 2020 23:28:18 -0700 Subject: [PATCH 2/2] repository: allow repository format upgrade with extensions Now that we officially permit repository extensions in repository format v0, permit upgrading a repository with extensions from v0 to v1 as well. For example, this means a repository where the user has set "extensions.preciousObjects" can use "git fetch --filter=blob:none origin" to upgrade the repository to use v1 and the partial clone extension. To avoid mistakes, continue to forbid repository format upgrades in v0 repositories with an unrecognized extension. This way, a v0 user using a misspelled extension field gets a chance to correct the mistake before updating to the less forgiving v1 format. While we're here, make the error message for failure to upgrade the repository format a bit shorter, and present it as an error, not a warning. Reported-by: Huan Huan Chen Signed-off-by: Jonathan Nieder Signed-off-by: Junio C Hamano --- cache.h | 1 - setup.c | 12 +++++++----- t/t0410-partial-clone.sh | 4 ++-- 3 files changed, 9 insertions(+), 8 deletions(-) diff --git a/cache.h b/cache.h index e5885cc9ead..0f0485ecfe2 100644 --- a/cache.h +++ b/cache.h @@ -1042,7 +1042,6 @@ struct repository_format { int worktree_config; int is_bare; int hash_algo; - int has_extensions; char *work_tree; struct string_list unknown_extensions; }; diff --git a/setup.c b/setup.c index 597b41b822c..e34b0735779 100644 --- a/setup.c +++ b/setup.c @@ -455,7 +455,6 @@ 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 @@ -553,13 +552,16 @@ int upgrade_repository_format(int target_version) if (repo_fmt.version >= target_version) return 0; - if (verify_repository_format(&repo_fmt, &err) < 0 || - (!repo_fmt.version && repo_fmt.has_extensions)) { - warning("unable to upgrade repository format from %d to %d: %s", - repo_fmt.version, target_version, err.buf); + if (verify_repository_format(&repo_fmt, &err) < 0) { + error("cannot upgrade repository format from %d to %d: %s", + repo_fmt.version, target_version, err.buf); strbuf_release(&err); return -1; } + if (!repo_fmt.version && repo_fmt.unknown_extensions.nr) + return error("cannot upgrade repository format: " + "unknown extension %s", + repo_fmt.unknown_extensions.items[0].string); strbuf_addf(&repo_version, "%d", target_version); git_config_set("core.repositoryformatversion", repo_version.buf); diff --git a/t/t0410-partial-clone.sh b/t/t0410-partial-clone.sh index 51d1eba6050..6aa0f313bdd 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 'converting to partial clone fails with noop extension' ' +test_expect_success 'convert to partial clone with noop extension' ' rm -fr server client && test_create_repo server && test_commit -C server my_commit 1 && @@ -50,7 +50,7 @@ test_expect_success 'converting to partial clone fails with noop extension' ' git clone --depth=1 "file://$(pwd)/server" client && test_cmp_config -C client 0 core.repositoryformatversion && git -C client config extensions.noop true && - test_must_fail git -C client fetch --unshallow --filter="blob:none" + git -C client fetch --unshallow --filter="blob:none" ' test_expect_success 'converting to partial clone fails with unrecognized extension' '