[PATCH] push: change submodule default to check

[Stefan Beller <sbeller@google.com>]

When working with submodules, it is easy to forget to push the submodules.
The setting 'check', which checks if any existing submodule is present on
at least one remote of the submodule remotes, is designed to prevent this
mistake.

Flipping the default to check for submodules is safer than the current
default of ignoring submodules while pushing.

However checking for submodules requires additional work[1], which annoys
users that do not use submodules, so we turn on the check for submodules
based on a cheap heuristic, the existence of the .gitmodules file.

[1] https://public-inbox.org/git/CA+55aFyos78qODyw57V=w13Ux5-8SvBqObJFAq22K+XKPWVbAA@mail.gmail.com/

Signed-off-by: Stefan Beller <sbeller@google.com>
---

On Tue, Oct 4, 2016 at 9:21 AM, Jeff King <peff@peff.net> wrote:
> On Tue, Oct 04, 2016 at 09:19:01AM -0700, Junio C Hamano 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.

I think it is one of the problems that causes serious problems, that is easy
to fix from the side of Git. This patch replaces sb/push-make-submodule-check-the-default
and should be cheap enough for non-submodule users to accept, but still helping
submodule users as it seems to be an ok-ish heuristic. (It is possible to use
submodules and currently have no .gitmodules file present, because you're in
a weird state; then the heuristic fails. By weird state I mean e.g. a bare
repository, or you just checked out an ancient version that has no submodules
yet, or you deleted it locally for whatever reason.)

So how about this patch?

Thanks,
Stefan

 builtin/push.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

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;
+}
+
 static void add_refspec(const char *ref)
 {
 	refspec_nr++;
@@ -552,6 +560,7 @@ int cmd_push(int argc, const char **argv, const char *prefix)
 	};
 
 	packet_trace_identity("push");
+	preset_submodule_default();
 	git_config(git_push_config, &flags);
 	argc = parse_options(argc, argv, prefix, options, push_usage, 0);
 	set_push_cert_flags(&flags, push_cert);
-- 
2.10.0.129.g35f6318