On Tue, Nov 01, 2016 at 06:33:38PM -0700, Junio C Hamano wrote:
> Jeff King <p...@peff.net> writes:
>
> > On Fri, Sep 30, 2016 at 05:19:37PM -0700, Junio C Hamano wrote:
> >
> >> Introduce a mechanism, where we estimate the number of objects in
> >> the repository upon the first request to abbreviate an object name
> >> with the default setting and come up with a sane default for the
> >> repository. Based on the expectation that we would see collision in
> >> a repository with 2^(2N) objects when using object names shortened
> >> to first N bits, use sufficient number of hexdigits to cover the
> >> number of objects in the repository. Each hexdigit (4-bits) we add
> >> to the shortened name allows us to have four times (2-bits) as many
> >> objects in the repository.
>
> I was idly browsing the draft release notes and then documentation
> and noticed that, even though the new default is to auto-scale,
> there is no mention of that in the documentation and there is no way
> to explicitly ask for auto-scaling.
>
> I wonder if we want to have something like this. I actually am
> inclined to drop the change to config.c and remove the new mention
> of "auto" in the documentation.
I doubt anybody cares that much either way, but in theory
core.abbrev=auto is a way to override core.abbrev=10 in /etc/gitconfig
or something. Though I'm having trouble envisioning a case where anybody
would set it in /etc/gitconfig, or why somebody would then want to
override that back to auto.
So I think it is fine either way (but I do agree that the core.abbrev
needs _some_ update to mention the auto-scaling behavior).
> diff --git a/config.c b/config.c
> index 83fdecb1bc..c363cca4a9 100644
> --- a/config.c
> +++ b/config.c
> @@ -834,10 +834,16 @@ static int git_default_core_config(const char *var,
> const char *value)
> }
>
> if (!strcmp(var, "core.abbrev")) {
> - int abbrev = git_config_int(var, value);
> - if (abbrev < minimum_abbrev || abbrev > 40)
> - return -1;
> - default_abbrev = abbrev;
> + if (!value)
> + return config_error_nonbool(var);
> + if (!strcasecmp(value, "auto"))
> + default_abbrev = -1;
> + else {
> + int abbrev = git_config_int(var, value);
> + if (abbrev < minimum_abbrev || abbrev > 40)
> + return -1;
> + default_abbrev = abbrev;
> + }
This isn't a new problem you added, but that "return -1" would probably
leave people confused:
$ git -c core.abbrev=2 log
fatal: unable to parse 'core.abbrev' from command-line config
Probably something like:
return error("abbrev length out of range: %d", abbrev);
would be more descriptive. I doubt it's a big deal in practice, though
(why would you set core.abbrev to something silly in the first place?).
-Peff
