Re: [PATCH] setup: warn about un-enabled extensions

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Jul 14, 2020, 12:21 UTC
Message-ID: <nycvar.QRO.7.76.6.2007141420300.52@tvgsbejvaqbjf.bet>
In-Reply-To: <0bede821-139a-d805-934a-142004abaa4c@gmail.com>

Hi Stolee,

On Mon, 13 Jul 2020, Derrick Stolee wrote:

Show 42 quoted lines

> On 7/13/2020 5:55 PM, Johannes Schindelin via GitGitGadget wrote:
> > From: Johannes Schindelin <johannes.schindelin@gmx.de>
> >
> > When any `extensions.*` setting is configured, we newly ignore it unless
> > `core.repositoryFormatVersion` is set to a positive value.
> >
> > This might be quite surprising, e.g. when calling `git config --worktree
> > [...]` elicits a warning that it requires
> > `extensions.worktreeConfig = true` when that setting _is_ configured
> > (but ignored because `core.repositoryFormatVersion` is unset).
> >
> > Let's warn about this situation specifically, especially because there
> > might be already setups out there that configured a sparse worktree
> > using Git v2.27.0 (which does set `extensions.worktreeConfig` but not
> > `core.repositoryFormatVersion`) and users might want to work in those
> > setups with Git v2.28.0, too.
> >
> > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> > ---
> >     Warn when extensions.* is ignored
> >
> >     I did actually run into this today. One of my pipelines is configured to
> >     clone a bare repository, then set up a sparse secondary worktree. This
> >     used to work, but all of a sudden, the git config --worktree
> >     core.sparseCheckout true call failed because I'm now using v2.28.0-rc0.
>
> I tried your situation with Junio's patch from earlier [1] [2].
>
> [1] https://lore.kernel.org/git/pull.674.git.1594668051847.gitgitgadget@gmail.com/
> [2] https://lore.kernel.org/git/xmqqpn8zmao1.fsf_-_@gitster.c.googlers.com/
>
> The issue here is that Junio's silent fix for sparse-checkout doesn't
> work here for "git config --worktree". However, I think that Johannes
> is making the same over-compensating warning message pattern as I was.
> That is, this warning happens for all extensions that are enabled when
> core.repositoryFormatVersion is less than 1.
>
> To attempt to summarize Junio's opinion, we should keep our situation
> isolated to this worktree config extension. Your patch does agree with
> the others in that we don't revert the behavior of failing to set the
> config, but I think in this instance we can specify the warning more
> carefully.

Okay.

> If you don't mind, I was already going to squash Junio's commit into
> mine (almost completely replacing mine) but I could add a small
> commit on top that provides the following improvement to the error
> message:

I don't mind at all. I'd just like to know that v2.28.0 avoids confusing users in the same was as v2.28.0-rc0 confused me.

Thanks, Dscho

Show 22 quoted lines

>
> diff --git a/builtin/config.c b/builtin/config.c
> index 5e39f618854..b5de7982a93 100644
> --- a/builtin/config.c
> +++ b/builtin/config.c
> @@ -678,8 +678,9 @@ int cmd_config(int argc, const char **argv, const char *prefix)
>                 else if (worktrees[0] && worktrees[1])
>                         die(_("--worktree cannot be used with multiple "
>                               "working trees unless the config\n"
> -                             "extension worktreeConfig is enabled. "
> -                             "Please read \"CONFIGURATION FILE\"\n"
> +                             "extension worktreeConfig is enabled "
> +                             "and core.repositoryFormatVersion is at least\n"
> +                             "1. Please read \"CONFIGURATION FILE\""
>                               "section in \"git help worktree\" for details"));
>                 else
>                         given_config_source.file = git_pathdup("config");
>
> Thanks,
> -Stolee
>
>