git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Stefan Beller <sbeller@google.com>
To: gitster@pobox.com
Cc: git@vger.kernel.org, peff@peff.net, hvoigt@hvoigt.net,
	torvalds@linux-foundation.org, Stefan Beller <sbeller@google.com>
Subject: [PATCHv3 1/2] push: change submodule default to check when submodules exist
Date: Tue,  4 Oct 2016 14:03:58 -0700	[thread overview]
Message-ID: <20161004210359.15266-1-sbeller@google.com> (raw)
In-Reply-To: <CAGZ79kbt+SZoogKTV_-rVfOOFzf6xrhWytrBo2H3r6NQw34WTw@mail.gmail.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 .git/modules directory.
That directory doesn't exist when no submodules are used and is only
created and populated when submodules are cloned/added.

When the submodule directory doesn't exist, a user may have changed the
gitlinks via plumbing commands. Currently the default is to not check.
RECURSE_SUBMODULES_DEFAULT is effectively RECURSE_SUBMODULES_OFF currently,
though it may change in the future. When no submodules exist such a check
is pointless as it would fail anyway, so let's just turn it off.

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

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

Jeff,
thanks for the suggestions, both git_path(..) as well as checking the config,
this seems quite readable to me:

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

diff --git a/builtin/push.c b/builtin/push.c
index 3bb9d6b..65bb1df 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -3,6 +3,7 @@
  */
 #include "cache.h"
 #include "refs.h"
+#include "dir.h"
 #include "run-command.h"
 #include "builtin.h"
 #include "remote.h"
@@ -22,6 +23,7 @@ static int deleterefs;
 static const char *receivepack;
 static int verbosity;
 static int progress = -1;
+static int has_submodules;
 static int recurse_submodules = RECURSE_SUBMODULES_DEFAULT;
 static enum transport_family family;
 
@@ -31,6 +33,14 @@ static const char **refspec;
 static int refspec_nr;
 static int refspec_alloc;
 
+static void preset_submodule_default(void)
+{
+	if (has_submodules || file_exists(git_path("modules")))
+		recurse_submodules = RECURSE_SUBMODULES_CHECK;
+	else
+		recurse_submodules = RECURSE_SUBMODULES_OFF;
+}
+
 static void add_refspec(const char *ref)
 {
 	refspec_nr++;
@@ -495,7 +505,8 @@ static int git_push_config(const char *k, const char *v, void *cb)
 		const char *value;
 		if (!git_config_get_value("push.recursesubmodules", &value))
 			recurse_submodules = parse_push_recurse_submodules_arg(k, value);
-	}
+	} else if (starts_with(k, "submodule.") && ends_with(k, ".url"))
+		has_submodules = 1;
 
 	return git_default_config(k, v, NULL);
 }
@@ -552,6 +563,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


  reply	other threads:[~2016-10-04 21:04 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-10-03 21:11 Slow pushes on 'pu' - even when up-to-date Linus Torvalds
2016-10-03 21:17 ` Stefan Beller
2016-10-03 21:23   ` Stefan Beller
2016-10-03 22:06     ` Junio C Hamano
2016-10-04 11:18 ` Heiko Voigt
2016-10-04 11:44   ` Jeff King
2016-10-04 12:04     ` Heiko Voigt
2016-10-04 12:07       ` Jeff King
2016-10-04 16:19     ` Junio C Hamano
2016-10-04 16:21       ` Jeff King
2016-10-04 16:40         ` [PATCH] push: change submodule default to check Stefan Beller
2016-10-04 17:34           ` Jeff King
2016-10-04 17:48             ` Stefan Beller
2016-10-04 17:54               ` Jeff King
2016-10-04 18:04                 ` Junio C Hamano
2016-10-04 18:08                   ` Stefan Beller
2016-10-04 18:28                     ` Jeff King
2016-10-04 19:29                       ` [PATCHv2 1/2] push: change submodule default to check when submodules exist Stefan Beller
2016-10-04 19:29                         ` [PATCH 2/2] builtin/push: move flags do_push Stefan Beller
2016-10-04 19:39                         ` [PATCHv2 1/2] push: change submodule default to check when submodules exist Jeff King
2016-10-04 19:51                           ` Stefan Beller
2016-10-04 21:03                             ` Stefan Beller [this message]
2016-10-05 13:53                               ` [PATCHv3 " Heiko Voigt
2016-10-06  9:23                                 ` Heiko Voigt
2016-10-06 17:20                                   ` Stefan Beller
2016-10-07 12:41                                     ` Heiko Voigt
2016-10-05 15:47                               ` Jeff King
2016-10-05 15:54                                 ` Stefan Beller
2016-10-04 18:00           ` [PATCH] push: change submodule default to check Junio C Hamano
2016-10-04 18:02             ` Junio C Hamano
2016-10-04 18:05             ` Stefan Beller

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20161004210359.15266-1-sbeller@google.com \
    --to=sbeller@google.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=hvoigt@hvoigt.net \
    --cc=peff@peff.net \
    --cc=torvalds@linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).