Re: [PATCH v6 4/4] check_repository_format_gently(): refuse extensions for old repositories

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 8, 2020, 16:59 UTC
Message-ID: <xmqqpna9lchh.fsf@gitster.c.googlers.com>
In-Reply-To: <20200605091004.208668-5-delphij@google.com>

Xin Li <delphij@google.com> writes:

Show 13 quoted lines

> 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?