Re: [PATCH] setup: warn if extensions exist on old format
- From
Taylor Blau <me@ttaylorr.com>
- Date
- Jul 13, 2020, 19:34 UTC
- Message-ID
- <20200713193405.GC77607@syl.lan>
- In-Reply-To
- <pull.674.git.1594668051847.gitgitgadget@gmail.com>
On Mon, Jul 13, 2020 at 07:20:51PM +0000, Derrick Stolee via GitGitGadget wrote:
Show 21 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. > > 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
s/upgrae/upgrade
> the version is unset but extensions exist.
I'm not sure that I want to get into a discussion about whether or not this logic is right while -rc0 is already out, but it seems like 14c7fa269e4 could be tweaked to be a little more tolerant of this case.
On the other hand, I think that the approach here is perfectly sensible, absent of a more invasive change to the logic in 14c7fa269e4.
Show 5 quoted lines
> 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/ocnfigured/configured
Show 9 quoted lines
> Signed-off-by: Derrick Stolee <dstolee@microsoft.com> > --- > setup: warn if extensions exist on old format > > This is based on xl/upgrade-repo-format. > > Can this be considered for v2.28.0-rc1? I think this is an important > shift in behavior that will disrupt many users if they upgrade without > it!
I would be happy to see something like this in -rc1, unless Junio has reservations about making this change at this point in the release cycle (I do not have any such concerns).
Show 43 quoted lines
> If not this warning or change, then how else can we help users who are
> in this situation? How can we save those who adopted the sparse-checkout
> builtin in recent versions from terrible post-upgrade headaches?
>
> Thanks, -Stolee
>
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-674%2Fderrickstolee%2Fsparse-checkout-warning-v1
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-674/derrickstolee/sparse-checkout-warning-v1
> Pull-Request: https://github.com/gitgitgadget/git/pull/674
>
> setup.c | 5 +++++
> t/t1091-sparse-checkout-builtin.sh | 9 +++++++++
> 2 files changed, 14 insertions(+)
>
> diff --git a/setup.c b/setup.c
> index eb066db6d8..6ff6c54d39 100644
> --- a/setup.c
> +++ b/setup.c
> @@ -542,6 +542,11 @@ static int check_repository_format_gently(const char *gitdir, struct repository_
> }
> }
>
> + 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;
> }
>
> diff --git a/t/t1091-sparse-checkout-builtin.sh b/t/t1091-sparse-checkout-builtin.sh
> index 88cdde255c..d249428f44 100755
> --- a/t/t1091-sparse-checkout-builtin.sh
> +++ b/t/t1091-sparse-checkout-builtin.sh
> @@ -68,6 +68,15 @@ test_expect_success 'git sparse-checkout init' '
> check_files repo a
> '
>
> +test_expect_success 'warning about core.repositoryFormatVersion' '
> + 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 &&I don't think it's that difficult to see that this patch is correct, but it might be worth testing this for the case that 'core.repositoryFormatVersion' is unset, too.
Show 11 quoted lines
> + git -C repo status 2>err && > + test_i18ngrep "some extensions are enabled, but core.repositoryFormatVersion=0" err > +' > + > test_expect_success 'git sparse-checkout list after init' ' > git -C repo sparse-checkout list >actual && > cat >expect <<-\EOF && > > base-commit: 14c7fa269e42df4133edd9ae7763b678ed6594cd > -- > gitgitgadget
Thanks, Taylor