git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* [PATCHv2 0/8] A reroll of sb/submodule-blanket-recursive
@ 2017-05-26 19:10 Stefan Beller
  2017-05-26 19:10 ` [PATCH 1/8] submodule recursing: do not write a config variable twice Stefan Beller
                   ` (8 more replies)
  0 siblings, 9 replies; 15+ messages in thread
From: Stefan Beller @ 2017-05-26 19:10 UTC (permalink / raw)
  To: bmwill; +Cc: git, gitster, Stefan Beller

v2:
* A reroll of sb/submodule-blanket-recursive.
* This requires ab/grep-preparatory-cleanup 
* It changed a lot from v1, as in v1 the tests did not work,
  hence the code was broken. Now it actually works.
* it also includes grep, fetch, push in addition to plain working tree
  manipulators.

Thanks,
Stefan

Stefan Beller (8):
  submodule recursing: do not write a config variable twice
  submodule test invocation: only pass additional arguments
  reset/checkout/read-tree: unify config callback for submodule
    recursion
  submodule loading: separate code path for .gitmodules and config
    overlay
  Introduce 'submodule.recurse' option for worktree manipulators
  builtin/grep.c: respect 'submodule.recurse' option
  builtin/push.c: respect 'submodule.recurse' option
  builtin/fetch.c: respect 'submodule.recurse' option

 Documentation/config.txt           |  5 +++
 builtin/checkout.c                 | 31 ++----------------
 builtin/fetch.c                    |  7 +++++
 builtin/grep.c                     |  3 ++
 builtin/push.c                     |  4 +++
 builtin/read-tree.c                | 32 ++++++-------------
 builtin/reset.c                    | 39 +++++++----------------
 submodule.c                        | 64 +++++++++++++++++++++++++++++++++-----
 submodule.h                        |  7 ++++-
 t/lib-submodule-update.sh          | 22 ++++++++++---
 t/t1013-read-tree-submodule.sh     |  4 +--
 t/t2013-checkout-submodule.sh      |  4 +--
 t/t5526-fetch-submodules.sh        | 10 ++++++
 t/t5531-deep-submodule-push.sh     | 21 +++++++++++++
 t/t7112-reset-submodule.sh         |  4 +--
 t/t7814-grep-recurse-submodules.sh | 18 +++++++++++
 16 files changed, 178 insertions(+), 97 deletions(-)

-- 
2.13.0.17.g582985b1e4


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 1/8] submodule recursing: do not write a config variable twice
  2017-05-26 19:10 [PATCHv2 0/8] A reroll of sb/submodule-blanket-recursive Stefan Beller
@ 2017-05-26 19:10 ` Stefan Beller
  2017-05-26 19:10 ` [PATCH 2/8] submodule test invocation: only pass additional arguments Stefan Beller
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 15+ messages in thread
From: Stefan Beller @ 2017-05-26 19:10 UTC (permalink / raw)
  To: bmwill; +Cc: git, gitster, Stefan Beller

The command line option for '--recurse-submodules' is implemented
using an OPTION_CALLBACK, which takes both the callback (that sets
the file static global variable) as well as passes the same file
static global variable to the option parsing machinery to assign it.
This is fixed in this commit by passing NULL as the variable. The
callback sets it instead

Signed-off-by: Stefan Beller <sbeller@google.com>
---
 builtin/checkout.c  | 2 +-
 builtin/read-tree.c | 2 +-
 builtin/reset.c     | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/builtin/checkout.c b/builtin/checkout.c
index bfa5419f33..0fd57672cc 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -1181,7 +1181,7 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
 				N_("second guess 'git checkout <no-such-branch>'")),
 		OPT_BOOL(0, "ignore-other-worktrees", &opts.ignore_other_worktrees,
 			 N_("do not check if another worktree is holding the given ref")),
-		{ OPTION_CALLBACK, 0, "recurse-submodules", &recurse_submodules,
+		{ OPTION_CALLBACK, 0, "recurse-submodules", NULL,
 			    "checkout", "control recursive updating of submodules",
 			    PARSE_OPT_OPTARG, option_parse_recurse_submodules },
 		OPT_BOOL(0, "progress", &opts.show_progress, N_("force progress reporting")),
diff --git a/builtin/read-tree.c b/builtin/read-tree.c
index 23e212ee8c..2a1b8a530e 100644
--- a/builtin/read-tree.c
+++ b/builtin/read-tree.c
@@ -157,7 +157,7 @@ int cmd_read_tree(int argc, const char **argv, const char *unused_prefix)
 			 N_("skip applying sparse checkout filter")),
 		OPT_BOOL(0, "debug-unpack", &opts.debug_unpack,
 			 N_("debug unpack-trees")),
-		{ OPTION_CALLBACK, 0, "recurse-submodules", &recurse_submodules,
+		{ OPTION_CALLBACK, 0, "recurse-submodules", NULL,
 			    "checkout", "control recursive updating of submodules",
 			    PARSE_OPT_OPTARG, option_parse_recurse_submodules },
 		OPT_END()
diff --git a/builtin/reset.c b/builtin/reset.c
index 5ce27fcaed..1e5f85b1fb 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -304,7 +304,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
 				N_("reset HEAD, index and working tree"), MERGE),
 		OPT_SET_INT(0, "keep", &reset_type,
 				N_("reset HEAD but keep local changes"), KEEP),
