Re: [PATCH] setup: warn if extensions exist on old format

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 13, 2020, 19:36 UTC
Message-ID: <xmqqblkjnr5m.fsf@gitster.c.googlers.com>
In-Reply-To: <pull.674.git.1594668051847.gitgitgadget@gmail.com>

"Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:

Show 14 quoted lines

> From: Derrick Stolee <dstolee@microsoft.com>
>
> Prior to 14c7fa269e4 (check_repository_format_gently(): refuse extensions
> for old repositories, 2020-06-05), Git was honoring configured
> extensions, even if core.repositoryFormatVersion was 0 (or unset). This
> was incorrect, and is now fixed.
>
> 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
> 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
> repo.

s/builting/command/ perhaps???

Show 14 quoted lines

>
> What is worse is that a user will be confronted with the following
> error if they try to run 'git sparse-checkout init' again:
>
> 	warning: unable to upgrade repository format from 0 to 1
>
> 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.

s/ocn/con/

> Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
> ---

Thanks for a thoughtful analysis of the situation and coming up with a plan to remedy.

> +	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'"));
> +	}

That reads well.

An alternative may be to grandfather some extensions that were enabled by git by mistake without updating the format version, and we update the repository even if the repository has extensions that should not exist, but those offending extensions are limited only to those that we decide to special case. That would make the end-user experience even smoother.

Is extenions.worktreeCOnfig the only one that needs this escape hatch?