Re: [PATCH v2] clone: respect configured fetch respecs during initial fetch

From: SZEDER Gábor <szeder@ira.uka.de>
Date: Apr 1, 2016, 04:20 UTC
Message-ID: <20160401062001.Horde.ONOWb-Prl7DzJnJKcoFoctm@webmail.informatik.kit.edu>
In-Reply-To: <xmqqbn5ul12t.fsf@gitster.mtv.corp.google.com>

Quoting Junio C Hamano <gitster@pobox.com>:

Show 21 quoted lines

> SZEDER Gábor <szeder@ira.uka.de> writes:
>
>> Quoting Junio C Hamano <gitster@pobox.com>:
>>
>>> IOW, special casing -c remote.origin.fetch=spec
>>> is a bad idea.
>>
>> I completely agree :)
>> But it's the other way around.
>>
>> 'remote.origin.fetch=spec' during clone is special _now_, because the
>> initial fetch ignores it, no matter where it is set.
>>
>> My patch makes it non-special, so that the initial fetch respects it,
>> the same way it already respects 'fetch.fsckObjects' and
>> 'fsck.unpackLimit', or the way the initial checkout respects e.g.
>> 'core.eol'.
>
> ... but does "git -c core.eol clone" leave that configuration in the
> resulting repository's .git/config?  "git -c user.name=foo" for that
> matter.

No, and it shouldn't.

'git clone -c core.eol', however, should and indeed does.

Show 21 quoted lines

> They may affect the one-shot operation but are not left in the
> resulting .git/config, which was what I was driving at.  To make
> clone behave as if it is truly a short-hand of
>
> 	git init
> 	git config ;# with default and necessary tweaks to adjust
> 		   ;# for things like -b, -o, --single-branch
>        git fetch
>        git checkout
>
> which by the way I think everybody agrees is a worth goal, then
> shouldn't the update to the current code be more like "prepare the
> default config, tweak with whatever configuration necessary, and
> re-read the config before driving the equivalent of 'git fetch'?"
>
> And the conclusion my rhetorical questions led to was that "adjust
> for things like..." should not include what comes from "-c var=val"
> because there is no sensible way to incorporate them in general.
>
> The most important point is that "-c var=val" is the wrong source of
> information to blindly propagete to the resulting .git/config.

In case of 'git -c var=val clone', I agree, but then again, 'git clone -c var=val' was specifically designed to write that 'var=val' to the new repository's config file.

Config variables set in the global or system-wide config files are not written to the new repository's config file either.

Show 12 quoted lines

> And
> the point of "--branches" option is not that it would be short and
> tidy, but it is more targetted.  With such an approach, nobody would
> imagine "git -c random.var=value clone" would be propagated into the
> resulting .git/config in a random and unspecified way.
>
> Once you learn what custom set of refs the user wants to fetch, you
> would need futzing of the refspecs like you did in your patch. That
> part of your patch is salvageable.  The part that special cased the
> information that came from "-c remote.origin.fetch" while ignoring
> others like user.name that came from exactly the same mechanism via
> "-c user.name" is not.

My patch did not special case anything and it did not change anything with respect to what config settings are written under what circumstances to the new repository's config file. All that already works properly and almost all those config settings are already taken into account when they should be by the commands in our conceptual model of 'git clone'. It doesn't matter at all where and how they were specified or whether they are written to the new repository's config file or not, almost all of them are taken into account.

Almost all, because there is that one exception: additional fetch refspecs are ignored during the initial fetch.

And all my patch did was to make the initial fetch aware of any additional fetch refspecs which are configured when the initial fetch is executed. (and again: no matter where those refspecs were specified and no matter whether they were written to the new config file)

Eh, I guess I should have went for a refined version of the RFC's commit message, rewriting it based on that conceptual modell caused way too much confusion.