Re: [PATCHv2 1/2] push: change submodule default to check when submodules exist

From: Jeff King <peff@peff.net>
Date: Oct 4, 2016, 19:39 UTC
Message-ID: <20161004193926.32w7yivkakqoadm2@sigill.intra.peff.net>
In-Reply-To: <20161004192910.30649-1-sbeller@google.com>

On Tue, Oct 04, 2016 at 12:29:09PM -0700, Stefan Beller wrote:

Show 10 quoted lines

> Jeff wrote:
> > Consulting .git/config is fine, I think. It's not like we don't read it
> > (sometimes multiple times!) during the normal course of the program
> > anyway. It's just a question of whether it makes more sense for the
> > heuristic to kick in after "init", or only after "update". I don't know
> > enough to have an opinion.
> 
> I think there is no difference in practice, however the "after update"
> is way easier to implement and hence more maintainable (one lstat instead of
> fiddeling with the config; that can go wrong easily).

Hmm, I would have thought it is the opposite; can't submodules exist in the working tree in their own ".git" directory? I know that's the "old" way of doing it, but I didn't know if it was totally deprecated.

Anyway, the config version is probably just:

  int config_check_submodule(const char *var, const char *value, void *data)
  {
	if (starts_with(var, "submodule.") && ends_with(var, ".path"))
		*(int *)data = 1;
	return 0;
  }

  ...
  int have_submodule = 0;
  git_config(config_check_submodule, &have_submodule);

But I don't care too much either way. that's just for reference.

Show 10 quoted lines

> @@ -31,6 +32,19 @@ static const char **refspec;
>  static int refspec_nr;
>  static int refspec_alloc;
>  
> +static void preset_submodule_default(void)
> +{
> +	struct strbuf sb = STRBUF_INIT;
> +	strbuf_addf(&sb, "%s/modules", get_git_dir());
> +
> +	if (file_exists(sb.buf))

Maybe just:

  if (file_exists(git_path("modules"))

-Peff