Re: [PATCH] push: change submodule default to check

[Jeff King <peff@peff.net>]

On Tue, Oct 04, 2016 at 09:40:36AM -0700, Stefan Beller wrote:

> >> Why should we even have a default different from today's?  If most
> >> repositories don't have submodules enabled at all, we can just let
> >> those working with submodules enabled to toggle their configuration
> >> and that is an very easy to understand solution, no?
> >
> > You will not see any complaint from me on that. I was taking for granted
> > that the current default is inconvenient to submodule users, but I don't
> > have any experience myself.
> >
> 
> And there I was trying to help submodule users not shoot in their foot.

Sorry if my reply came off as snarky. I really did mean it literally. I
do not know if the end goal is good or not, so all of my discussion was
just assuming it was.

So in that vein...

> diff --git a/builtin/push.c b/builtin/push.c
> index 3bb9d6b..d7d664a 100644
> --- a/builtin/push.c
> +++ b/builtin/push.c
> @@ -22,7 +22,7 @@ static int deleterefs;
>  static const char *receivepack;
>  static int verbosity;
>  static int progress = -1;
> -static int recurse_submodules = RECURSE_SUBMODULES_DEFAULT;
> +static int recurse_submodules;
>  static enum transport_family family;
>  
>  static struct push_cas_option cas;
> @@ -31,6 +31,14 @@ static const char **refspec;
>  static int refspec_nr;
>  static int refspec_alloc;
>  
> +static void preset_submodule_default(void)
> +{
> +	if (file_exists(".gitmodules"))
> +		recurse_submodules = RECURSE_SUBMODULES_CHECK;
> +	else
> +		recurse_submodules = RECURSE_SUBMODULES_DEFAULT;
> +}

This does seem like a reasonable heuristic. I wonder if you want to
confirm that we actually have a worktree (and are in it) before looking
at file_exists(). It's unlikely that looking at ".gitmodules" in a bare
repo would trigger in practice, but it does not hurt to be careful.

-Peff