git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* repositoryformatversion and extensions
@ 2024-03-11 11:54 Edward Thomson
  2024-03-21  9:32 ` Patrick Steinhardt
  0 siblings, 1 reply; 3+ messages in thread
From: Edward Thomson @ 2024-03-11 11:54 UTC (permalink / raw
  To: Git Mailing List

While looking at worktree configuration, I realized that the
extensions.worktreeconfig configuration option was honored even when
core.repositoryformatversion was unset, or explicitly set to 0.

Re-reading the docs for repository-version, it doesn't explicitly state
that extensions _require_ repositoryformatversion >= 1, it instead
states that:

> 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.

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?

Is there documentation on which extensions are supported at which
repository format versions?

Are there other extensions that are not documented in repository-version?

Cheers-
Ed


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: repositoryformatversion and extensions
  2024-03-11 11:54 repositoryformatversion and extensions Edward Thomson
@ 2024-03-21  9:32 ` Patrick Steinhardt
  2024-03-27 10:08   ` Jeff King
  0 siblings, 1 reply; 3+ messages in thread
From: Patrick Steinhardt @ 2024-03-21  9:32 UTC (permalink / raw
  To: Edward Thomson; +Cc: Git Mailing List

[-- Attachment #1: Type: text/plain, Size: 3014 bytes --]

On Mon, Mar 11, 2024 at 11:54:55AM +0000, Edward Thomson wrote:
> While looking at worktree configuration, I realized that the
> extensions.worktreeconfig configuration option was honored even when
> core.repositoryformatversion was unset, or explicitly set to 0.
> 
> Re-reading the docs for repository-version, it doesn't explicitly state
> that extensions _require_ repositoryformatversion >= 1, it instead
> states that:
> 
> > 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.

> 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?

In any case, there are two extensions that make us set version 1 right
now: `objectFormat` and `refStorage`. For the latter it is quite
important for any client to bail out if it's not understood by them so
that we don't accidentally start to write e.g. loose refs in a reftable
repository.

> Is there documentation on which extensions are supported at which
> repository format versions?

Not that I'd know of. Right now, it includes "noop-v1", "objectFormat"
and "refStorage". But I only derive that info from `handle_extension()`
in "setup.c".

> Are there other extensions that are not documented in repository-version?

The only extension I'm aware of that is missing is "noop-v1". That
extension is mostly just used for testing purposes though, so it's of
rather dubious value to document it. On the other hand, the "noop"
extension is documented, as well.

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.

Patrick

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: repositoryformatversion and extensions
  2024-03-21  9:32 ` Patrick Steinhardt
@ 2024-03-27 10:08   ` Jeff King
  0 siblings, 0 replies; 3+ messages in thread
From: Jeff King @ 2024-03-27 10:08 UTC (permalink / raw
  To: Patrick Steinhardt; +Cc: Edward Thomson, Git Mailing List

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@gmail.com/

-Peff


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2024-03-27 10:08 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-03-11 11:54 repositoryformatversion and extensions Edward Thomson
2024-03-21  9:32 ` Patrick Steinhardt
2024-03-27 10:08   ` Jeff King

Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).