-		{ OPTION_CALLBACK, 0, "recurse-submodules", &recurse_submodules,
+		{ OPTION_CALLBACK, 0, "recurse-submodules", NULL,
 			    "reset", "control recursive updating of submodules",
 			    PARSE_OPT_OPTARG, option_parse_recurse_submodules },
 		OPT_BOOL('p', "patch", &patch_mode, N_("select hunks interactively")),
-- 
2.13.0.17.g582985b1e4


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 2/8] submodule test invocation: only pass additional arguments
  2017-05-26 19:10 [PATCHv2 0/8] A reroll of sb/submodule-blanket-recursive Stefan Beller
  2017-05-26 19:10 ` [PATCH 1/8] submodule recursing: do not write a config variable twice Stefan Beller
@ 2017-05-26 19:10 ` Stefan Beller
  2017-05-26 19:10 ` [PATCH 3/8] reset/checkout/read-tree: unify config callback for submodule recursion Stefan Beller
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 15+ messages in thread
From: Stefan Beller @ 2017-05-26 19:10 UTC (permalink / raw)
  To: bmwill; +Cc: git, gitster, Stefan Beller

In a later patch we want to introduce a config option to trigger the
submodule recursing by default. As this option should be available and
uniform across all commands that deal with submodules we'd want to test
for this option in the submodule update library.

So instead of calling the whole test set again for
"git -c submodule.recurse foo" instead of "git foo --recurse-submodules",
we'd only want to introduce one basic test that tests if the option is
recognized and respected to not overload the test suite.

Change the test functions by taking only the argument and assemble the
command inside the test function by embedding the arguments into the
command that is "git $arguments --recurse-submodules".

It would be nice to do this for all functions in lib-submodule-update,
but we cannot do that for the non-recursing tests, as there we do not
just pass in a git command but whole functions. (See t3426 for example)

Signed-off-by: Stefan Beller <sbeller@google.com>
---
 t/lib-submodule-update.sh      | 10 ++++++----
 t/t1013-read-tree-submodule.sh |  4 ++--
 t/t2013-checkout-submodule.sh  |  4 ++--
 t/t7112-reset-submodule.sh     |  4 ++--
 4 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/t/lib-submodule-update.sh b/t/lib-submodule-update.sh
index f0b1b18206..0272c4d8ca 100755
--- a/t/lib-submodule-update.sh
+++ b/t/lib-submodule-update.sh
@@ -781,8 +781,9 @@ test_submodule_forced_switch () {
 # - Removing a submodule with a git directory absorbs the submodules
 #   git directory first into the superproject.
 
-test_submodule_switch_recursing () {
-	command="$1"
+test_submodule_switch_recursing_with_args () {
+	cmd_args="$1"
+	command="git $cmd_args --recurse-submodules"
 	RESULTDS=success
 	if test "$KNOWN_FAILURE_DIRECTORY_SUBMODULE_CONFLICTS" = 1
 	then
@@ -1021,8 +1022,9 @@ test_submodule_switch_recursing () {
 # Test that submodule contents are updated when switching between commits
 # that change a submodule, but throwing away local changes in
 # the superproject as well as the submodule is allowed.
-test_submodule_forced_switch_recursing () {
-	command="$1"
+test_submodule_forced_switch_recursing_with_args () {
+	cmd_args="$1"
+	command="git $cmd_args --recurse-submodules"
 	RESULT=success
 	if test "$KNOWN_FAILURE_DIRECTORY_SUBMODULE_CONFLICTS" = 1
 	then
diff --git a/t/t1013-read-tree-submodule.sh b/t/t1013-read-tree-submodule.sh
index de1ba02dc5..2c8d620324 100755
--- a/t/t1013-read-tree-submodule.sh
+++ b/t/t1013-read-tree-submodule.sh
@@ -9,9 +9,9 @@ KNOWN_FAILURE_SUBMODULE_RECURSIVE_NESTED=1
 KNOWN_FAILURE_DIRECTORY_SUBMODULE_CONFLICTS=1
 KNOWN_FAILURE_SUBMODULE_OVERWRITE_IGNORED_UNTRACKED=1
 
-test_submodule_switch_recursing "git read-tree --recurse-submodules -u -m"
+test_submodule_switch_recursing_with_args "read-tree -u -m"
 
-test_submodule_forced_switch_recursing "git read-tree --recurse-submodules -u --reset"
+test_submodule_forced_switch_recursing_with_args "read-tree -u --reset"
 
 test_submodule_switch "git read-tree -u -m"
 
diff --git a/t/t2013-checkout-submodule.sh b/t/t2013-checkout-submodule.sh
index e8f70b806f..c962a02277 100755
--- a/t/t2013-checkout-submodule.sh
+++ b/t/t2013-checkout-submodule.sh
@@ -65,9 +65,9 @@ test_expect_success '"checkout <submodule>" honors submodule.*.ignore from .git/
 
 KNOWN_FAILURE_DIRECTORY_SUBMODULE_CONFLICTS=1
 KNOWN_FAILURE_SUBMODULE_RECURSIVE_NESTED=1
-test_submodule_switch_recursing "git checkout --recurse-submodules"
+test_submodule_switch_recursing_with_args "checkout"
 
-test_submodule_forced_switch_recursing "git checkout -f --recurse-submodules"
+test_submodule_forced_switch_recursing_with_args "checkout -f"
 
 test_submodule_switch "git checkout"
 
diff --git a/t/t7112-reset-submodule.sh b/t/t7112-reset-submodule.sh
index f86ccdf215..a1cb9ff858 100755
--- a/t/t7112-reset-submodule.sh
+++ b/t/t7112-reset-submodule.sh
@@ -9,9 +9,9 @@ KNOWN_FAILURE_SUBMODULE_RECURSIVE_NESTED=1
 KNOWN_FAILURE_DIRECTORY_SUBMODULE_CONFLICTS=1
 KNOWN_FAILURE_SUBMODULE_OVERWRITE_IGNORED_UNTRACKED=1
 
-test_submodule_switch_recursing "git reset --recurse-submodules --keep"
+test_submodule_switch_recursing_with_args "reset --keep"
 
-test_submodule_forced_switch_recursing "git reset --hard --recurse-submodules"
+test_submodule_forced_switch_recursing_with_args "reset --hard"
 
 test_submodule_switch "git reset --keep"
 
-- 
2.13.0.17.g582985b1e4


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 3/8] reset/checkout/read-tree: unify config callback for submodule recursion
  2017-05-26 19:10 [PATCHv2 0/8] A reroll of sb/submodule-blanket-recursive Stefan Beller
  2017-05-26 19:10 ` [PATCH 1/8] submodule recursing: do not write a config variable twice Stefan Beller
  2017-05-26 19:10 ` [PATCH 2/8] submodule test invocation: only pass additional arguments Stefan Beller
@ 2017-05-26 19:10 ` Stefan Beller
  2017-05-26 19:10 ` [PATCH 4/8] submodule loading: separate code path for .gitmodules and config overlay Stefan Beller
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 15+ messages in thread
From: Stefan Beller @ 2017-05-26 19:10 UTC (permalink / raw)
  To: bmwill; +Cc: git, gitster, Stefan Beller

The callback function is essentially duplicated 3 times. Remove all
of them and offer a new callback function, that lives in submodule.c

By putting the callback function there, we no longer need the function
'set_config_update_recurse_submodules', nor duplicate the global variable
in each builtin as well as submodule.c

In the three builtins we have different 2 ways how to load the .gitmodules
and config file, which are slightly different. git-checkout has to load
the submodule config all the time due to 23b4c7bcc5 (checkout: Use
submodule.*.ignore settings from .git/config and .gitmodules, 2010-08-28)

git-reset and git-read-tree do not respect these diff settings, so loading
the submodule configuration is optional. Also put that into submodule.c
for code deduplication.

Signed-off-by: Stefan Beller <sbeller@google.com>
---
 builtin/checkout.c  | 27 +--------------------------
 builtin/read-tree.c | 28 +++-------------------------
 builtin/reset.c     | 27 ++-------------------------
 submodule.c         | 33 +++++++++++++++++++++++++++------
 submodule.h         |  6 +++++-
 5 files changed, 38 insertions(+), 83 deletions(-)

diff --git a/builtin/checkout.c b/builtin/checkout.c
index 0fd57672cc..acff6039d6 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -21,31 +21,12 @@
 #include "submodule-config.h"
 #include "submodule.h"
 
-static int recurse_submodules = RECURSE_SUBMODULES_DEFAULT;
-
 static const char * const checkout_usage[] = {
 	N_("git checkout [<options>] <branch>"),
 	N_("git checkout [<options>] [<branch>] -- <file>..."),
 	NULL,
 };
 
-static int option_parse_recurse_submodules(const struct option *opt,
-					   const char *arg, int unset)
-{
-	if (unset) {
-		recurse_submodules = RECURSE_SUBMODULES_OFF;
-		return 0;
-	}
-	if (arg)
-		recurse_submodules =
-			parse_update_recurse_submodules_arg(opt->long_name,
-							    arg);
-	else
-		recurse_submodules = RECURSE_SUBMODULES_ON;
-
-	return 0;
-}
-
 struct checkout_opts {
 	int patch_mode;
 	int quiet;
@@ -1183,7 +1164,7 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
 			 N_("do not check if another worktree is holding the given ref")),
 		{ OPTION_CALLBACK, 0, "recurse-submodules", NULL,
 			    "checkout", "control recursive updating of submodules",
-			    PARSE_OPT_OPTARG, option_parse_recurse_submodules },
+			    PARSE_OPT_OPTARG, option_parse_recurse_submodules_worktree_updater },
 		OPT_BOOL(0, "progress", &opts.show_progress, N_("force progress reporting")),
 		OPT_END(),
 	};
@@ -1214,12 +1195,6 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
 		git_xmerge_config("merge.conflictstyle", conflict_style, NULL);
 	}
 
-	if (recurse_submodules != RECURSE_SUBMODULES_OFF) {
-		git_config(submodule_config, NULL);
-		if (recurse_submodules != RECURSE_SUBMODULES_DEFAULT)
-			set_config_update_recurse_submodules(recurse_submodules);
-	}
-
 	if ((!!opts.new_branch + !!opts.new_branch_force + !!opts.new_orphan_branch) > 1)
 		die(_("-b, -B and --orphan are mutually exclusive"));
 
diff --git a/builtin/read-tree.c b/builtin/read-tree.c
index 2a1b8a530e..8a889ef4c3 100644
--- a/builtin/read-tree.c
+++ b/builtin/read-tree.c
@@ -21,7 +21,6 @@
 static int nr_trees;
 static int read_empty;
 static struct tree *trees[MAX_UNPACK_TREES];
-static int recurse_submodules = RECURSE_SUBMODULES_DEFAULT;
 
 static int list_tree(unsigned char *sha1)
 {
@@ -99,23 +98,6 @@ static int debug_merge(const struct cache_entry * const *stages,
 	return 0;
 }
 
-static int option_parse_recurse_submodules(const struct option *opt,
-					   const char *arg, int unset)
-{
-	if (unset) {
-		recurse_submodules = RECURSE_SUBMODULES_OFF;
-		return 0;
-	}
-	if (arg)
-		recurse_submodules =
-			parse_update_recurse_submodules_arg(opt->long_name,
-							    arg);
-	else
-		recurse_submodules = RECURSE_SUBMODULES_ON;
-
-	return 0;
-}
-
 static struct lock_file lock_file;
 
 int cmd_read_tree(int argc, const char **argv, const char *unused_prefix)
@@ -159,7 +141,7 @@ int cmd_read_tree(int argc, const char **argv, const char *unused_prefix)
 			 N_("debug unpack-trees")),
 		{ OPTION_CALLBACK, 0, "recurse-submodules", NULL,
 			    "checkout", "control recursive updating of submodules",
-			    PARSE_OPT_OPTARG, option_parse_recurse_submodules },
+			    PARSE_OPT_OPTARG, option_parse_recurse_submodules_worktree_updater },
 		OPT_END()
 	};
 
@@ -173,13 +155,9 @@ int cmd_read_tree(int argc, const char **argv, const char *unused_prefix)
 	argc = parse_options(argc, argv, unused_prefix, read_tree_options,
 			     read_tree_usage, 0);
 
-	hold_locked_index(&lock_file, LOCK_DIE_ON_ERROR);
+	load_submodule_cache();
 
-	if (recurse_submodules != RECURSE_SUBMODULES_DEFAULT) {
-		gitmodules_config();
-		git_config(submodule_config, NULL);
-		set_config_update_recurse_submodules(RECURSE_SUBMODULES_ON);
-	}
+	hold_locked_index(&lock_file, LOCK_DIE_ON_ERROR);
 
 	prefix_set = opts.prefix ? 1 : 0;
 	if (1 < opts.merge + opts.reset + prefix_set)
diff --git a/builtin/reset.c b/builtin/reset.c
index 1e5f85b1fb..6f89dc5494 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -24,25 +24,6 @@
 #include "submodule.h"
 #include "submodule-config.h"
 
-static int recurse_submodules = RECURSE_SUBMODULES_DEFAULT;
-
-static int option_parse_recurse_submodules(const struct option *opt,
-					   const char *arg, int unset)
-{
-	if (unset) {
-		recurse_submodules = RECURSE_SUBMODULES_OFF;
-		return 0;
-	}
-	if (arg)
-		recurse_submodules =
-			parse_update_recurse_submodules_arg(opt->long_name,
-							    arg);
-	else
-		recurse_submodules = RECURSE_SUBMODULES_ON;
-
-	return 0;
-}
-
 static const char * const git_reset_usage[] = {
 	N_("git reset [--mixed | --soft | --hard | --merge | --keep] [-q] [<commit>]"),
 	N_("git reset [-q] [<tree-ish>] [--] <paths>..."),
@@ -306,7 +287,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
 				N_("reset HEAD but keep local changes"), KEEP),
 		{ OPTION_CALLBACK, 0, "recurse-submodules", NULL,
 			    "reset", "control recursive updating of submodules",
-			    PARSE_OPT_OPTARG, option_parse_recurse_submodules },
+			    PARSE_OPT_OPTARG, option_parse_recurse_submodules_worktree_updater },
 		OPT_BOOL('p', "patch", &patch_mode, N_("select hunks interactively")),
 		OPT_BOOL('N', "intent-to-add", &intent_to_add,
 				N_("record only the fact that removed paths will be added later")),
@@ -319,11 +300,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
 						PARSE_OPT_KEEP_DASHDASH);
 	parse_args(&pathspec, argv, prefix, patch_mode, &rev);
 
-	if (recurse_submodules != RECURSE_SUBMODULES_DEFAULT) {
-		gitmodules_config();
-		git_config(submodule_config, NULL);
-		set_config_update_recurse_submodules(RECURSE_SUBMODULES_ON);
-	}
+	load_submodule_cache();
 
 	unborn = !strcmp(rev, "HEAD") && get_sha1("HEAD", oid.hash);
 	if (unborn) {
diff --git a/submodule.c b/submodule.c
index 54825100b2..c9e764b519 100644
--- a/submodule.c
+++ b/submodule.c
@@ -18,7 +18,7 @@
 #include "worktree.h"
 
 static int config_fetch_recurse_submodules = RECURSE_SUBMODULES_ON_DEMAND;
-static int config_update_recurse_submodules = RECURSE_SUBMODULES_DEFAULT;
+static int config_update_recurse_submodules = RECURSE_SUBMODULES_OFF;
 static int parallel_jobs = 1;
 static struct string_list changed_submodule_paths = STRING_LIST_INIT_DUP;
 static int initialized_fetch_ref_tips;
@@ -169,6 +169,32 @@ int submodule_config(const char *var, const char *value, void *cb)
 	return 0;
 }
 
+int option_parse_recurse_submodules_worktree_updater(const struct option *opt,
+						     const char *arg, int unset)
+{
+	if (unset) {
+		config_update_recurse_submodules = RECURSE_SUBMODULES_OFF;
+		return 0;
+	}
+	if (arg)
+		config_update_recurse_submodules =
+			parse_update_recurse_submodules_arg(opt->long_name,
+							    arg);
+	else
+		config_update_recurse_submodules = RECURSE_SUBMODULES_ON;
+
+	return 0;
+}
+
+void load_submodule_cache(void)
+{
+	if (config_update_recurse_submodules == RECURSE_SUBMODULES_OFF)
+		return;
+
+	gitmodules_config();
+	git_config(submodule_config, NULL);
+}
+
 void gitmodules_config(void)
 {
 	const char *work_tree = get_git_work_tree();
@@ -596,11 +622,6 @@ void set_config_fetch_recurse_submodules(int value)
 	config_fetch_recurse_submodules = value;
 }
 
-void set_config_update_recurse_submodules(int value)
-{
-	config_update_recurse_submodules = value;
-}
-
 int should_update_submodules(void)
 {
 	return config_update_recurse_submodules == RECURSE_SUBMODULES_ON;
diff --git a/submodule.h b/submodule.h
index 1277480add..b13f120f76 100644
--- a/submodule.h
+++ b/submodule.h
@@ -39,6 +39,11 @@ extern void stage_updated_gitmodules(void);
 extern void set_diffopt_flags_from_submodule_config(struct diff_options *,
 		const char *path);
 extern int submodule_config(const char *var, const char *value, void *cb);
+
+struct option;
+int option_parse_recurse_submodules_worktree_updater(const struct option *opt,
+						     const char *arg, int unset);
+void load_submodule_cache(void);
 extern void gitmodules_config(void);
 extern void gitmodules_config_sha1(const unsigned char *commit_sha1);
 extern int is_submodule_initialized(const char *path);
@@ -65,7 +70,6 @@ extern void show_submodule_inline_diff(FILE *f, const char *path,
 		const char *del, const char *add, const char *reset,
 		const struct diff_options *opt);
 extern void set_config_fetch_recurse_submodules(int value);
-extern void set_config_update_recurse_submodules(int value);
 /* Check if we want to update any submodule.*/
 extern int should_update_submodules(void);
 /*
-- 
2.13.0.17.g582985b1e4


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 4/8] submodule loading: separate code path for .gitmodules and config overlay
  2017-05-26 19:10 [PATCHv2 0/8] A reroll of sb/submodule-blanket-recursive Stefan Beller
                   ` (2 preceding siblings ...)
  2017-05-26 19:10 ` [PATCH 3/8] reset/checkout/read-tree: unify config callback for submodule recursion Stefan Beller
@ 2017-05-26 19:10 ` Stefan Beller
  2017-05-26 19:10 ` [PATCH 5/8] Introduce 'submodule.recurse' option for worktree manipulators Stefan Beller
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 15+ messages in thread
From: Stefan Beller @ 2017-05-26 19:10 UTC (permalink / raw)
  To: bmwill; +Cc: git, gitster, Stefan Beller

The .gitmodules file is not supposed to have all the options available,
that are available in the configuration so separate it out.

A configuration option such as the hypothetical submodule.color.diff
that determines in which color a submodule change is printed,
is a very user specific thing, that the .gitmodules file should
not tamper with.

The .gitmodules file should only be used for settings that required
to setup the project in which the .gitmodules file is tracked. As the
minimum this would only include the name<->path mapping of the
submodule and its URL and branch.

Any further setting (such as 'fetch.recursesubmodules' or
'submodule.<name>.{update, ignore, shallow}') is not specific
to the project setup requirements, but rather is a distribution
of suggested developer configurations.  In other areas of Git
a suggested developer configuration is not transported in-tree
but via other means.  In an organisation this could be done
by deploying an opinionated system wide config (/etc/gitconfig)
or by putting the settings in the users home directory when
they start at the organisation. In open source projects this
is often accomplished via extensive READMEs (cf. our
SubmittingPatches/CodingGuidlines).

As a later patch in this series wants to introduce
a generic submodule recursion option, we want to make
sure that switch is not exposed via the gitmodules file.

Signed-off-by: Stefan Beller <sbeller@google.com>
---
 submodule.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/submodule.c b/submodule.c
index c9e764b519..78cccb7563 100644
--- a/submodule.c
+++ b/submodule.c
@@ -153,7 +153,8 @@ void set_diffopt_flags_from_submodule_config(struct diff_options *diffopt,
 	}
 }
 
-int submodule_config(const char *var, const char *value, void *cb)
+/* For loading from the .gitmodules file. */
+static int git_modules_config(const char *var, const char *value, void *cb)
 {
 	if (!strcmp(var, "submodule.fetchjobs")) {
 		parallel_jobs = git_config_int(var, value);
@@ -169,6 +170,12 @@ int submodule_config(const char *var, const char *value, void *cb)
 	return 0;
 }
 
+/* Loads all submodule settings from the config */
+int submodule_config(const char *var, const char *value, void *cb)
+{
+	return git_modules_config(var, value, cb);
+}
+
 int option_parse_recurse_submodules_worktree_updater(const struct option *opt,
 						     const char *arg, int unset)
 {
@@ -222,7 +229,8 @@ void gitmodules_config(void)
 		}
 
 		if (!gitmodules_is_unmerged)
-			git_config_from_file(submodule_config, gitmodules_path.buf, NULL);
+			git_config_from_file(git_modules_config,
+				gitmodules_path.buf, NULL);
 		strbuf_release(&gitmodules_path);
 	}
 }
@@ -233,7 +241,7 @@ void gitmodules_config_sha1(const unsigned char *commit_sha1)
 	unsigned char sha1[20];
 
 	if (gitmodule_sha1_from_commit(commit_sha1, sha1, &rev)) {
-		git_config_from_blob_sha1(submodule_config, rev.buf,
+		git_config_from_blob_sha1(git_modules_config, rev.buf,
 					  sha1, NULL);
 	}
 	strbuf_release(&rev);
-- 
2.13.0.17.g582985b1e4


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 5/8] Introduce 'submodule.recurse' option for worktree manipulators
  2017-05-26 19:10 [PATCHv2 0/8] A reroll of sb/submodule-blanket-recursive Stefan Beller
                   ` (3 preceding siblings ...)
  2017-05-26 19:10 ` [PATCH 4/8] submodule loading: separate code path for .gitmodules and config overlay Stefan Beller
@ 2017-05-26 19:10 ` Stefan Beller
  2017-05-26 19:10 ` [PATCH 6/8] builtin/grep.c: respect 'submodule.recurse' option Stefan Beller
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 15+ messages in thread
From: Stefan Beller @ 2017-05-26 19:10 UTC (permalink / raw)
  To: bmwill; +Cc: git, gitster, Stefan Beller

Any command that understands '--recurse-submodules' can have its
default changed to true, by setting the new 'submodule.recurse'
option.

This patch includes read-tree/checkout/reset for working tree
manipulating commands. Later patches will cover other commands.

Signed-off-by: Stefan Beller <sbeller@google.com>
---
 Documentation/config.txt  |  5 +++++
 builtin/checkout.c        |  2 +-
 builtin/read-tree.c       | 10 +++++++++-
 builtin/reset.c           | 10 +++++++++-
 submodule.c               | 23 +++++++++++++++++++++--
 submodule.h               |  1 +
 t/lib-submodule-update.sh | 12 ++++++++++++
 7 files changed, 58 insertions(+), 5 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 475e874d51..e367becf72 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -3063,6 +3063,11 @@ submodule.active::
 	submodule's path to determine if the submodule is of interest to git
 	commands.
 
+submodule.recurse::
+	Specifies if commands recurse into submodules by default. This
+	applies to all commands that have a `--recurse-submodules` option.
+	Defaults to false.
+
 submodule.fetchJobs::
 	Specifies how many submodules are fetched/cloned at the same time.
 	A positive integer allows up to that number of submodules fetched
diff --git a/builtin/checkout.c b/builtin/checkout.c
index acff6039d6..9ccc4a1d52 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -854,7 +854,7 @@ static int git_checkout_config(const char *var, const char *value, void *cb)
 	}
 
 	if (starts_with(var, "submodule."))
-		return parse_submodule_config_option(var, value);
+		return submodule_config(var, value, NULL);
 
 	return git_xmerge_config(var, value, NULL);
 }
diff --git a/builtin/read-tree.c b/builtin/read-tree.c
index 8a889ef4c3..6dd70cd430 100644
--- a/builtin/read-tree.c
+++ b/builtin/read-tree.c
@@ -98,6 +98,14 @@ static int debug_merge(const struct cache_entry * const *stages,
 	return 0;
 }
 
+int git_read_tree_config(const char *var, const char *value, void *cb)
+{
+	if (!strcmp(var, "submodule.recurse"))
+		return git_default_submodule_config(var, value, cb);
+
+	return git_default_config(var, value, cb);
+}
+
 static struct lock_file lock_file;
 
 int cmd_read_tree(int argc, const char **argv, const char *unused_prefix)
@@ -150,7 +158,7 @@ int cmd_read_tree(int argc, const char **argv, const char *unused_prefix)
 	opts.src_index = &the_index;
 	opts.dst_index = &the_index;
 
-	git_config(git_default_config, NULL);
+	git_config(git_read_tree_config, NULL);
 
 	argc = parse_options(argc, argv, unused_prefix, read_tree_options,
 			     read_tree_usage, 0);
diff --git a/builtin/reset.c b/builtin/reset.c
index 6f89dc5494..8ccdb7437e 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -266,6 +266,14 @@ static int reset_refs(const char *rev, const struct object_id *oid)
 	return update_ref_status;
 }
 
+int git_reset_config(const char *var, const char *value, void *cb)
+{
+	if (!strcmp(var, "submodule.recurse"))
+		return git_default_submodule_config(var, value, cb);
+
+	return git_default_config(var, value, cb);
+}
+
 int cmd_reset(int argc, const char **argv, const char *prefix)
 {
 	int reset_type = NONE, update_ref_status = 0, quiet = 0;
@@ -294,7 +302,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
 		OPT_END()
 	};
 
-	git_config(git_default_config, NULL);
+	git_config(git_reset_config, NULL);
 
 	argc = parse_options(argc, argv, prefix, options, git_reset_usage,
 						PARSE_OPT_KEEP_DASHDASH);
diff --git a/submodule.c b/submodule.c
index 78cccb7563..2b157dc995 100644
--- a/submodule.c
+++ b/submodule.c
@@ -16,6 +16,7 @@
 #include "quote.h"
 #include "remote.h"
 #include "worktree.h"
+#include "parse-options.h"
 
 static int config_fetch_recurse_submodules = RECURSE_SUBMODULES_ON_DEMAND;
 static int config_update_recurse_submodules = RECURSE_SUBMODULES_OFF;
@@ -170,10 +171,28 @@ static int git_modules_config(const char *var, const char *value, void *cb)
 	return 0;
 }
 
-/* Loads all submodule settings from the config */
+/* Loads all submodule settings from the config. */
 int submodule_config(const char *var, const char *value, void *cb)
 {
-	return git_modules_config(var, value, cb);
+	if (!strcmp(var, "submodule.recurse")) {
+		int v = git_config_bool(var, value) ?
+			RECURSE_SUBMODULES_ON : RECURSE_SUBMODULES_OFF;
+		config_update_recurse_submodules = v;
+		return 0;
+	} else {
+		return git_modules_config(var, value, cb);
+	}
+}
+
+/* Cheap function that only determines if we're interested in submodules at all */
+int git_default_submodule_config(const char *var, const char *value, void *cb)
+{
+	if (!strcmp(var, "submodule.recurse")) {
+		int v = git_config_bool(var, value) ?
+			RECURSE_SUBMODULES_ON : RECURSE_SUBMODULES_OFF;
+		config_update_recurse_submodules = v;
+	}
+	return 0;
 }
 
 int option_parse_recurse_submodules_worktree_updater(const struct option *opt,
diff --git a/submodule.h b/submodule.h
index b13f120f76..d920ca1d5a 100644
--- a/submodule.h
+++ b/submodule.h
@@ -39,6 +39,7 @@ extern void stage_updated_gitmodules(void);
 extern void set_diffopt_flags_from_submodule_config(struct diff_options *,
 		const char *path);
 extern int submodule_config(const char *var, const char *value, void *cb);
+extern int git_default_submodule_config(const char *var, const char *value, void *cb);
 
 struct option;
 int option_parse_recurse_submodules_worktree_updater(const struct option *opt,
diff --git a/t/lib-submodule-update.sh b/t/lib-submodule-update.sh
index 0272c4d8ca..52beadad96 100755
--- a/t/lib-submodule-update.sh
+++ b/t/lib-submodule-update.sh
@@ -990,6 +990,18 @@ test_submodule_switch_recursing_with_args () {
 		)
 	'
 
+	test_expect_success "git -c submodule.recurse=true $cmd_args: modified submodule updates submodule work tree" '
+		prolog &&
+		reset_work_tree_to_interested add_sub1 &&
+		(
+			cd submodule_update &&
+			git branch -t modify_sub1 origin/modify_sub1 &&
+			git -c submodule.recurse=true $cmd_args modify_sub1 &&
+			test_superproject_content origin/modify_sub1 &&
+			test_submodule_content sub1 origin/modify_sub1
+		)
+	'
+
 	# Updating a submodule to an invalid sha1 doesn't update the
 	# superproject nor the submodule's work tree.
 	test_expect_success "$command: updating to a missing submodule commit fails" '
-- 
2.13.0.17.g582985b1e4


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 6/8] builtin/grep.c: respect 'submodule.recurse' option
  2017-05-26 19:10 [PATCHv2 0/8] A reroll of sb/submodule-blanket-recursive Stefan Beller
                   ` (4 preceding siblings ...)
  2017-05-26 19:10 ` [PATCH 5/8] Introduce 'submodule.recurse' option for worktree manipulators Stefan Beller
@ 2017-05-26 19:10 ` Stefan Beller
  2017-05-26 19:10 ` [PATCH 7/8] builtin/push.c: " Stefan Beller
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 15+ messages in thread
From: Stefan Beller @ 2017-05-26 19:10 UTC (permalink / raw)
  To: bmwill; +Cc: git, gitster, Stefan Beller

In builtin/grep.c we parse the config before evaluating the command line
options. This makes the task of teaching grep to respect the new config
option 'submodule.recurse' very easy by just parsing that option.

As an alternative I had implemented a similar structure to treat
submodules as the fetch/push command have, including
* aligning the meaning of the 'recurse_submodules' to possible submodule
  values RECURSE_SUBMODULES_* as defined in submodule.h.
* having a callback to parse the value and
* reacting to the RECURSE_SUBMODULES_DEFAULT state that was the initial
  state.

However all this is not needed for a true boolean value, so let's keep
it simple. However this adds another place where "submodule.recurse" is
parsed.

Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/grep.c                     |  3 +++
 t/t7814-grep-recurse-submodules.sh | 18 ++++++++++++++++++
 2 files changed, 21 insertions(+)

diff --git a/builtin/grep.c b/builtin/grep.c
index b1095362fb..454e263820 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -302,6 +302,9 @@ static int grep_cmd_config(const char *var, const char *value, void *cb)
 #endif
 	}
 
+	if (!strcmp(var, "submodule.recurse"))
+		recurse_submodules = git_config_bool(var, value);
+
 	return st;
 }
 
diff --git a/t/t7814-grep-recurse-submodules.sh b/t/t7814-grep-recurse-submodules.sh
index 3a58197f47..7184113b9b 100755
--- a/t/t7814-grep-recurse-submodules.sh
+++ b/t/t7814-grep-recurse-submodules.sh
@@ -33,6 +33,24 @@ test_expect_success 'grep correctly finds patterns in a submodule' '
 	test_cmp expect actual
 '
 
+test_expect_success 'grep finds patterns in a submodule via config' '
+	test_config submodule.recurse true &&
+	# expect from previous test
+	git grep -e "(3|4)" >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'grep --no-recurse-submodules overrides config' '
+	test_config submodule.recurse true &&
+	cat >expect <<-\EOF &&
+	a:(1|2)d(3|4)
+	b/b:(3|4)
+	EOF
+
+	git grep -e "(3|4)" --no-recurse-submodules >actual &&
+	test_cmp expect actual
+'
+
 test_expect_success 'grep and basic pathspecs' '
 	cat >expect <<-\EOF &&
 	submodule/a:(1|2)d(3|4)
-- 
2.13.0.17.g582985b1e4


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 7/8] builtin/push.c: respect 'submodule.recurse' option
  2017-05-26 19:10 [PATCHv2 0/8] A reroll of sb/submodule-blanket-recursive Stefan Beller
                   ` (5 preceding siblings ...)
  2017-05-26 19:10 ` [PATCH 6/8] builtin/grep.c: respect 'submodule.recurse' option Stefan Beller
@ 2017-05-26 19:10 ` Stefan Beller
  2017-05-26 19:10 ` [PATCH 8/8] builtin/fetch.c: " Stefan Beller
  2017-05-30  5:30 ` [PATCHv2 0/8] A reroll of sb/submodule-blanket-recursive Junio C Hamano
  8 siblings, 0 replies; 15+ messages in thread
