git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Christian Couder <christian.couder@gmail.com>
Cc: git@vger.kernel.org, "Nguyen Thai Ngoc Duy" <pclouds@gmail.com>,
	"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>,
	"Christian Couder" <chriscool@tuxfamily.org>
Subject: Re: [PATCH v3 08/21] Documentation/git-update-index: talk about core.splitIndex config var
Date: Tue, 27 Dec 2016 11:07:37 -0800	[thread overview]
Message-ID: <xmqqpokd9ocw.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: 20161226102222.17150-9-chriscool@tuxfamily.org

Christian Couder <christian.couder@gmail.com> writes:

> Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
> ---
>  Documentation/git-update-index.txt | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/Documentation/git-update-index.txt b/Documentation/git-update-index.txt
> index 7386c93162..e091b2a409 100644
> --- a/Documentation/git-update-index.txt
> +++ b/Documentation/git-update-index.txt
> @@ -171,6 +171,12 @@ may not support it yet.
>  	given again, all changes in $GIT_DIR/index are pushed back to
>  	the shared index file. This mode is designed for very large
>  	indexes that take a significant amount of time to read or write.
> ++
> +These options take effect whatever the value of the `core.splitIndex`
> +configuration variable (see linkgit:git-config[1]).

Doesn't the "whatever..." clause in this sentence lack its verb
(presumably, "is", right after "variable")?

> +emitted when the change goes against the configured value, as the
> +configured value will take effect next time the index is read and this
> +will remove the intended effect of the option.

It feels somewhat strange to see a warning in this case. 

If the user configures the system to usually do X, and issues a
command that explicitly does Y (or "not-X"), we do not warn for the
command invocation if it is a single-shot operation.  That's the
usual "command line overrides configuration" an end-user expects.

I think you made this deviate from the usual end-user expectation
because it is unpredictable when the configuration kicks in the next
time to undo the effect of this command line request.  And I agree
that it is a very un-nice thing to do to the users if we did not
warn here, if we are to keep that unpredictableness.

But stepping back a bit, we know the user's intention is that with
"--split-index" the user wants to use the split index, even though
the user may have configuration not to use the split index.  Don't
we want to honor that intention?  In order to honor that intention
without getting confused by the configured variable to tell us not
to split, another way is to destroy that unpredictableness.  For
that, shouldn't we automatically remove the configuration that says
"do not split" (by perhaps set it to "do split")?  Doing so still
may need a warning that says "we disabled your previously configured
setting while at it", but it would be much better than a warning
message that essentially says "we do it once, but we will disobey
you at an unpredictable time", I would think.

  reply	other threads:[~2016-12-27 19:13 UTC|newest]

Thread overview: 56+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-12-26 10:22 [PATCH v3 00/21] Add configuration options for split-index Christian Couder
2016-12-26 10:22 ` [PATCH v3 01/21] config: mark an error message up for translation Christian Couder
2016-12-26 10:22 ` [PATCH v3 02/21] config: add git_config_get_split_index() Christian Couder
2016-12-26 10:22 ` [PATCH v3 03/21] split-index: add {add,remove}_split_index() functions Christian Couder
2016-12-26 10:22 ` [PATCH v3 04/21] read-cache: add and then use tweak_split_index() Christian Couder
2016-12-26 10:22 ` [PATCH v3 05/21] update-index: warn in case of split-index incoherency Christian Couder
2016-12-26 10:22 ` [PATCH v3 06/21] t1700: add tests for core.splitIndex Christian Couder
2016-12-27 19:04   ` Junio C Hamano
2017-01-02  8:29     ` Christian Couder
2017-01-03 12:58       ` Junio C Hamano
2016-12-26 10:22 ` [PATCH v3 07/21] Documentation/config: add information " Christian Couder
2016-12-26 10:22 ` [PATCH v3 08/21] Documentation/git-update-index: talk about core.splitIndex config var Christian Couder
2016-12-27 19:07   ` Junio C Hamano [this message]
2017-01-02  9:39     ` Christian Couder
2017-01-07 21:38       ` Junio C Hamano
2017-01-09 11:18         ` Duy Nguyen
2017-01-23 15:55           ` Christian Couder
2017-01-23 19:59             ` Junio C Hamano
2016-12-26 10:22 ` [PATCH v3 09/21] config: add git_config_get_max_percent_split_change() Christian Couder
2016-12-26 10:22 ` [PATCH v3 10/21] read-cache: regenerate shared index if necessary Christian Couder
2016-12-27 19:08   ` Junio C Hamano
2017-01-02 11:23     ` Christian Couder
2016-12-26 10:22 ` [PATCH v3 11/21] t1700: add tests for splitIndex.maxPercentChange Christian Couder
2016-12-26 10:22 ` [PATCH v3 12/21] Documentation/config: add splitIndex.maxPercentChange Christian Couder
2016-12-27 19:09   ` Junio C Hamano
2017-01-02 13:50     ` Christian Couder
2016-12-26 10:22 ` [PATCH v3 13/21] sha1_file: make check_and_freshen_file() non static Christian Couder
2016-12-27 19:09   ` Junio C Hamano
2016-12-26 10:22 ` [PATCH v3 14/21] read-cache: touch shared index files when used Christian Couder
2016-12-27 19:10   ` Junio C Hamano
2017-01-02 14:09     ` Christian Couder
2017-01-07 21:46       ` Junio C Hamano
2017-01-09 10:55         ` Duy Nguyen
2017-01-09 14:34           ` Junio C Hamano
2017-01-19 12:13             ` Duy Nguyen
2017-01-19 19:00               ` Junio C Hamano
2017-01-20 10:44                 ` Duy Nguyen
2017-01-23 18:14                 ` Christian Couder
2017-01-23 18:53                   ` Junio C Hamano
2017-01-25 14:35                     ` Christian Couder
2017-01-25 20:52                       ` Junio C Hamano
2017-01-31 14:06                         ` Christian Couder
2017-01-31 19:22                           ` Junio C Hamano
2017-01-31 21:09                             ` Junio C Hamano
2016-12-26 10:22 ` [PATCH v3 15/21] config: add git_config_get_expiry() from gc.c Christian Couder
2016-12-26 10:22 ` [PATCH v3 16/21] read-cache: unlink old sharedindex files Christian Couder
2016-12-26 10:22 ` [PATCH v3 17/21] t1700: test shared index file expiration Christian Couder
2016-12-26 10:22 ` [PATCH v3 18/21] read-cache: refactor read_index_from() Christian Couder
2016-12-26 10:22 ` [PATCH v3 19/21] read-cache: use freshen_shared_index() in read_index_from() Christian Couder
2016-12-26 10:22 ` [PATCH v3 20/21] Documentation/config: add splitIndex.sharedIndexExpire Christian Couder
2016-12-27 19:11   ` Junio C Hamano
2017-01-02 14:33     ` Christian Couder
2016-12-26 10:22 ` [PATCH v3 21/21] Documentation/git-update-index: explain splitIndex.* Christian Couder
2016-12-27 19:13   ` Junio C Hamano
2017-01-02 16:04     ` Christian Couder
2016-12-26 10:29 ` [PATCH v3 00/21] Add configuration options for split-index 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=xmqqpokd9ocw.fsf@gitster.mtv.corp.google.com \
    --to=gitster@pobox.com \
    --cc=avarab@gmail.com \
    --cc=chriscool@tuxfamily.org \
    --cc=christian.couder@gmail.com \
    --cc=git@vger.kernel.org \
    --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.
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).