Subject: Re: [PATCH 13/16] init: allow alternate backends to be set for new repos

From: Jeff King <[email-masked]>
Date: Wed, 23 Dec 2015 15:01:33 -0500

On Wed, Dec 23, 2015 at 10:52:41AM +0100, Michael Haggerty wrote:

> On 12/05/2015 08:44 AM, Jeff King wrote:
> > [...]
> > I think the config option needs to be extensions.refsBackendType, too,
> > per the logic in 00a09d5 (introduce "extensions" form of
> > core.repositoryformatversion, 2015-06-23). And I guess it needs to bump
> > core.repositoryformatversion to "1".
> 
> I think also, strictly speaking, the extensions.refsBackendType option
> should be ignored if core.repositoryFormatVersion is not "1". In
> practice, it probably makes more sense for the code to error out in that
> case because it is likely the result of a bug.

Yeah, I agree. It's not wrong, but it may be a good indication the user
is confused. Perhaps issuing a warning would be appropriate.

> Note that if the user explicitly chooses the "files" backend, it would
> be preferable to leave "core.repositoryFormatVersion" at "0" (assuming
> that no other extension is being used) and leave
> "extensions.refsBackendType" unset. This approach creates a repository
> that is compatible with older clients that don't know about
> refsBackendTypes.

Also agreed. This is going to be the case for basically every
"extension" option. If the extension config key isn't present, there
will be some default backwards-compatible behavior, and for
compatibility, you are always better off turning off the extension than
setting turning it "on" with that default value. This is the case for
the "preciousObjects" extension: there is no point in setting it to
"false" as opposed to removing it.

So I think the rules for setting an extension value should be:

  1. If you're setting it to the default value, then remove it
     completely.

  2. If you're removing it, and there are no other extensions, set
     repositoryFormatVersion back to 0.

Of course there's currently no code that sets extension values. And
doing step 1 is going to be specific to each extension. So this a
general philosophy to follow, not something we would write in a
function.

-Peff