From: Stefan Beller @ 2017-05-26 19:10 UTC (permalink / raw)
  To: bmwill; +Cc: git, gitster, Stefan Beller

The closest mapping from the boolean 'submodule.recurse' set to "yes"
to the variety of submodule push modes is "on-demand", so implement that.

Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/push.c                 |  4 ++++
 t/t5531-deep-submodule-push.sh | 21 +++++++++++++++++++++
 2 files changed, 25 insertions(+)

diff --git a/builtin/push.c b/builtin/push.c
index a597759d8f..258648d5fd 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -498,6 +498,10 @@ 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 (!strcmp(k, "submodule.recurse")) {
+		int val = git_config_bool(k, v) ?
+			RECURSE_SUBMODULES_ON_DEMAND : RECURSE_SUBMODULES_OFF;
+		recurse_submodules = val;
 	}
 
 	return git_default_config(k, v, NULL);
diff --git a/t/t5531-deep-submodule-push.sh b/t/t5531-deep-submodule-push.sh
index 57ba322628..712c595fd8 100755
--- a/t/t5531-deep-submodule-push.sh
+++ b/t/t5531-deep-submodule-push.sh
@@ -126,6 +126,27 @@ test_expect_success 'push succeeds if submodule commit not on remote but using o
 	)
 '
 
