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

From: SZEDER Gábor <szeder.dev@gmail.com>
Date: May 26, 2017, 14:01 UTC
Message-ID: <CAM0VKjkSMnemwPbFihXiQui3wm_wYmQeQmgYrFs5bfsH1jMg1A@mail.gmail.com>
In-Reply-To: <20170515110557.11913-3-szeder.dev@gmail.com>

On Mon, May 15, 2017 at 1:05 PM, SZEDER Gábor <szeder.dev@gmail.com> wrote:

Show 25 quoted lines

> Due to limitations/bugs in the current implementation, some
> configuration variables specified via 'git clone -c var=val' (or 'git
> -c var=val clone') are ignored during the initial fetch and checkout.
>
> Let the users know which configuration variables are known to be
> ignored ('remote.origin.mirror' and 'remote.origin.tagOpt') under the
> documentation of 'git clone -c'.
>
> Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
> ---
>  Documentation/git-clone.txt | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt
> index ec41d3d69..4f1e7d4ba 100644
> --- a/Documentation/git-clone.txt
> +++ b/Documentation/git-clone.txt
> @@ -186,6 +186,10 @@ 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.
> +       Note that due to limitations of the current implementation some
> +       configuration variables don't take effect during the initial
> +       fetch and checkout.  Configuration variables known to not take
> +       effect are: `remote.<name>.mirror` and `remote.<name>.tagOpt`.

A few notes to this patch, because I didn't like that "known to not take effect" expression, and looked into how some configuration variables are handled during the initial fetch and checkout. Far from comprehensive, but here they are anyway:

Concerning the initial fetch:

 - Configuration variables influencing the refspecs to be fetched are
   currently ignored.  These are the fetch refspecs, of course, and
   remote.<name>.{mirror,tagOpt}.
   This series makes fetch refspecs work.  The other two are mentioned
   in this patch, and are less urgent, because their functionality is
   or will soon be available through command line options (--mirror
   and --no-tags).
 - remote.<name>.url is strange.  It's not just ignored, it's ignored
   so much that it isn't even written to the config file when
   specified as 'git clone -c ...'.  Nonetheless, specifying it for
   'git clone' doesn't make much sense in the first place, does it?
   So I think it's actually good that it's ignored and it isn't worth
   mentioning it in the docs.
 - Some fetch-related config variables, e.g.
   remote.<name>.{prune,skipDefaultUpdate,skipFetchAll} or
   fetch.{prune,output}, don't matter during the initial fetch.
 - Other fetch-related config variables, e.g.
   fetch.{fsckObjects,unpackLimit}, are handled deep down in
   fetch-pack and work properly.
 - Transport-specific config variables, e.g. url.<base>.insteadOf,
   remote.<name>.{proxy,uploadpack,vcs}, or http.*, if applicable, are
   handled in the transport layer or remote helper.
 - I'm not sure about submodule-related config variables, but there
   are command line options for that.

I'm not sure about the initial checkout, in particular I'm not sure how many configuration variables there are that could/should influence the initial checkout (or any checkout, for that matter).

 - core.autocrlf works properly, we even have a test for that.
 - filter.<name>.smudge is read during initial checkout, but I'm not
   sure whether that should do anything, since no attributes files
   exist at that point.
 - I glanced through builtin/checkout.c to see whether it looks at any
   configuration variables that clone doesn't, and while it does so,
   it seems to only look at variables that are either irrelevant
   during the initial checkout, e.g. 'diff.ignoresubmodules' and
   'merge.conflictstyle', or are submodule-specific, and about those I
   have no idea.

Like I said, far from comprehensive, but I think at least the fetch part is well covered.