git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: Michael Haggerty <mhagger@alum.mit.edu>
Cc: Duy Nguyen <pclouds@gmail.com>,
	David Turner <dturner@twopensource.com>,
	Git Mailing List <git@vger.kernel.org>
Subject: Re: [PATCH 13/16] init: allow alternate backends to be set for new repos
Date: Wed, 23 Dec 2015 15:01:33 -0500	[thread overview]
Message-ID: <20151223200133.GA19199@sigill.intra.peff.net> (raw)
In-Reply-To: <567A6EE9.3030600@alum.mit.edu>

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

  reply	other threads:[~2015-12-23 20:01 UTC|newest]

Thread overview: 73+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-12-03  0:35 [PATCH 00/16] LMDB refs backend atop pre-vtable David Turner
2015-12-03  0:35 ` [PATCH 01/16] refs: add a backend method structure with transaction functions David Turner
2015-12-05  0:07   ` Junio C Hamano
2015-12-03  0:35 ` [PATCH 02/16] refs: add methods for misc ref operations David Turner
2015-12-11 23:33   ` Junio C Hamano
2015-12-11 23:49     ` David Turner
2015-12-11 23:39   ` Junio C Hamano
2015-12-11 23:49     ` David Turner
2015-12-12  0:23       ` Junio C Hamano
2015-12-12  0:48         ` David Turner
2015-12-18  4:06     ` Howard Chu
2015-12-03  0:35 ` [PATCH 03/16] refs: add methods for the ref iterators David Turner
2016-01-03  0:06   ` David Aguilar
2016-01-04 19:01     ` Junio C Hamano
2016-01-05 13:43       ` Michael Haggerty
2016-01-05 18:56         ` Junio C Hamano
2016-01-04 19:12     ` Ronnie Sahlberg
2016-01-04 20:26       ` Junio C Hamano
2016-01-05  1:17         ` Jeff King
2016-01-05  3:29           ` Junio C Hamano
2015-12-03  0:35 ` [PATCH 04/16] refs: add do_for_each_per_worktree_ref David Turner
2015-12-11 23:52   ` Junio C Hamano
2015-12-12  0:01     ` David Turner
2015-12-03  0:35 ` [PATCH 05/16] refs: add methods for reflog David Turner
2015-12-03  0:35 ` [PATCH 06/16] refs: add method for initial ref transaction commit David Turner
2015-12-03  0:35 ` [PATCH 07/16] refs: add method for delete_refs David Turner
2015-12-03  0:35 ` [PATCH 08/16] refs: add methods to init refs backend and db David Turner
2015-12-23  5:33   ` Michael Haggerty
2015-12-23  6:54     ` David Turner
2015-12-03  0:35 ` [PATCH 09/16] refs: add method to rename refs David Turner
2015-12-03  0:35 ` [PATCH 10/16] refs: make lock generic David Turner
2015-12-03  0:35 ` [PATCH 11/16] refs: move duplicate check to common code David Turner
2015-12-23  6:27   ` Michael Haggerty
2016-01-05 16:42     ` David Turner
2015-12-03  0:35 ` [PATCH 12/16] refs: always handle non-normal refs in files backend David Turner
2015-12-23  8:02   ` Michael Haggerty
2016-01-06  0:13     ` David Turner
2016-01-06 23:41     ` [PATCH/RFC v2 1/3] refs: allow log-only updates David Turner
2016-01-06 23:41       ` [PATCH/RFC v2 2/3] refs: resolve symbolic refs first David Turner
2016-01-06 23:41       ` [PATCH/RFC v2 3/3] refs: always handle non-normal refs in files backend David Turner
2016-01-08 12:52         ` David Turner
2016-01-06 23:42     ` [PATCH 12/16] " David Turner
2015-12-03  0:35 ` [PATCH 13/16] init: allow alternate backends to be set for new repos David Turner
2015-12-05  0:07   ` Junio C Hamano
2015-12-05  6:30   ` Duy Nguyen
2015-12-05  7:44     ` Jeff King
2015-12-08  0:38       ` David Turner
2015-12-23  9:52       ` Michael Haggerty
2015-12-23 20:01         ` Jeff King [this message]
2015-12-10 18:02   ` Jeff King
2015-12-10 19:36     ` David Turner
2015-12-23 11:30   ` [PATCH] clone: use child_process for recursive checkouts Michael Haggerty
2016-01-06 23:41     ` David Turner
2015-12-23 13:34   ` [PATCH 13/16] init: allow alternate backends to be set for new repos Michael Haggerty
2016-01-05 17:26     ` David Turner
2016-01-05 18:03       ` Junio C Hamano
2016-01-05 18:24         ` David Turner
2016-01-06 12:02         ` Michael Haggerty
2016-01-06 12:52     ` Duy Nguyen
2016-01-07  3:31       ` Shawn Pearce
2015-12-03  0:35 ` [PATCH 14/16] refs: allow ref backend to be set for clone David Turner
2015-12-23 13:51   ` Michael Haggerty
2015-12-23 20:23     ` Eric Sunshine
2015-12-03  0:35 ` [PATCH 15/16] refs: add LMDB refs backend David Turner
2015-12-05  0:08   ` Junio C Hamano
2015-12-05  0:25     ` David Turner
2015-12-17  1:00   ` Jonathan Nieder
2015-12-17  2:31     ` David Turner
2015-12-17 20:49       ` Jonathan Nieder
2015-12-23 14:32   ` Michael Haggerty
2016-01-08 16:05     ` David Turner
2015-12-03  0:35 ` [PATCH 16/16] refs: tests for lmdb backend David Turner
2015-12-22 23:56 ` [PATCH 00/16] LMDB refs backend atop pre-vtable David Turner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20151223200133.GA19199@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=dturner@twopensource.com \
    --cc=git@vger.kernel.org \
    --cc=mhagger@alum.mit.edu \
    --cc=pclouds@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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 NNTP newsgroup(s).