All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Cc: "Christian Couder" <christian.couder@gmail.com>,
	git <git@vger.kernel.org>, "Jeff King" <peff@peff.net>,
	"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 15:03:14 -0800	[thread overview]
Message-ID: <xmqqy4cvco25.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <CACBZZX7QW2J6DcMSXTa1y+QdMrqq5DXs1Fu3m8toV5a4yZKNjw@mail.gmail.com> ("Ævar Arnfjörð Bjarmason"'s message of "Tue, 15 Dec 2015 22:53:09 +0100")

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

> Of course hindsight is 20/20, but I think that given what's been
> covered in this thread it's been established that it's categorically
> better that if we introduce features like these that they be
> configured through the normal configuration facility rather than the
> configuration being sticky to the index.

I doubt that any such thing has been established at all in this
thread.  It may be true that you and perhaps Christian loudly
repeated it, but loudly repeating something and establishing
something as a fact are slightly different.

The thing is, I do not necessarily view this as "configuration".
The way I see the feature is that you say "--untracked" when you
want the states of untracked paths be kept track of in the index,
just like you say "git add Makefile" when you want the state of
'Makefile' be kept track of in the index.  Either the index keeps
track of it, or it doesn't, based solely on user's request, and the
bit to tell us which is the case is already in the index, exactly
because that is part of the data that is kept track of in the index.

> Since the change in performance really isn't noticeable except on
> really large repositories, which are more likely to have someone
> involved watching the changelog on upgrades I think that's OK.

Especially it is dubious to me that the trade-off you are making
with this design is a good one.  In order to avoid paying a one-time
cost to run "update-index --untracked-cache" at sites that _do_ want
to use that feature (and after that, if you teach "git init" and
"git clone" to pay attention to the "give you the default"
configuration to run it for you, so that your users won't have to),
you are forcing all codepaths that makes any write to the index (not
just "init"-time) to make an extra check with the configuration all
the time for everybody, because you made the presence of the
untracked cache data in the index not usable as a sign that the user
wants to use that feature.  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.

  reply	other threads:[~2015-12-15 23:03 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 [this message]
2015-12-16  1:10                     ` Ævar Arnfjörð Bjarmason
2015-12-16  2:46                     ` Jeff King
2015-12-16  5:20                       ` Junio C Hamano
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

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

  git send-email \
    --in-reply-to=xmqqy4cvco25.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.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.