Re: [PATCHv5 2/2] Documentation/clone: document ignored configuration variables

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Jun 16, 2017, 21:10 UTC
Message-ID: <87vanv8wq4.fsf@gmail.com>
In-Reply-To: <20170616204109.GB133952@aiede.mtv.corp.google.com>

On Fri, Jun 16 2017, Jonathan Nieder jotted:

Show 31 quoted lines

> Ævar Arnfjörð Bjarmason wrote:
>> On Fri, Jun 16 2017, SZEDER Gábor jotted:
>
>>> --- a/Documentation/git-clone.txt
>>> +++ b/Documentation/git-clone.txt
>>> @@ -186,6 +186,11 @@ objects from the source repository into a pack in the cloned repository.
>>>  	values are given for the same key, each value will be written to
>>>  	the config file. This makes it safe, for example, to add
>>>  	additional fetch refspecs to the origin remote.
>>> ++
>>> +Due to limitations if the current implementation, some configuration
>>> +variables do not take effect until after the initial fetch and checkout.
>>> +Configuration variables known to not take effect are:
>>> +`remote.<name>.mirror` and `remote.<name>.tagOpt`.
>>>
>>>  --depth <depth>::
>>>  	Create a 'shallow' clone with a history truncated to the
> [...]
>> But this is now cooking in pu, Junio: is it clear that this patchu
>> as-cooking ideally shouldn't land in next/master without the fix on top
>> which I mentioned in my mail above? I can just submit that as a patch on
>> top, but I'm confused about the current state with this cooking in pu,
>> so I thought I'd ask first how this should be handled.
>
> I think it's simplest to write a patch on top that discusses --no-tags.
> That way, Junio (and anyone else applying the patch) has the
> flexibility to apply or cherry-pick this change to old branches
> without the --no-tags discussion and newer branches with it.
>
> Would you like to write it (or suggest wording), or would you prefer
> if someone else does?

I can do that no problem.

I just first wanted to clarify what the status of this was, from SZEDER's comments in the referenced E-Mails I had the impression that this was only meant for an old maintenance release:

    SZEDER: "I assume because, as a bugfix, it will be included in
    maintenance releases for older releases, and those won't have the
    '--no-tags' option."

But thinking about it I don't see why we'd be doing such minor doc changes in an old maintenance release. I haven't read the whole backlog of this topic though, so maybe I missed something.

I initially suggested just adding "Instead supply the --mirror and --no-tags options, respectively" to the patch quoted above.

But actually, thinking about this again now, and being recently familiar with this code after having implemented on --no-tags, I think this whole wording is just misleading. It makes it sound as though git clone is init + fetch, and that the initial fetch just ignores these two specific options because of some quirk of the implementation.

In reality clone doesn't use fetch at all, they just share some of the underlying fetch machinery.

I think something like this as a replacement is better, assuming this really needs to be applied to pre-2.13.0:

    diff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt
    index 35cc34b2fb..2169e5c97f 100644
    --- a/Documentation/git-clone.txt
    +++ b/Documentation/git-clone.txt
    @@ -189,6 +189,14 @@ objects from the source repository into a pack in the cloned repository.
            values are given for the same key, each value will be written to
            the config file. This makes it safe, for example, to add
            additional fetch refspecs to the origin remote.
    ++
    +The underlying implementation of `git clone` isn't equivalent to `git
    +init` followed by a `git fetch`, but the two share some of the
    +underlying fetch machinery. Because of this, setting configuration
    +variables which would impact `git fetch` doesn't have any effect on
    +`git clone` at all. For example, setting `remote.<name>.mirror` and
    +`remote.<name>.tagOpt` will do to change how the initial fetch is
    +carried out.

     --depth <depth>::
            Create a 'shallow' clone with a history truncated to the

Or, in case this just needs to be applied on top of master we can mention --no-tags:

    diff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt
    index 83c8e9b394..52a371176e 100644
    --- a/Documentation/git-clone.txt
    +++ b/Documentation/git-clone.txt
    @@ -189,6 +189,16 @@ objects from the source repository into a pack in the cloned repository.
            values are given for the same key, each value will be written to
            the config file. This makes it safe, for example, to add
            additional fetch refspecs to the origin remote.
    ++
    +The underlying implementation of `git clone` isn't equivalent to `git
    +init` followed by a `git fetch`, but the two share some of the
    +underlying fetch machinery. Because of this, setting configuration
    +variables which would impact `git fetch` doesn't have any effect on
    +`git clone` at all.
    ++
    +For example, setting `remote.<name>.mirror` and `remote.<name>.tagOpt`
    +will do to change how the initial fetch is carried out. Instead supply
    +the --mirror and --no-tags options, respectively.

     --depth <depth>::
            Create a 'shallow' clone with a history truncated to the