+test_expect_success 'push succeeds if submodule commit not on remote but using auto-on-demand via submodule.recurse config' '
+	(
+		cd work/gar/bage &&
+		>recurse-on-demand-from-submodule-recurse-config &&
+		git add recurse-on-demand-from-submodule-recurse-config &&
+		git commit -m "Recurse submodule.recurse from config junk"
+	) &&
+	(
+		cd work &&
+		git add gar/bage &&
+		git commit -m "Recurse submodule.recurse from config for gar/bage" &&
+		git -c submodule.recurse push ../pub.git master &&
+		# Check that the supermodule commit got there
+		git fetch ../pub.git &&
+		git diff --quiet FETCH_HEAD master &&
+		# Check that the submodule commit got there too
+		cd gar/bage &&
+		git diff --quiet origin/master master
+	)
+'
+
 test_expect_success 'push recurse-submodules on command line overrides config' '
 	(
 		cd work/gar/bage &&
-- 
2.13.0.17.g582985b1e4


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 8/8] builtin/fetch.c: respect 'submodule.recurse' option
  2017-05-26 19:10 [PATCHv2 0/8] A reroll of sb/submodule-blanket-recursive Stefan Beller
                   ` (6 preceding siblings ...)
  2017-05-26 19:10 ` [PATCH 7/8] builtin/push.c: " Stefan Beller
