[v6,4/4] check_repository_format_gently(): refuse extensions for old repositories
diff mbox series

Message ID 20200605091004.208668-5-delphij@google.com
State New
Headers show
Series
  • fetch: allow adding a filter after initial clone
Related show

Commit Message

Xin Li June 5, 2020, 9:10 a.m. UTC
Previously, extensions were recognized regardless of repository format
version.  If the user sets an undefined "extensions" value on a
repository of version 0 and that value is used by a future git version,
they might get an undesired result.

Because all extensions now also upgrade repository versions, tightening
the check would help avoid this for future extensions.

Signed-off-by: Xin Li <delphij@google.com>
---
 setup.c                  | 12 +++++++++---
 t/t0410-partial-clone.sh | 11 +++++++++++
 2 files changed, 20 insertions(+), 3 deletions(-)

Comments

Junio C Hamano June 8, 2020, 4:59 p.m. UTC | #1
Xin Li <delphij@google.com> writes:

> Previously, extensions were recognized regardless of repository format
> version.  If the user sets an undefined "extensions" value on a
> repository of version 0 and that value is used by a future git version,
> they might get an undesired result.
>
> Because all extensions now also upgrade repository versions, tightening
> the check would help avoid this for future extensions.
>
> Signed-off-by: Xin Li <delphij@google.com>
> ---
>  setup.c                  | 12 +++++++++---
>  t/t0410-partial-clone.sh | 11 +++++++++++
>  2 files changed, 20 insertions(+), 3 deletions(-)

Thanks.

People, I think this is the only bit that is potentially
controversial with the risk of breaking existing set-up.  
Its potentially negative effect can be seen in the patch to
test in the previous step.

     test_expect_success 'enable worktreeConfig extension' '
    +	git config core.repositoryformatversion 1 &&
        git config extensions.worktreeConfig true &&

It used to be that, by mistake, you could just say

    git config extensions.worktreeConfig true

in an existing repository (with extra worktrees) and expect that
alone is enough to let you use per-worktree configuration files.

With this patch, that is no longer true, because the extensions.*
will not take effect unless you upgrade core.repositoryformatversion
to 1.

It is the right thing to do in the longer term (we could even
consider it a bugfix), and the workaround is simple enough.

Unless the user is somehow using a configuration variable whose name
begins with "extensions." for whatever random purpose, that is.  If
the user's setup uses extensions.bar, there is no workaround
possible other than renaming the variable to put it outside
extensions.* hierarchy, and update whatever tool that is using that
variable to use the renamed variable.

Comments?

Patch
diff mbox series

diff --git a/setup.c b/setup.c
index 597b41b822..eb066db6d8 100644
--- a/setup.c
+++ b/setup.c
@@ -507,9 +507,15 @@  static int check_repository_format_gently(const char *gitdir, struct repository_
 		die("%s", err.buf);
 	}
 
-	repository_format_precious_objects = candidate->precious_objects;
-	set_repository_format_partial_clone(candidate->partial_clone);
-	repository_format_worktree_config = candidate->worktree_config;
+	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;
+	}
 	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 16ad000382..463dc3a8be 100755
--- a/t/t0410-partial-clone.sh
+++ b/t/t0410-partial-clone.sh
@@ -42,6 +42,17 @@  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' '
+	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 &&
+	test_must_fail git -C client fetch --unshallow --filter="blob:none"
+'
+
 test_expect_success 'missing reflog object, but promised by a commit, passes fsck' '
 	rm -rf repo &&
 	test_create_repo repo &&