On Mon, Jul 13, 2020 at 07:20:51PM +0000, Derrick Stolee via GitGitGadget wrote: > From: Derrick Stolee <dstolee@xxxxxxxxxxxxx> > > 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. > 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 > Signed-off-by: Derrick Stolee <dstolee@xxxxxxxxxxxxx> > --- > 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). > 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. > + 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