@ 2017-05-26 19:10 ` Stefan Beller
  2017-05-30  5:30 ` [PATCHv2 0/8] A reroll of sb/submodule-blanket-recursive Junio C Hamano
  8 siblings, 0 replies; 15+ messages in thread
From: Stefan Beller @ 2017-05-26 19:10 UTC (permalink / raw)
  To: bmwill; +Cc: git, gitster, Stefan Beller

Signed-off-by: Stefan Beller <sbeller@google.com>
---
 builtin/fetch.c             |  7 +++++++
 t/t5526-fetch-submodules.sh | 10 ++++++++++
 2 files changed, 17 insertions(+)

diff --git a/builtin/fetch.c b/builtin/fetch.c
index 5f2c2ab23e..c1ec3b03c3 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -73,6 +73,13 @@ static int git_fetch_config(const char *k, const char *v, void *cb)
 		fetch_prune_config = git_config_bool(k, v);
 		return 0;
 	}
+
+	if (!strcmp(k, "submodule.recurse")) {
+		int r = git_config_bool(k, v) ?
+			RECURSE_SUBMODULES_ON : RECURSE_SUBMODULES_OFF;
+		recurse_submodules = r;
+	}
+
 	return git_default_config(k, v, cb);
 }
 
diff --git a/t/t5526-fetch-submodules.sh b/t/t5526-fetch-submodules.sh
index f3b0a8d30a..162baf101f 100755
--- a/t/t5526-fetch-submodules.sh
+++ b/t/t5526-fetch-submodules.sh
@@ -71,6 +71,16 @@ test_expect_success "fetch --recurse-submodules recurses into submodules" '
 	test_i18ncmp expect.err actual.err
 '
 
