On Thu, Mar 21, 2024 at 10:32:11AM +0100, Patrick Steinhardt wrote:
> > > When reading the core.repositoryformatversion variable, a git
> > > implementation which supports version 1 MUST also read any configuration
> > > keys found in the extensions section of the configuration file.
>
> Yeah, indeed. As far as I understand, the major difference between
> version 1 and version 0 is that version 1 asks the Git client to bail
> out as soon as there is any extension that isn't understood by the
> client. To me this behaviour is somewhat surprising because it doesn't
> allow for in-between states where some extensions are mandatory whereas
> others aren't. It would have been better to have a mechanism that
> disregards the version but makes each extension itself highlight whether
> it is mandatory or not.
The point of bumping "0" to "1" at all was that older versions of Git,
which did not understand extensions at all, would continue to bail. So
yes, I do think it's unfortunate that we do not insist that the version
is set to "1" when parsing extensions, but it is a bug for any writer to
set an extension and _not_ bump the version. Versions of Git older than
v2.6.3 would quietly ignore them.
And you're correct that it does not allow for non-mandatory extensions.
But then, that is the whole point of the extensions.* mechanism: it is
to make sure that versions of Git that do not understand the particular
extension refuse to run. If something isn't mandatory, then it should
not be in extensions.* at all. It should be regular config, with the
knowledge that older versions may ignore it.
At least that was how it was all originally designed. I think later
extensions did not always correctly set the repository version. We tried
to fix that later, but ran into compatibility issues. See 11664196ac
(Revert "check_repository_format_gently(): refuse extensions for old
repositories", 2020-07-15).
So that splits us our extensions into two sets:
- ones that existed at the time of 11664196ac, which are allowed with
v0 for compatibility. These are found in handle_extension_v0() in
Git's code
- everything else that is added should do things correctly, and should
go into handle_extension_v1()
So getting back to Ed's question...
> > Despite that, at least one extension (objectformat) does require
> > core.repositoryformatversion >= 1. Though, do note that objectformat
> > is not mentioned in the repository-version documentation.
> >
> > What other extensions require repository format version >= 1? Is
> > core.objectformat the outlier here?
...the answer is that it's the other way around. Those older extensions
are the outliers. But since we don't add extensions often, for a long
time it was the only one that did things right. ;) And now refstorage
does it right, too.
If we add more extensions, they will also behave that way.
> In any case, the policy of the Git project is that any new extension
> should only ever be understood when the repository format version is >=
> 1 from now on. At least, that's what the code tells us:
>
> ```
> /*
> * Do not add new extensions to this function. It handles extensions which are
> * respected even in v0-format repositories for historical compatibility.
> */
> static enum extension_result handle_extension_v0(const char *var,
> const char *value,
> const char *ext,
> struct repository_format *data)
> ```
>
> So it's probably safe to assume that every new extension should be at
> least v1 from now on.
Right, exactly.
If you really want more gory details, I think the discussion is all in:
https://lore.kernel.org/git/pull.675.git.1594677321039.gitgitgadget@xxxxxxxxx/
-Peff
