Re: [PATCH 2/4] clone: call git_config before parse_options

[Sean Barag <sean@barag.org>]

> From: Junio C Hamano <gitster@pobox.com>
> > From: Sean Barag <sean@barag.org>
> >
> > While Junio's request [1] was to avoids the unusual  "write config
> > then immediately read it" pattern that exists in `cmd_clone`,
> > Johannes mentioned that --template can write new config values that
> > aren't automatically included in the environment [2]. This requires
> > a config re-read after `init_db` is called.
> >
> > Moving the initial config up does allow settings from config to be
> > overwritten by ones provided via CLI options in a more natural way
> > though, so that part of Junio's suggestion remains.
>
> The title says what the code does after this change.  The code calls
> git_config() before calling parse_options(), but not much in the
> proposed log message explains what the patch tries to achieve by doing
> so.
>
> The above refers to suggestions but does not describe what problem the
> patch is trying to address and what approach is taken to address it.

Thanks for the pointer - I completely agree.  Rewriting for v2.

> > +static int git_clone_config(const char *k, const char *v, void *cb)
> > +{
> > +	return git_default_config(k, v, cb);
> > +}
> > +
> >  static int write_one_config(const char *key, const char *value, void *data)
> >  {
> > +	/*
> > +	 * give git_config_default a chance to write config values back to the environment, since
> > +	 * git_config_set_multivar_gently only deals with config-file writes
> > +	 */
>
> Overlong lines...

How embarrassing!  Re-wrapped for v2.

> > +	int apply_failed = git_default_config(key, value, data);
>
> Not git_clone_config()?  Presumably you'll make git_clone_config()
> recognise more variables than git_default_config() does, and the
> caller of this helper wants us to recognise "clone.*" that are
> ignored by git_default_config() callback, no?

Great catch!  This gets changed to `git_clone_config` in patch 4/4
anyway, so there's no need for the confusing intermediate state in 2/4.
Will be fixed in v2.

--
Thanks for being so patient with a newbie :)
Sean