+test_expect_success "submodule.recurse option triggers recursive fetch" '
+	add_upstream_commit &&
+	(
+		cd downstream &&
+		git -c submodule.recurse fetch >../actual.out 2>../actual.err
+	) &&
+	test_must_be_empty actual.out &&
+	test_i18ncmp expect.err actual.err
+'
+
 test_expect_success "fetch --recurse-submodules -j2 has the same output behaviour" '
 	add_upstream_commit &&
 	(
-- 
2.13.0.17.g582985b1e4


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCHv2 0/8] A reroll of sb/submodule-blanket-recursive
  2017-05-26 19:10 [PATCHv2 0/8] A reroll of sb/submodule-blanket-recursive Stefan Beller
                   ` (7 preceding siblings ...)
  2017-05-26 19:10 ` [PATCH 8/8] builtin/fetch.c: " Stefan Beller
@ 2017-05-30  5:30 ` Junio C Hamano
  2017-06-01  0:30   ` [PATCHv3 0/4] " Stefan Beller
  8 siblings, 1 reply; 15+ messages in thread
From: Junio C Hamano @ 2017-05-30  5:30 UTC (permalink / raw)
  To: Stefan Beller; +Cc: bmwill, git

Stefan Beller <sbeller@google.com> writes:

> v2:
> * A reroll of sb/submodule-blanket-recursive.
> * This requires ab/grep-preparatory-cleanup 

2/8 seems to be more stale than sb/checkout-recurse-submodules that
was merged at f1101cef to 'master'.  I'll try to merge Ævar's series
to 'master' before that merge, queue these patches and see if the
resulting history is too messy.

Thanks.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCHv3 0/4] A reroll of sb/submodule-blanket-recursive
  2017-05-30  5:30 ` [PATCHv2 0/8] A reroll of sb/submodule-blanket-recursive Junio C Hamano
@ 2017-06-01  0:30   ` Stefan Beller
  2017-06-01  0:30     ` [PATCHv3 1/4] Introduce 'submodule.recurse' option for worktree manipulators Stefan Beller
                       ` (3 more replies)
  0 siblings, 4 replies; 15+ messages in thread
From: Stefan Beller @ 2017-06-01  0:30 UTC (permalink / raw)
  To: gitster; +Cc: bmwill, git, sbeller

v3:
* rerolling only the top-4 patches of sb/submodule-blanket-recursive.
  (base: 1d789d089280539ca39b83aabb67860929d39b75)
* fixes function declarations that should be static, thanks Ramsay!

v2:
* A reroll of sb/submodule-blanket-recursive.
* This requires ab/grep-preparatory-cleanup
* It changed a lot from v1, as in v1 the tests did not work,
  hence the code was broken. Now it actually works.
* it also includes grep, fetch, push in addition to plain working tree
  manipulators.

Thanks,
Stefan

Stefan Beller (4):
  Introduce 'submodule.recurse' option for worktree manipulators
  builtin/grep.c: respect 'submodule.recurse' option
  builtin/push.c: respect 'submodule.recurse' option
  builtin/fetch.c: respect 'submodule.recurse' option

 Documentation/config.txt           |  5 +++++
 builtin/checkout.c                 |  2 +-
 builtin/fetch.c                    |  7 +++++++
 builtin/grep.c                     |  3 +++
 builtin/push.c                     |  4 ++++
 builtin/read-tree.c                | 10 +++++++++-
 builtin/reset.c                    | 10 +++++++++-
 submodule.c                        | 23 +++++++++++++++++++++--
 submodule.h                        |  1 +
 t/lib-submodule-update.sh          | 12 ++++++++++++
 t/t5526-fetch-submodules.sh        | 10 ++++++++++
 t/t5531-deep-submodule-push.sh     | 21 +++++++++++++++++++++
 t/t7814-grep-recurse-submodules.sh | 18 ++++++++++++++++++
 13 files changed, 121 insertions(+), 5 deletions(-)

-- 
2.13.0.17.gab62347cd9


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCHv3 1/4] Introduce 'submodule.recurse' option for worktree manipulators
  2017-06-01  0:30   ` [PATCHv3 0/4] " Stefan Beller
@ 2017-06-01  0:30     ` Stefan Beller
  2017-06-01  0:30     ` [PATCHv3 2/4] builtin/grep.c: respect 'submodule.recurse' option Stefan Beller
                       ` (2 subsequent siblings)
  3 siblings, 0 replies; 15+ messages in thread
From: Stefan Beller @ 2017-06-01  0:30 UTC (permalink / raw)
  To: gitster; +Cc: bmwill, git, sbeller

Any command that understands '--recurse-submodules' can have its
default changed to true, by setting the new 'submodule.recurse'
option.

This patch includes read-tree/checkout/reset for working tree
manipulating commands. Later patches will cover other commands.

Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/config.txt  |  5 +++++
 builtin/checkout.c        |  2 +-
 builtin/read-tree.c       | 10 +++++++++-
 builtin/reset.c           | 10 +++++++++-
 submodule.c               | 23 +++++++++++++++++++++--
 submodule.h               |  1 +
 t/lib-submodule-update.sh | 12 ++++++++++++
 7 files changed, 58 insertions(+), 5 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index e0b9fd0bc3..f60c504e86 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -3065,6 +3065,11 @@ submodule.active::
 	submodule's path to determine if the submodule is of interest to git
 	commands.
 
