git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Jeff King <peff@peff.net>
Cc: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>,
	"Christian Couder" <christian.couder@gmail.com>,
	git <git@vger.kernel.org>,
	"Nguyen Thai Ngoc Duy" <pclouds@gmail.com>,
	"David Turner" <dturner@twopensource.com>,
	"Eric Sunshine" <sunshine@sunshineco.com>,
	"Torsten Bögershausen" <tboegi@web.de>,
	"Christian Couder" <chriscool@tuxfamily.org>
Subject: Re: [PATCH 7/8] config: add core.untrackedCache
Date: Tue, 15 Dec 2015 21:20:25 -0800	[thread overview]
Message-ID: <xmqqwpsfdl5y.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <20151216024605.GA618@sigill.intra.peff.net> (Jeff King's message of "Tue, 15 Dec 2015 21:46:05 -0500")

Jeff King <peff@peff.net> writes:

>> If the feature is something only those with really large repositories
>> care about, is it a good trade-off to make everybody pay the runtime
>> cost and make code more complex and fragile?  I am not yet convinced.
>
> I'm not sure I understand the runtime and complexity costs of the config
> option. Isn't it just:
>
>   if (core_untracked_cache) {
> 	/* do the thing */
>   }
>
> and loading core_untracked_cache in git_default_core_config()? Versus:
>
>   if (the_index.has_untracked_cache) {
>         /* do the thing */
>   }

The latter is pretty much so, but the former needs to be a bit more
involved, e.g. more like:

        if (core_untracked_cache) {
            if (!the_index.has_untracked_cache)
                create_and_attach_uc(&the_index);
            /* do the thing */
        } else {
            if (the_index.has_untracked_cache)
                destroy and remove untracked cache;
        }

Otherwise, after adding the cache to the index, flipping the config
off, doing things with the index and working tree and then filpping
the config back on, the index would have a stale cache that does not
know what happened to the working tree while config was off, and
your "git status" will start throwing a wrong result.

This is why the_index.has_untracked_cache is not just a simple "Do I
want to use this feature?" boolean configuration.  The index also
stores the real data, and "Am I using this feature?" bit goes hand
in hand with that real data.  Thinking that this is merely a boolean
configuration is the real source of the confusion, and introducing a
config that overrules what the user has stored in the index needs to
add complexity.

The additional complexity may (or may not) be justifiable, but in
any case "all other things being equal, this is a config" feels like
a flawed observation.

  reply	other threads:[~2015-12-16  5:20 UTC|newest]

Thread overview: 54+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-12-08 17:15 [PATCH 0/8] Untracked cache improvements Christian Couder
2015-12-08 17:15 ` [PATCH 1/8] update-index: add untracked cache notifications Christian Couder
2015-12-08 19:03   ` Junio C Hamano
2015-12-11  8:51     ` Christian Couder
2015-12-08 17:15 ` [PATCH 2/8] update-index: use enum for untracked cache options Christian Couder
2015-12-08 19:11   ` Junio C Hamano
2015-12-10 10:37     ` Christian Couder
2015-12-10 18:46       ` Junio C Hamano
2015-12-11  9:10         ` Christian Couder
2015-12-11 17:44           ` Junio C Hamano
2015-12-12  9:25             ` Christian Couder
2015-12-08 17:15 ` [PATCH 3/8] update-index: add --test-untracked-cache Christian Couder
2015-12-08 17:15 ` [PATCH 4/8] update-index: move 'uc' var declaration Christian Couder
2015-12-08 17:15 ` [PATCH 5/8] dir: add add_untracked_cache() Christian Couder
2015-12-09  7:37   ` Torsten Bögershausen
2015-12-11  8:54     ` Christian Couder
2015-12-08 17:15 ` [PATCH 6/8] dir: add remove_untracked_cache() Christian Couder
2015-12-08 19:15   ` Junio C Hamano
2015-12-09  7:39   ` Torsten Bögershausen
2015-12-08 17:15 ` [PATCH 7/8] config: add core.untrackedCache Christian Couder
2015-12-08 19:28   ` Junio C Hamano
2015-12-08 22:43     ` Junio C Hamano
2015-12-14 12:18       ` Christian Couder
2015-12-14 19:44         ` Junio C Hamano
2015-12-14 21:30           ` Junio C Hamano
2015-12-15  9:34             ` Christian Couder
2015-12-15  9:49               ` Torsten Bögershausen
2015-12-15 16:42                 ` Christian Couder
2015-12-15 10:02               ` Duy Nguyen
2015-12-15 16:35                 ` Christian Couder
2015-12-15 13:04             ` Ævar Arnfjörð Bjarmason
2015-12-15 13:42               ` Christian Couder
2015-12-15 19:40               ` Junio C Hamano
2015-12-15 21:53                 ` Ævar Arnfjörð Bjarmason
2015-12-15 23:03                   ` Junio C Hamano
2015-12-16  1:10                     ` Ævar Arnfjörð Bjarmason
2015-12-16  2:46                     ` Jeff King
2015-12-16  5:20                       ` Junio C Hamano [this message]
2015-12-16  6:05                         ` Junio C Hamano
2015-12-17  7:44                           ` Jeff King
2015-12-17 12:26                             ` Duy Nguyen
     [not found]                               ` <CAP8UFD0S_rWKjWiq_enkN+QVtvnq9fuwAxuuVTXTxu-F1mw4dg@mail.gmail.com>
2015-12-18 22:40                                 ` Fwd: " Christian Couder
2015-12-21 18:30                               ` Junio C Hamano
2015-12-22  8:27                                 ` Duy Nguyen
2015-12-22 16:33                                   ` Junio C Hamano
2015-12-24  1:56                                     ` Junio C Hamano
2015-12-24  9:49                                       ` Duy Nguyen
2015-12-27 20:21                                         ` Junio C Hamano
2015-12-24 20:54                                       ` Christian Couder
     [not found]                             ` <CAP8UFD0LAQG+gQ5EhYYLjo5=tpW3_ah6GV-mgRbgTjjgNmdorA@mail.gmail.com>
2015-12-18 22:38                               ` Fwd: " Christian Couder
2015-12-17 12:36                   ` Duy Nguyen
2015-12-18 23:24                     ` Christian Couder
2015-12-09 13:19   ` Torsten Bögershausen
2015-12-08 17:15 ` [PATCH 8/8] t7063: add tests for core.untrackedCache Christian Couder

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

  List information: http://vger.kernel.org/majordomo-info.html

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

  git send-email \
    --in-reply-to=xmqqwpsfdl5y.fsf@gitster.mtv.corp.google.com \
    --to=gitster@pobox.com \
    --cc=avarab@gmail.com \
    --cc=chriscool@tuxfamily.org \
    --cc=christian.couder@gmail.com \
    --cc=dturner@twopensource.com \
    --cc=git@vger.kernel.org \
    --cc=pclouds@gmail.com \
    --cc=peff@peff.net \
    --cc=sunshine@sunshineco.com \
    --cc=tboegi@web.de \
    /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.
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).