+submodule.recurse::
+	Specifies if commands recurse into submodules by default. This
+	applies to all commands that have a `--recurse-submodules` option.
+	Defaults to false.
+
 submodule.fetchJobs::
 	Specifies how many submodules are fetched/cloned at the same time.
 	A positive integer allows up to that number of submodules fetched
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 56ea723b75..e289b7d477 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -855,7 +855,7 @@ static int git_checkout_config(const char *var, const char *value, void *cb)
 	}
 
 	if (starts_with(var, "submodule."))
-		return parse_submodule_config_option(var, value);
+		return submodule_config(var, value, NULL);
 
 	return git_xmerge_config(var, value, NULL);
 }
diff --git a/builtin/read-tree.c b/builtin/read-tree.c
index 8a889ef4c3..7fd55140db 100644
--- a/builtin/read-tree.c
+++ b/builtin/read-tree.c
@@ -98,6 +98,14 @@ static int debug_merge(const struct cache_entry * const *stages,
 	return 0;
 }
 
+static int git_read_tree_config(const char *var, const char *value, void *cb)
+{
+	if (!strcmp(var, "submodule.recurse"))
+		return git_default_submodule_config(var, value, cb);
+
+	return git_default_config(var, value, cb);
+}
+
 static struct lock_file lock_file;
 
 int cmd_read_tree(int argc, const char **argv, const char *unused_prefix)
@@ -150,7 +158,7 @@ int cmd_read_tree(int argc, const char **argv, const char *unused_prefix)
 	opts.src_index = &the_index;
 	opts.dst_index = &the_index;
 
-	git_config(git_default_config, NULL);
+	git_config(git_read_tree_config, NULL);
 
 	argc = parse_options(argc, argv, unused_prefix, read_tree_options,
 			     read_tree_usage, 0);
diff --git a/builtin/reset.c b/builtin/reset.c
index 6f89dc5494..585cfe0745 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -266,6 +266,14 @@ static int reset_refs(const char *rev, const struct object_id *oid)
 	return update_ref_status;
 }
 
+static int git_reset_config(const char *var, const char *value, void *cb)
+{
+	if (!strcmp(var, "submodule.recurse"))
+		return git_default_submodule_config(var, value, cb);
+
+	return git_default_config(var, value, cb);
+}
+
 int cmd_reset(int argc, const char **argv, const char *prefix)
 {
 	int reset_type = NONE, update_ref_status = 0, quiet = 0;
@@ -294,7 +302,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
 		OPT_END()
 	};
 
-	git_config(git_default_config, NULL);
+	git_config(git_reset_config, NULL);
 
 	argc = parse_options(argc, argv, prefix, options, git_reset_usage,
 						PARSE_OPT_KEEP_DASHDASH);
diff --git a/submodule.c b/submodule.c
index 78cccb7563..2b157dc995 100644
--- a/submodule.c
+++ b/submodule.c
@@ -16,6 +16,7 @@
 #include "quote.h"
 #include "remote.h"
 #include "worktree.h"
+#include "parse-options.h"
 
 static int config_fetch_recurse_submodules = RECURSE_SUBMODULES_ON_DEMAND;
 static int config_update_recurse_submodules = RECURSE_SUBMODULES_OFF;
@@ -170,10 +171,28 @@ static int git_modules_config(const char *var, const char *value, void *cb)
 	return 0;
 }
 
-/* Loads all submodule settings from the config */
+/* Loads all submodule settings from the config. */
 int submodule_config(const char *var, const char *value, void *cb)
 {
-	return git_modules_config(var, value, cb);
+	if (!strcmp(var, "submodule.recurse")) {
+		int v = git_config_bool(var, value) ?
+			RECURSE_SUBMODULES_ON : RECURSE_SUBMODULES_OFF;
+		config_update_recurse_submodules = v;
+		return 0;
+	} else {
+		return git_modules_config(var, value, cb);
+	}
+}
+
+/* Cheap function that only determines if we're interested in submodules at all */
+int git_default_submodule_config(const char *var, const char *value, void *cb)
+{
+	if (!strcmp(var, "submodule.recurse")) {
+		int v = git_config_bool(var, value) ?
+			RECURSE_SUBMODULES_ON : RECURSE_SUBMODULES_OFF;
+		config_update_recurse_submodules = v;
+	}
+	return 0;
 }
 
 int option_parse_recurse_submodules_worktree_updater(const struct option *opt,
diff --git a/submodule.h b/submodule.h
index b13f120f76..d920ca1d5a 100644
--- a/submodule.h
+++ b/submodule.h
@@ -39,6 +39,7 @@ extern void stage_updated_gitmodules(void);
 extern void set_diffopt_flags_from_submodule_config(struct diff_options *,
 		const char *path);
 extern int submodule_config(const char *var, const char *value, void *cb);
+extern int git_default_submodule_config(const char *var, const char *value, void *cb);
 
 struct option;
 int option_parse_recurse_submodules_worktree_updater(const struct option *opt,
diff --git a/t/lib-submodule-update.sh b/t/lib-submodule-update.sh
index 0272c4d8ca..52beadad96 100755
--- a/t/lib-submodule-update.sh
+++ b/t/lib-submodule-update.sh
@@ -990,6 +990,18 @@ test_submodule_switch_recursing_with_args () {
 		)
 	'
 
+	test_expect_success "git -c submodule.recurse=true $cmd_args: modified submodule updates submodule work tree" '
+		prolog &&
+		reset_work_tree_to_interested add_sub1 &&
+		(
+			cd submodule_update &&
+			git branch -t modify_sub1 origin/modify_sub1 &&
+			git -c submodule.recurse=true $cmd_args modify_sub1 &&
+			test_superproject_content origin/modify_sub1 &&
+			test_submodule_content sub1 origin/modify_sub1
+		)
+	'
+
 	# Updating a submodule to an invalid sha1 doesn't update the
 	# superproject nor the submodule's work tree.
 	test_expect_success "$command: updating to a missing submodule commit fails" '
-- 
2.13.0.17.gab62347cd9


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCHv3 2/4] builtin/grep.c: respect 'submodule.recurse' option
  2017-06-01  0:30   ` [PATCHv3 0/4] " Stefan Beller
  2017-06-01  0:30     ` [PATCHv3 1/4] Introduce 'submodule.recurse' option for worktree manipulators Stefan Beller
@ 2017-06-01  0:30     ` Stefan Beller
  2017-06-01  0:30     ` [PATCHv3 3/4] builtin/push.c: " Stefan Beller
  2017-06-01  0:30     ` [PATCHv3 4/4] builtin/fetch.c: " Stefan Beller
  3 siblings, 0 replies; 15+ messages in thread
From: Stefan Beller @ 2017-06-01  0:30 UTC (permalink / raw)
  To: gitster; +Cc: bmwill, git, sbeller

In builtin/grep.c we parse the config before evaluating the command line
options. This makes the task of teaching grep to respect the new config
option 'submodule.recurse' very easy by just parsing that option.

As an alternative I had implemented a similar structure to treat
submodules as the fetch/push command have, including
* aligning the meaning of the 'recurse_submodules' to possible submodule
  values RECURSE_SUBMODULES_* as defined in submodule.h.
* having a callback to parse the value and
* reacting to the RECURSE_SUBMODULES_DEFAULT state that was the initial
  state.

However all this is not needed for a true boolean value, so let's keep
it simple. However this adds another place where "submodule.recurse" is
parsed.

Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/grep.c                     |  3 +++
 t/t7814-grep-recurse-submodules.sh | 18 ++++++++++++++++++
 2 files changed, 21 insertions(+)

diff --git a/builtin/grep.c b/builtin/grep.c
index b1095362fb..454e263820 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -302,6 +302,9 @@ static int grep_cmd_config(const char *var, const char *value, void *cb)
 #endif
 	}
 
+	if (!strcmp(var, "submodule.recurse"))
+		recurse_submodules = git_config_bool(var, value);
+
 	return st;
 }
 
diff --git a/t/t7814-grep-recurse-submodules.sh b/t/t7814-grep-recurse-submodules.sh
index 3a58197f47..7184113b9b 100755
--- a/t/t7814-grep-recurse-submodules.sh
+++ b/t/t7814-grep-recurse-submodules.sh
@@ -33,6 +33,24 @@ test_expect_success 'grep correctly finds patterns in a submodule' '
 	test_cmp expect actual
 '
 
+test_expect_success 'grep finds patterns in a submodule via config' '
+	test_config submodule.recurse true &&
+	# expect from previous test
+	git grep -e "(3|4)" >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'grep --no-recurse-submodules overrides config' '
+	test_config submodule.recurse true &&
+	cat >expect <<-\EOF &&
+	a:(1|2)d(3|4)
+	b/b:(3|4)
+	EOF
+
+	git grep -e "(3|4)" --no-recurse-submodules >actual &&
+	test_cmp expect actual
+'
+
 test_expect_success 'grep and basic pathspecs' '
 	cat >expect <<-\EOF &&
 	submodule/a:(1|2)d(3|4)
-- 
2.13.0.17.gab62347cd9


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCHv3 3/4] builtin/push.c: respect 'submodule.recurse' option
  2017-06-01  0:30   ` [PATCHv3 0/4] " Stefan Beller
  2017-06-01  0:30     ` [PATCHv3 1/4] Introduce 'submodule.recurse' option for worktree manipulators Stefan Beller
  2017-06-01  0:30     ` [PATCHv3 2/4] builtin/grep.c: respect 'submodule.recurse' option Stefan Beller
@ 2017-06-01  0:30     ` Stefan Beller
  2017-06-01  0:30     ` [PATCHv3 4/4] builtin/fetch.c: " Stefan Beller
  3 siblings, 0 replies; 15+ messages in thread
From: Stefan Beller @ 2017-06-01  0:30 UTC (permalink / raw)
  To: gitster; +Cc: bmwill, git, sbeller

The closest mapping from the boolean 'submodule.recurse' set to "yes"
to the variety of submodule push modes is "on-demand", so implement that.

Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/push.c                 |  4 ++++
 t/t5531-deep-submodule-push.sh | 21 +++++++++++++++++++++
 2 files changed, 25 insertions(+)

diff --git a/builtin/push.c b/builtin/push.c
index a597759d8f..258648d5fd 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -498,6 +498,10 @@ 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 (!strcmp(k, "submodule.recurse")) {
+		int val = git_config_bool(k, v) ?
+			RECURSE_SUBMODULES_ON_DEMAND : RECURSE_SUBMODULES_OFF;
+		recurse_submodules = val;
 	}
 
 	return git_default_config(k, v, NULL);
diff --git a/t/t5531-deep-submodule-push.sh b/t/t5531-deep-submodule-push.sh
index 57ba322628..712c595fd8 100755
--- a/t/t5531-deep-submodule-push.sh
+++ b/t/t5531-deep-submodule-push.sh
@@ -126,6 +126,27 @@ test_expect_success 'push succeeds if submodule commit not on remote but using o
 	)
 '
 
+test_expect_success 'push succeeds if submodule commit not on remote but using auto-on-demand via submodule.recurse config' '
+	(
+		cd work/gar/bage &&
+		>recurse-on-demand-from-submodule-recurse-config &&
+		git add recurse-on-demand-from-submodule-recurse-config &&
+		git commit -m "Recurse submodule.recurse from config junk"
+	) &&
+	(
+		cd work &&
+		git add gar/bage &&
+		git commit -m "Recurse submodule.recurse from config for gar/bage" &&
+		git -c submodule.recurse push ../pub.git master &&
+		# Check that the supermodule commit got there
+		git fetch ../pub.git &&
+		git diff --quiet FETCH_HEAD master &&
+		# Check that the submodule commit got there too
+		cd gar/bage &&
+		git diff --quiet origin/master master
+	)
+'
+
 test_expect_success 'push recurse-submodules on command line overrides config' '
 	(
 		cd work/gar/bage &&
-- 
2.13.0.17.gab62347cd9


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCHv3 4/4] builtin/fetch.c: respect 'submodule.recurse' option
  2017-06-01  0:30   ` [PATCHv3 0/4] " Stefan Beller
                       ` (2 preceding siblings ...)
  2017-06-01  0:30     ` [PATCHv3 3/4] builtin/push.c: " Stefan Beller
@ 2017-06-01  0:30     ` Stefan Beller
  3 siblings, 0 replies; 15+ messages in thread
From: Stefan Beller @ 2017-06-01  0:30 UTC (permalink / raw)
  To: gitster; +Cc: bmwill, git, sbeller

Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/fetch.c             |  7 +++++++
 t/t5526-fetch-submodules.sh | 10 ++++++++++
 2 files changed, 17 insertions(+)

diff --git a/builtin/fetch.c b/builtin/fetch.c
index 5f2c2ab23e..c1ec3b03c3 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -73,6 +73,13 @@ static int git_fetch_config(const char *k, const char *v, void *cb)
 		fetch_prune_config = git_config_bool(k, v);
 		return 0;
 	}
+
+	if (!strcmp(k, "submodule.recurse")) {
+		int r = git_config_bool(k, v) ?
+			RECURSE_SUBMODULES_ON : RECURSE_SUBMODULES_OFF;
+		recurse_submodules = r;
+	}
+
 	return git_default_config(k, v, cb);
 }
 
diff --git a/t/t5526-fetch-submodules.sh b/t/t5526-fetch-submodules.sh
index f3b0a8d30a..162baf101f 100755
--- a/t/t5526-fetch-submodules.sh
+++ b/t/t5526-fetch-submodules.sh
@@ -71,6 +71,16 @@ test_expect_success "fetch --recurse-submodules recurses into submodules" '
 	test_i18ncmp expect.err actual.err
 '
 
+test_expect_success "submodule.recurse option triggers recursive fetch" '
+	add_upstream_commit &&
+	(
+		cd downstream &&
+		git -c submodule.recurse fetch >../actual.out 2>../actual.err
+	) &&
+	test_must_be_empty actual.out &&
+	test_i18ncmp expect.err actual.err
+'
+
 test_expect_success "fetch --recurse-submodules -j2 has the same output behaviour" '
 	add_upstream_commit &&
 	(
-- 
2.13.0.17.gab62347cd9


^ permalink raw reply related	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2017-06-01  0:31 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-05-26 19:10 [PATCHv2 0/8] A reroll of sb/submodule-blanket-recursive Stefan Beller
2017-05-26 19:10 ` [PATCH 1/8] submodule recursing: do not write a config variable twice Stefan Beller
2017-05-26 19:10 ` [PATCH 2/8] submodule test invocation: only pass additional arguments Stefan Beller
2017-05-26 19:10 ` [PATCH 3/8] reset/checkout/read-tree: unify config callback for submodule recursion Stefan Beller
2017-05-26 19:10 ` [PATCH 4/8] submodule loading: separate code path for .gitmodules and config overlay Stefan Beller
2017-05-26 19:10 ` [PATCH 5/8] Introduce 'submodule.recurse' option for worktree manipulators Stefan Beller
2017-05-26 19:10 ` [PATCH 6/8] builtin/grep.c: respect 'submodule.recurse' option Stefan Beller
2017-05-26 19:10 ` [PATCH 7/8] builtin/push.c: " Stefan Beller
2017-05-26 19:10 ` [PATCH 8/8] builtin/fetch.c: " Stefan Beller
2017-05-30  5:30 ` [PATCHv2 0/8] A reroll of sb/submodule-blanket-recursive Junio C Hamano
2017-06-01  0:30   ` [PATCHv3 0/4] " Stefan Beller
2017-06-01  0:30     ` [PATCHv3 1/4] Introduce 'submodule.recurse' option for worktree manipulators Stefan Beller
2017-06-01  0:30     ` [PATCHv3 2/4] builtin/grep.c: respect 'submodule.recurse' option Stefan Beller
2017-06-01  0:30     ` [PATCHv3 3/4] builtin/push.c: " Stefan Beller
2017-06-01  0:30     ` [PATCHv3 4/4] builtin/fetch.c: " Stefan Beller

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).