[PATCH v3 0/3] Teach fetch and pull to recursively fetch submodules

[PATCH v3 0/3] Teach fetch and pull to recursively fetch submodules

[Jens Lehmann]

So here is the third iteration of the recursive fetch/pull series.

Changes since v2:

* The default now is to not recurse into submodules (also see "Still
  to do")

* The command line option has been renamed to "--recurse-submodules" as
  proposed at the GitTogether

* The config options are named "submodule.<name>.fetchRecurseSubmodules"
  and "fetch.recurseSubmodules" now

* Switched the order of the two patches adding the config options


Question:

* Should the "--submodule-prefix" option - which is only used internally
  now - be a hidden option to "git fetch"?


Still to do:

* Add a mode to only fetch those submodules where new recorded commits
  are fetched in the superproject (and maybe later even fetch only those
  commits in the submodule which are referenced in the superprojects
  fetch). This could be a sane default - especially for projects having
  lots of submodules - and could be enabled by using then to be added
  "--recurse-submodules=changed" and "fetch[.]recurseSubmodules=changed"
  options where the configuration uses other defaults (I'm not really
  convinced yet 'changed' is a very good name but couldn't come up with
  a better one yet. So suggestions for alternatives are very welcome :-)

* Boost performance by concurrently fetching submodules (after my first
  experiments this must be configurable, e.g. how many fetch commands
  to run at the same time, as some git servers barf on too many fetches
  run at the same time)


But nonetheless I think this patch series is ok for inclusion as it does
not change default behavior and gives people the opportunity to play with
recursive fetch/pull by enabling one of the introduced config options.


Jens Lehmann (3):
  fetch/pull: Add the --recurse-submodules option
  Add the 'fetch.recurseSubmodules' config setting
  Submodules: Add the "fetchRecurseSubmodules" config option

 Documentation/config.txt        |   12 +++
 Documentation/fetch-options.txt |   13 +++
 Documentation/gitmodules.txt    |    8 ++
 builtin/fetch.c                 |   64 ++++++++++---
 git-pull.sh                     |   10 ++-
 submodule.c                     |   98 +++++++++++++++++++-
 submodule.h                     |    5 +
 t/t5526-fetch-submodules.sh     |  195 +++++++++++++++++++++++++++++++++++++++
 8 files changed, 388 insertions(+), 17 deletions(-)
 create mode 100755 t/t5526-fetch-submodules.sh

-- 
1.7.3.2.337.g9376c

[PATCH v3 1/3] fetch/pull: Add the --recurse-submodules option

[Jens Lehmann]

Until now you had to call "git submodule update" (without -N|--no-fetch
option) or something like "git submodule foreach git fetch" to fetch
new commits in populated submodules from their remote.

This could lead to "(commits not present)" messages in the output of
"git diff --submodule" (which is used by "git gui" and "gitk") after
fetching or pulling new commits in the superproject and is an obstacle for
implementing recursive checkout of submodules. Also "git submodule
update" cannot fetch changes when disconnected, so it was very easy to
forget to fetch the submodule changes before disconnecting only to
discover later that they are needed.

This patch adds the "--recurse-submodules" option to recursively fetch
each populated submodule from the url configured in the .git/config of the
submodule at the end of each "git fetch" or during "git pull" in the
superproject. The submodule paths are taken from the index.

The option "--submodule-prefix" is added to "git fetch" to be able to
print out the full paths of nested submodules.

Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de>
---
 Documentation/fetch-options.txt |   12 ++++
 builtin/fetch.c                 |   53 ++++++++++++++-----
 git-pull.sh                     |    7 ++-
 submodule.c                     |   66 +++++++++++++++++++++++-
 submodule.h                     |    3 +
 t/t5526-fetch-submodules.sh     |  109 +++++++++++++++++++++++++++++++++++++++
 6 files changed, 233 insertions(+), 17 deletions(-)
 create mode 100755 t/t5526-fetch-submodules.sh

diff --git a/Documentation/fetch-options.txt b/Documentation/fetch-options.txt
index 5ce1e72..bd36466 100644
--- a/Documentation/fetch-options.txt
+++ b/Documentation/fetch-options.txt
@@ -66,6 +66,18 @@ ifndef::git-pull[]
 	linkgit:git-config[1].
 endif::git-pull[]

+--recurse-submodules::
+	Use this option to fetch new commits of all populated submodules too.
+
+ifndef::git-pull[]
+--submodule-prefix::
+	When recursing into nested submodules it is not sufficient to just
+	print out the submodule path, all directories already traversed from
+	the superproject have to be printed too. This option is used to provide
+	that information, its value is prepended to the local submodule path
+	before it is printed out.
+endif::git-pull[]
+
 -u::
 --update-head-ok::
 	By default 'git fetch' refuses to update the head which
diff --git a/builtin/fetch.c b/builtin/fetch.c
index d35f000..658bb4c 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -12,6 +12,7 @@
 #include "parse-options.h"
 #include "sigchain.h"
 #include "transport.h"
+#include "submodule.h"

 static const char * const builtin_fetch_usage[] = {
 	"git fetch [<options>] [<repository> [<refspec>...]]",
@@ -28,12 +29,13 @@ enum {
 };

 static int all, append, dry_run, force, keep, multiple, prune, update_head_ok, verbosity;
-static int progress;
+static int progress, recurse_submodules;
 static int tags = TAGS_DEFAULT;
 static const char *depth;
 static const char *upload_pack;
 static struct strbuf default_rla = STRBUF_INIT;
 static struct transport *transport;
+static const char *submodule_prefix = "";

 static struct option builtin_fetch_options[] = {
 	OPT__VERBOSITY(&verbosity),
@@ -53,6 +55,8 @@ static struct option builtin_fetch_options[] = {
 		    "do not fetch all tags (--no-tags)", TAGS_UNSET),
 	OPT_BOOLEAN('p', "prune", &prune,
 		    "prune tracking branches no longer on remote"),
+	OPT_BOOLEAN(0, "recurse-submodules", &recurse_submodules,
+		    "control recursive fetching of submodules"),
 	OPT_BOOLEAN(0, "dry-run", &dry_run,
 		    "dry run"),
 	OPT_BOOLEAN('k', "keep", &keep, "keep downloaded pack"),
@@ -61,6 +65,8 @@ static struct option builtin_fetch_options[] = {
 	OPT_BOOLEAN(0, "progress", &progress, "force progress reporting"),
 	OPT_STRING(0, "depth", &depth, "DEPTH",
 		   "deepen history of shallow clone"),
+	OPT_STRING(0, "submodule-prefix", &submodule_prefix, "DIR",
+		   "prepend this to submodule path output"),
 	OPT_END()
 };

@@ -784,28 +790,36 @@ static int add_remote_or_group(const char *name, struct string_list *list)
 	return 1;
 }

-static int fetch_multiple(struct string_list *list)
+static void add_options_to_argv(int *argc, const char **argv)
 {
-	int i, result = 0;
-	const char *argv[11] = { "fetch", "--append" };
-	int argc = 2;
-
 	if (dry_run)
-		argv[argc++] = "--dry-run";
+		argv[(*argc)++] = "--dry-run";
 	if (prune)
-		argv[argc++] = "--prune";
+		argv[(*argc)++] = "--prune";
 	if (update_head_ok)
-		argv[argc++] = "--update-head-ok";
+		argv[(*argc)++] = "--update-head-ok";
 	if (force)
-		argv[argc++] = "--force";
+		argv[(*argc)++] = "--force";
 	if (keep)
-		argv[argc++] = "--keep";
+		argv[(*argc)++] = "--keep";
+	if (recurse_submodules)
+		argv[(*argc)++] = "--recurse-submodules";
 	if (verbosity >= 2)
-		argv[argc++] = "-v";
+		argv[(*argc)++] = "-v";
 	if (verbosity >= 1)
-		argv[argc++] = "-v";
+		argv[(*argc)++] = "-v";
 	else if (verbosity < 0)
-		argv[argc++] = "-q";
+		argv[(*argc)++] = "-q";
+
+}
+
+static int fetch_multiple(struct string_list *list)
+{
+	int i, result = 0;
+	const char *argv[12] = { "fetch", "--append" };
+	int argc = 2;
+
+	add_options_to_argv(&argc, argv);

 	if (!append && !dry_run) {
 		int errcode = truncate_fetch_head();
@@ -926,6 +940,17 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)
 		}
 	}

+	if (!result && recurse_submodules) {
+		const char *options[10];
+		int num_options = 0;
+		gitmodules_config();
+		git_config(submodule_config, NULL);
+		add_options_to_argv(&num_options, options);
+		result = fetch_populated_submodules(num_options, options,
+						    submodule_prefix,
+						    verbosity < 0);
+	}
+
 	/* All names were strdup()ed or strndup()ed */
 	list.strdup_strings = 1;
 	string_list_clear(&list, 0);
diff --git a/git-pull.sh b/git-pull.sh
index 8eb74d4..5804c62 100755
--- a/git-pull.sh
+++ b/git-pull.sh
@@ -38,7 +38,7 @@ test -z "$(git ls-files -u)" || die_conflict
 test -f "$GIT_DIR/MERGE_HEAD" && die_merge

 strategy_args= diffstat= no_commit= squash= no_ff= ff_only=
-log_arg= verbosity= progress=
+log_arg= verbosity= progress= recurse_submodules=
 merge_args=
 curr_branch=$(git symbolic-ref -q HEAD)
 curr_branch_short="${curr_branch#refs/heads/}"
@@ -105,6 +105,9 @@ do
 	--no-r|--no-re|--no-reb|--no-reba|--no-rebas|--no-rebase)
 		rebase=false
 		;;
+	--recurse-submodules)
+		recurse_submodules=--recurse-submodules
+		;;
 	--d|--dr|--dry|--dry-|--dry-r|--dry-ru|--dry-run)
 		dry_run=--dry-run
 		;;
@@ -220,7 +223,7 @@ test true = "$rebase" && {
 	done
 }
 orig_head=$(git rev-parse -q --verify HEAD)
-git fetch $verbosity $progress $dry_run --update-head-ok "$@" || exit 1
+git fetch $verbosity $progress $dry_run $recurse_submodules --update-head-ok "$@" || exit 1
 test -z "$dry_run" || exit 0

 curr_head=$(git rev-parse -q --verify HEAD)
diff --git a/submodule.c b/submodule.c
index 91a4758..4d9b774 100644
--- a/submodule.c
+++ b/submodule.c
@@ -63,7 +63,7 @@ void set_diffopt_flags_from_submodule_config(struct diff_options *diffopt,
 	}
 }

-static int submodule_config(const char *var, const char *value, void *cb)
+int submodule_config(const char *var, const char *value, void *cb)
 {
 	if (!prefixcmp(var, "submodule."))
 		return parse_submodule_config_option(var, value);
@@ -229,6 +229,70 @@ void show_submodule_summary(FILE *f, const char *path,
 	strbuf_release(&sb);
 }

+int fetch_populated_submodules(int num_options, const char **options,
+			       const char *prefix, int quiet)
+{
+	int i, result = 0, argc = 0;
+	struct child_process cp;
+	const char **argv;
+	struct string_list_item *name_for_path;
+	const char *work_tree = get_git_work_tree();
+	if (!work_tree)
+		return 0;
+
+	if (!the_index.initialized)
+		if (read_cache() < 0)
+			die("index file corrupt");
+
+	argv = xcalloc(num_options + 5, sizeof(const char *));
+	argv[argc++] = "fetch";
+	for (i = 0; i < num_options; i++)
+		argv[argc++] = options[i];
+	argv[argc++] = "--submodule-prefix";
+
+	memset(&cp, 0, sizeof(cp));
+	cp.argv = argv;
+	cp.env = local_repo_env;
+	cp.git_cmd = 1;
+	cp.no_stdin = 1;
+
+	for (i = 0; i < active_nr; i++) {
+		struct strbuf submodule_path = STRBUF_INIT;
+		struct strbuf submodule_git_dir = STRBUF_INIT;
+		struct strbuf submodule_prefix = STRBUF_INIT;
+		struct cache_entry *ce = active_cache[i];
+		const char *git_dir, *name;
+
+		if (!S_ISGITLINK(ce->ce_mode))
+			continue;
+
+		name = ce->name;
+		name_for_path = unsorted_string_list_lookup(&config_name_for_path, ce->name);
+		if (name_for_path)
+			name = name_for_path->util;
+
+		strbuf_addf(&submodule_path, "%s/%s", work_tree, ce->name);
+		strbuf_addf(&submodule_git_dir, "%s/.git", submodule_path.buf);
+		strbuf_addf(&submodule_prefix, "%s%s/", prefix, ce->name);
+		git_dir = read_gitfile_gently(submodule_git_dir.buf);
+		if (!git_dir)
+			git_dir = submodule_git_dir.buf;
+		if (is_directory(git_dir)) {
+			if (!quiet)
+				printf("Fetching submodule %s%s\n", prefix, ce->name);
+			cp.dir = submodule_path.buf;
+			argv[argc] = submodule_prefix.buf;
+			if (run_command(&cp))
+				result = 1;
+		}
+		strbuf_release(&submodule_path);
+		strbuf_release(&submodule_git_dir);
+		strbuf_release(&submodule_prefix);
+	}
+	free(argv);
+	return result;
+}
+
 unsigned is_submodule_modified(const char *path, int ignore_untracked)
 {
 	ssize_t len;
diff --git a/submodule.h b/submodule.h
index 386f410..b39d7a1 100644
--- a/submodule.h
+++ b/submodule.h
@@ -5,6 +5,7 @@ struct diff_options;

 void set_diffopt_flags_from_submodule_config(struct diff_options *diffopt,
 		const char *path);
+int submodule_config(const char *var, const char *value, void *cb);
 void gitmodules_config();
 int parse_submodule_config_option(const char *var, const char *value);
 void handle_ignore_submodules_arg(struct diff_options *diffopt, const char *);
@@ -12,6 +13,8 @@ void show_submodule_summary(FILE *f, const char *path,
 		unsigned char one[20], unsigned char two[20],
 		unsigned dirty_submodule,
 		const char *del, const char *add, const char *reset);
+int fetch_populated_submodules(int num_options, const char **options,
+			       const char *prefix, int quiet);
 unsigned is_submodule_modified(const char *path, int ignore_untracked);
 int merge_submodule(unsigned char result[20], const char *path, const unsigned char base[20],
 		    const unsigned char a[20], const unsigned char b[20]);
diff --git a/t/t5526-fetch-submodules.sh b/t/t5526-fetch-submodules.sh
new file mode 100755
index 0000000..107317a
--- /dev/null
+++ b/t/t5526-fetch-submodules.sh
@@ -0,0 +1,109 @@
+#!/bin/sh
+# Copyright (c) 2010, Jens Lehmann
+
+test_description='Recursive "git fetch" for submodules'
+
+. ./test-lib.sh
+
+pwd=$(pwd)
+
+add_upstream_commit() {
+	(
+		cd submodule &&
+		head1=$(git rev-parse --short HEAD) &&
+		echo new >> subfile &&
+		test_tick &&
+		git add subfile &&
+		git commit -m new subfile &&
+		head2=$(git rev-parse --short HEAD) &&
+		echo "From $pwd/submodule" > ../expect.err &&
+		echo "   $head1..$head2  master     -> origin/master" >> ../expect.err
+	)
+	(
+		cd deepsubmodule &&
+		head1=$(git rev-parse --short HEAD) &&
+		echo new >> deepsubfile &&
+		test_tick &&
+		git add deepsubfile &&
+		git commit -m new deepsubfile &&
+		head2=$(git rev-parse --short HEAD) &&
+		echo "From $pwd/deepsubmodule" >> ../expect.err &&
+		echo "   $head1..$head2  master     -> origin/master" >> ../expect.err
+	)
+}
+
+test_expect_success setup '
+	mkdir deepsubmodule &&
+	(
+		cd deepsubmodule &&
+		git init &&
+		echo deepsubcontent > deepsubfile &&
+		git add deepsubfile &&
+		git commit -m new deepsubfile
+	) &&
+	mkdir submodule &&
+	(
+		cd submodule &&
+		git init &&
+		echo subcontent > subfile &&
+		git add subfile &&
+		git submodule add "$pwd/deepsubmodule" deepsubmodule &&
+		git commit -a -m new
+	) &&
+	git submodule add "$pwd/submodule" submodule &&
+	git commit -am initial &&
+	git clone . downstream &&
+	(
+		cd downstream &&
+		git submodule update --init --recursive
+	) &&
+	echo "Fetching submodule submodule" > expect.out &&
+	echo "Fetching submodule submodule/deepsubmodule" >> expect.out
+'
+
+test_expect_success "fetch --recurse-submodules recurses into submodules" '
+	add_upstream_commit &&
+	(
+		cd downstream &&
+		git fetch --recurse-submodules >../actual.out 2>../actual.err
+	) &&
+	test_cmp expect.out actual.out &&
+	test_cmp expect.err actual.err
+'
+
+test_expect_success "fetch alone only fetches superproject" '
+	add_upstream_commit &&
+	(
+		cd downstream &&
+		git fetch >../actual.out 2>../actual.err
+	) &&
+	! test -s actual.out &&
+	! test -s actual.err
+'
+
+test_expect_success "--quiet propagates to submodules" '
+	(
+		cd downstream &&
+		git fetch --recurse-submodules --quiet >../actual.out 2>../actual.err
+	) &&
+	! test -s actual.out &&
+	! test -s actual.err
+'
+
+test_expect_success "--dry-run propagates to submodules" '
+	add_upstream_commit &&
+	(
+		cd downstream &&
+		git fetch --recurse-submodules --dry-run >../actual.out 2>../actual.err
+	) &&
+	test_cmp expect.out actual.out &&
+	test_cmp expect.err actual.err &&
+	(
+		cd downstream &&
+		git fetch --recurse-submodules >../actual.out 2>../actual.err
+	) &&
+	test_cmp expect.out actual.out &&
+	test_cmp expect.err actual.err
+'
+
+test_done
-- 
1.7.3.2.337.g9376c

[PATCH v3 2/3] Add the 'fetch.recurseSubmodules' config setting

[Jens Lehmann]

This new boolean option can be used to override the default for "git
fetch" and "git pull", which is to not recurse into populated submodules
and fetch all new commits there too.

Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de>
---
 Documentation/config.txt        |    5 +++++
 Documentation/fetch-options.txt |    5 +++--
 builtin/fetch.c                 |   21 ++++++++++++++++-----
 git-pull.sh                     |    3 +++
 submodule.c                     |   18 +++++++++++++++++-
 submodule.h                     |    4 +++-
 t/t5526-fetch-submodules.sh     |   36 ++++++++++++++++++++++++++++++++++++
 7 files changed, 83 insertions(+), 9 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 538ebb5..453c1eb 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -879,6 +879,11 @@ diff.wordRegex::
 	sequences that match the regular expression are "words", all other
 	characters are *ignorable* whitespace.

+fetch.recurseSubmodules::
+	A boolean value which changes the behavior for fetch and pull, the
+	default is to not recursively fetch populated sumodules unless
+	configured otherwise.
+
 fetch.unpackLimit::
 	If the number of objects fetched over the git native
 	transfer is below this
diff --git a/Documentation/fetch-options.txt b/Documentation/fetch-options.txt
index bd36466..6aea5f6 100644
--- a/Documentation/fetch-options.txt
+++ b/Documentation/fetch-options.txt
@@ -66,8 +66,9 @@ ifndef::git-pull[]
 	linkgit:git-config[1].
 endif::git-pull[]

---recurse-submodules::
-	Use this option to fetch new commits of all populated submodules too.
+--[no-]recurse-submodules::
+	This option controls if new commits of all populated submodules should
+	be fetched too (see linkgit:git-config[1]).

 ifndef::git-pull[]
 --submodule-prefix::
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 658bb4c..eb2296d 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -28,8 +28,14 @@ enum {
 	TAGS_SET = 2
 };

+enum {
+	RECURSE_SUBMODULES_OFF = 0,
+	RECURSE_SUBMODULES_DEFAULT = 1,
+	RECURSE_SUBMODULES_ON = 2
+};
+
 static int all, append, dry_run, force, keep, multiple, prune, update_head_ok, verbosity;
-static int progress, recurse_submodules;
+static int progress, recurse_submodules = RECURSE_SUBMODULES_DEFAULT;
 static int tags = TAGS_DEFAULT;
 static const char *depth;
 static const char *upload_pack;
@@ -55,8 +61,9 @@ static struct option builtin_fetch_options[] = {
 		    "do not fetch all tags (--no-tags)", TAGS_UNSET),
 	OPT_BOOLEAN('p', "prune", &prune,
 		    "prune tracking branches no longer on remote"),
-	OPT_BOOLEAN(0, "recurse-submodules", &recurse_submodules,
-		    "control recursive fetching of submodules"),
+	OPT_SET_INT(0, "recurse-submodules", &recurse_submodules,
+		    "control recursive fetching of submodules",
+		    RECURSE_SUBMODULES_ON),
 	OPT_BOOLEAN(0, "dry-run", &dry_run,
 		    "dry run"),
 	OPT_BOOLEAN('k', "keep", &keep, "keep downloaded pack"),
@@ -802,7 +809,7 @@ static void add_options_to_argv(int *argc, const char **argv)
 		argv[(*argc)++] = "--force";
 	if (keep)
 		argv[(*argc)++] = "--keep";
-	if (recurse_submodules)
+	if (recurse_submodules == RECURSE_SUBMODULES_ON)
 		argv[(*argc)++] = "--recurse-submodules";
 	if (verbosity >= 2)
 		argv[(*argc)++] = "-v";
@@ -940,14 +947,18 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)
 		}
 	}

-	if (!result && recurse_submodules) {
+	if (!result && (recurse_submodules != RECURSE_SUBMODULES_OFF)) {
 		const char *options[10];
 		int num_options = 0;
+		/* Set recursion as default when we already are recursing */
+		if (submodule_prefix[0])
+			set_config_fetch_recurse_submodules(1);
 		gitmodules_config();
 		git_config(submodule_config, NULL);
 		add_options_to_argv(&num_options, options);
 		result = fetch_populated_submodules(num_options, options,
 						    submodule_prefix,
+						    recurse_submodules == RECURSE_SUBMODULES_ON,
 						    verbosity < 0);
 	}

diff --git a/git-pull.sh b/git-pull.sh
index 5804c62..a5ab195 100755
--- a/git-pull.sh
+++ b/git-pull.sh
@@ -108,6 +108,9 @@ do
 	--recurse-submodules)
 		recurse_submodules=--recurse-submodules
 		;;
+	--no-recurse-submodules)
+		recurse_submodules=--no-recurse-submodules
+		;;
 	--d|--dr|--dry|--dry-|--dry-r|--dry-ru|--dry-run)
 		dry_run=--dry-run
 		;;
diff --git a/submodule.c b/submodule.c
index 4d9b774..01d75f5 100644
--- a/submodule.c
+++ b/submodule.c
@@ -11,6 +11,7 @@

 struct string_list config_name_for_path;
 struct string_list config_ignore_for_name;
+static int config_fetch_recurse_submodules;

 static int add_submodule_odb(const char *path)
 {
@@ -67,6 +68,10 @@ int submodule_config(const char *var, const char *value, void *cb)
 {
 	if (!prefixcmp(var, "submodule."))
 		return parse_submodule_config_option(var, value);
+	else if (!strcmp(var, "fetch.recursesubmodules")) {
+		config_fetch_recurse_submodules = git_config_bool(var, value);
+		return 0;
+	}
 	return 0;
 }

@@ -229,8 +234,14 @@ void show_submodule_summary(FILE *f, const char *path,
 	strbuf_release(&sb);
 }

+void set_config_fetch_recurse_submodules(int value)
+{
+	config_fetch_recurse_submodules = value;
+}
+
 int fetch_populated_submodules(int num_options, const char **options,
-			       const char *prefix, int quiet)
+			       const char *prefix, int ignore_config,
+			       int quiet)
 {
 	int i, result = 0, argc = 0;
 	struct child_process cp;
@@ -271,6 +282,11 @@ int fetch_populated_submodules(int num_options, const char **options,
 		if (name_for_path)
 			name = name_for_path->util;

+		if (!ignore_config) {
+			if (!config_fetch_recurse_submodules)
+				continue;
+		}
+
 		strbuf_addf(&submodule_path, "%s/%s", work_tree, ce->name);
 		strbuf_addf(&submodule_git_dir, "%s/.git", submodule_path.buf);
 		strbuf_addf(&submodule_prefix, "%s%s/", prefix, ce->name);
diff --git a/submodule.h b/submodule.h
index b39d7a1..4729023 100644
--- a/submodule.h
+++ b/submodule.h
@@ -13,8 +13,10 @@ void show_submodule_summary(FILE *f, const char *path,
 		unsigned char one[20], unsigned char two[20],
 		unsigned dirty_submodule,
 		const char *del, const char *add, const char *reset);
+void set_config_fetch_recurse_submodules(int value);
 int fetch_populated_submodules(int num_options, const char **options,
-			       const char *prefix, int quiet);
+			       const char *prefix, int ignore_config,
+			       int quiet);
 unsigned is_submodule_modified(const char *path, int ignore_untracked);
 int merge_submodule(unsigned char result[20], const char *path, const unsigned char base[20],
 		    const unsigned char a[20], const unsigned char b[20]);
diff --git a/t/t5526-fetch-submodules.sh b/t/t5526-fetch-submodules.sh
index 107317a..69ed3e2 100755
--- a/t/t5526-fetch-submodules.sh
+++ b/t/t5526-fetch-submodules.sh
@@ -106,4 +106,40 @@ test_expect_success "--dry-run propagates to submodules" '
 	test_cmp expect.err actual.err
 '

+test_expect_success "recurseSubmodules=true propagates into submodules" '
+	add_upstream_commit &&
+	(
+		cd downstream &&
+		git config fetch.recurseSubmodules true
+		git fetch >../actual.out 2>../actual.err
+	) &&
+	test_cmp expect.out actual.out &&
+	test_cmp expect.err actual.err
+'
+
+test_expect_success "--recurse-submodules overrides config in submodule" '
+	add_upstream_commit &&
+	(
+		cd downstream &&
+		(
+			cd submodule &&
+			git config fetch.recurseSubmodules false
+		) &&
+		git fetch --recurse-submodules >../actual.out 2>../actual.err
+	) &&
+	test_cmp expect.out actual.out &&
+	test_cmp expect.err actual.err
+'
+
+test_expect_success "--no-recurse-submodules overrides config setting" '
+	add_upstream_commit &&
+	(
+		cd downstream &&
+		git config fetch.recurseSubmodules true
+		git fetch --no-recurse-submodules >../actual.out 2>../actual.err
+	) &&
+	! test -s actual.out &&
+	! test -s actual.err
+'
+
 test_done
-- 
1.7.3.2.337.g9376c

[PATCH v3 3/3] Submodules: Add the "fetchRecurseSubmodules" config option

[Jens Lehmann]

The new boolean "fetchRecurseSubmodules" config option controls the
behavior for "git fetch" and "git pull". It specifies if these commands
should recurse into submodules and fetch new commits there too and can be
set separately for each submodule.

In the .gitmodules file "submodule.<name>.fetchRecurseSubmodules" entries
are read before looking for them in .git/config. Thus settings found in
.git/config will override those from .gitmodules, thereby allowing the
user to ignore settings given by the remote side while also letting
upstream set reasonable defaults for those users who don't have special
needs.

This configuration can be overridden by the command line option
"--[no-]recurse-submodules" of "git fetch" and "git pull".

Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de>
---
 Documentation/config.txt        |    7 +++++
 Documentation/fetch-options.txt |    2 +-
 Documentation/gitmodules.txt    |    8 ++++++
 submodule.c                     |   20 ++++++++++++++-
 t/t5526-fetch-submodules.sh     |   50 +++++++++++++++++++++++++++++++++++++++
 5 files changed, 84 insertions(+), 3 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 453c1eb..cad9658 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1796,6 +1796,13 @@ submodule.<name>.update::
 	URL and other values found in the `.gitmodules` file.  See
 	linkgit:git-submodule[1] and linkgit:gitmodules[5] for details.

+submodule.<name>.fetchRecurseSubmodules::
+	This option can be used to enable/disable recursive fetching of this
+	submodule. It can be overriden by using the --[no-]recurse-submodules
+	command line option to "git fetch" and "git pull".
+	This setting will override that from in the linkgit:gitmodules[5]
+	file.
+
 submodule.<name>.ignore::
 	Defines under what circumstances "git status" and the diff family show
 	a submodule as modified. When set to "all", it will never be considered
diff --git a/Documentation/fetch-options.txt b/Documentation/fetch-options.txt
index 6aea5f6..dea7a29 100644
--- a/Documentation/fetch-options.txt
+++ b/Documentation/fetch-options.txt
@@ -68,7 +68,7 @@ endif::git-pull[]

 --[no-]recurse-submodules::
 	This option controls if new commits of all populated submodules should
-	be fetched too (see linkgit:git-config[1]).
+	be fetched too (see linkgit:git-config[1] and linkgit:gitmodules[5]).

 ifndef::git-pull[]
 --submodule-prefix::
diff --git a/Documentation/gitmodules.txt b/Documentation/gitmodules.txt
index bcffd95..6c93202 100644
--- a/Documentation/gitmodules.txt
+++ b/Documentation/gitmodules.txt
@@ -44,6 +44,14 @@ submodule.<name>.update::
 	This config option is overridden if 'git submodule update' is given
 	the '--merge' or '--rebase' options.

+submodule.<name>.fetchRecurseSubmodules::
+	This option can be used to enable/disable recursive fetching of this
+	submodule. If this option is also present in the submodules entry in
+	.git/config of the superproject, the setting there will override the
+	one found in .gitmodules.
+	Both settings can be overriden on the command line by using the
+	"--[no-]recurse-submodules" option to "git fetch" and "git pull"..
+
 submodule.<name>.ignore::
 	Defines under what circumstances "git status" and the diff family show
 	a submodule as modified. When set to "all", it will never be considered
diff --git a/submodule.c b/submodule.c
index 01d75f5..4e62900 100644
--- a/submodule.c
+++ b/submodule.c
@@ -10,6 +10,7 @@
 #include "string-list.h"

 struct string_list config_name_for_path;
+struct string_list config_fetch_recurse_submodules_for_name;
 struct string_list config_ignore_for_name;
 static int config_fetch_recurse_submodules;

@@ -105,6 +106,14 @@ int parse_submodule_config_option(const char *var, const char *value)
 			config = string_list_append(&config_name_for_path, xstrdup(value));
 		config->util = strbuf_detach(&submodname, NULL);
 		strbuf_release(&submodname);
+	} else if ((len > 23) && !strcmp(var + len - 23, ".fetchrecursesubmodules")) {
+		strbuf_add(&submodname, var, len - 23);
+		config = unsorted_string_list_lookup(&config_fetch_recurse_submodules_for_name, submodname.buf);
+		if (!config)
+			config = string_list_append(&config_fetch_recurse_submodules_for_name,
+						    strbuf_detach(&submodname, NULL));
+		config->util = git_config_bool(var, value) ? (void *)1 : NULL;
+		strbuf_release(&submodname);
 	} else if ((len > 7) && !strcmp(var + len - 7, ".ignore")) {
 		if (strcmp(value, "untracked") && strcmp(value, "dirty") &&
 		    strcmp(value, "all") && strcmp(value, "none")) {
@@ -283,8 +292,15 @@ int fetch_populated_submodules(int num_options, const char **options,
 			name = name_for_path->util;

 		if (!ignore_config) {
-			if (!config_fetch_recurse_submodules)
-				continue;
+			struct string_list_item *fetch_recurse_submodules_option;
+			fetch_recurse_submodules_option = unsorted_string_list_lookup(&config_fetch_recurse_submodules_for_name, name);
+			if (fetch_recurse_submodules_option) {
+				if (!fetch_recurse_submodules_option->util)
+					continue;
+			} else {
+				if (!config_fetch_recurse_submodules)
+					continue;
+			}
 		}

 		strbuf_addf(&submodule_path, "%s/%s", work_tree, ce->name);
diff --git a/t/t5526-fetch-submodules.sh b/t/t5526-fetch-submodules.sh
index 69ed3e2..c070faf 100755
--- a/t/t5526-fetch-submodules.sh
+++ b/t/t5526-fetch-submodules.sh
@@ -81,6 +81,56 @@ test_expect_success "fetch alone only fetches superproject" '
 	! test -s actual.err
 '

+test_expect_success "fetch --no-recurse-submodules only fetches superproject" '
+	(
+		cd downstream &&
+		git fetch --no-recurse-submodules >../actual.out 2>../actual.err
+	) &&
+	! test -s actual.out &&
+	! test -s actual.err
+'
+
+test_expect_success "using fetchRecurseSubmodules=true in .gitmodules recurses into submodules" '
+	(
+		cd downstream &&
+		git config -f .gitmodules submodule.submodule.fetchRecurseSubmodules true &&
+		git fetch >../actual.out 2>../actual.err
+	) &&
+	test_cmp expect.out actual.out &&
+	test_cmp expect.err actual.err
+'
+
+test_expect_success "--no-recurse-submodules overrides .gitmodules config" '
+	add_upstream_commit &&
+	(
+		cd downstream &&
+		git fetch --no-recurse-submodules >../actual.out 2>../actual.err
+	) &&
+	! test -s actual.out &&
+	! test -s actual.err
+'
+
+test_expect_success "using fetchRecurseSubmodules=false in .git/config overrides setting in .gitmodules" '
+	(
+		cd downstream &&
+		git config submodule.submodule.fetchRecurseSubmodules false &&
+		git fetch >../actual.out 2>../actual.err
+	) &&
+	! test -s actual.out &&
+	! test -s actual.err
+'
+
+test_expect_success "--recurse-submodules overrides fetchRecurseSubmodules setting from .git/config" '
+	(
+		cd downstream &&
+		git fetch --recurse-submodules >../actual.out 2>../actual.err &&
+		git config -f --unset .gitmodules submodule.submodule.fetchRecurseSubmodules true &&
+		git config --unset submodule.submodule.fetchRecurseSubmodules
+	) &&
+	test_cmp expect.out actual.out &&
+	test_cmp expect.err actual.err
+'
+
 test_expect_success "--quiet propagates to submodules" '
 	(
 		cd downstream &&
-- 
1.7.3.2.337.g9376c

Re: [PATCH v3 2/3] Add the 'fetch.recurseSubmodules' config setting

[Jonathan Nieder]

Jens Lehmann wrote:

> --- a/t/t5526-fetch-submodules.sh
> +++ b/t/t5526-fetch-submodules.sh
> @@ -106,4 +106,40 @@ test_expect_success "--dry-run propagates to submodules" '
>  	test_cmp expect.err actual.err
>  '
> 
> +test_expect_success "recurseSubmodules=true propagates into submodules" '
> +	add_upstream_commit &&
> +	(
> +		cd downstream &&
> +		git config fetch.recurseSubmodules true
> +		git fetch >../actual.out 2>../actual.err
> +	) &&
> +	test_cmp expect.out actual.out &&
> +	test_cmp expect.err actual.err
> +'

This configuration item is read from .gitmodules, too, right?  Would
it be easy (or desirable) to make it not read from there?  Either way,
it would be nice to have a test so the behavior doesn't change without
anyone noticing.

Re: [PATCH v3 0/3] Teach fetch and pull to recursively fetch submodules

[Jonathan Nieder]

Jens Lehmann wrote:

> * Should the "--submodule-prefix" option - which is only used internally
>   now - be a hidden option to "git fetch"?

Yes.  (Any option that is useless outside scripts should be, imho.)

> But nonetheless I think this patch series is ok for inclusion as it does
> not change default behavior and gives people the opportunity to play with
> recursive fetch/pull by enabling one of the introduced config options.

Except for the .gitmodules detail I mentioned, it looks good to me.
Looking forward to trying it out.

Thanks,
Jonathan

Re: [PATCH v3 2/3] Add the 'fetch.recurseSubmodules' config setting

[Jens Lehmann]

Am 11.11.2010 01:02, schrieb Jonathan Nieder:
> Jens Lehmann wrote:
> 
>> --- a/t/t5526-fetch-submodules.sh
>> +++ b/t/t5526-fetch-submodules.sh
>> @@ -106,4 +106,40 @@ test_expect_success "--dry-run propagates to submodules" '
>>  	test_cmp expect.err actual.err
>>  '
>>
>> +test_expect_success "recurseSubmodules=true propagates into submodules" '
>> +	add_upstream_commit &&
>> +	(
>> +		cd downstream &&
>> +		git config fetch.recurseSubmodules true
>> +		git fetch >../actual.out 2>../actual.err
>> +	) &&
>> +	test_cmp expect.out actual.out &&
>> +	test_cmp expect.err actual.err
>> +'
> 
> This configuration item is read from .gitmodules, too, right?  Would
> it be easy (or desirable) to make it not read from there?  Either way,
> it would be nice to have a test so the behavior doesn't change without
> anyone noticing.

"fetch.recurseSubmodules" is only read from .git/config. The one read
first from .gitmodules and then from .git/config is the per-submodule
setting "submodule.<name>.fetchRecurseSubmodules" added in 3/3. But
maybe I should add a test that "fetch.recurseSubmodules" also works
when set in the global config ...

Re: [PATCH v3 0/3] Teach fetch and pull to recursively fetch submodules

[Jens Lehmann]

Am 11.11.2010 01:05, schrieb Jonathan Nieder:
> Jens Lehmann wrote:
> 
>> * Should the "--submodule-prefix" option - which is only used internally
>>   now - be a hidden option to "git fetch"?
> 
> Yes.  (Any option that is useless outside scripts should be, imho.)

Ok, I will change that.

>> But nonetheless I think this patch series is ok for inclusion as it does
>> not change default behavior and gives people the opportunity to play with
>> recursive fetch/pull by enabling one of the introduced config options.
> 
> Except for the .gitmodules detail I mentioned, it looks good to me.
> Looking forward to trying it out.

Thanks!

Re: [PATCH v3 2/3] Add the 'fetch.recurseSubmodules' config setting

[Jonathan Nieder]

Jens Lehmann wrote:
> Am 11.11.2010 01:02, schrieb Jonathan Nieder:

>> This configuration item is read from .gitmodules, too, right?  Would
>> it be easy (or desirable) to make it not read from there?  Either way,
>> it would be nice to have a test so the behavior doesn't change without
>> anyone noticing.
>
> "fetch.recurseSubmodules" is only read from .git/config. The one read
> first from .gitmodules and then from .git/config is the per-submodule
> setting "submodule.<name>.fetchRecurseSubmodules" added in 3/3.

Sorry for the nonsense.  Would it be easy (or desirable) to make
_that_ one not be read from .gitmodules?

>                                                                 But
> maybe I should add a test that "fetch.recurseSubmodules" also works
> when set in the global config ...

Yes, I agree that such a test would be good (though less important
than a test for the behavior with no configuration set at all, say).
A test to demonstrate that configuration aside from submodule.* is not
read from .gitmodules would also be good imho, but I don't think it's
a blocker for anything. :)

Re: [PATCH v3 2/3] Add the 'fetch.recurseSubmodules' config setting

[Junio C Hamano]

Jonathan Nieder <jrnieder@gmail.com> writes:

>> "fetch.recurseSubmodules" is only read from .git/config. The one read
>> first from .gitmodules and then from .git/config is the per-submodule
>> setting "submodule.<name>.fetchRecurseSubmodules" added in 3/3.
>
> Sorry for the nonsense.  Would it be easy (or desirable) to make
> _that_ one not be read from .gitmodules?

I think the motivation behind having a way to read it from .gitmodules is
so that project can suggest the default for convenience (e.g. "almost
everybody who interacts with this project wants these submodules checked
out and kept updated").

Traditionally the suggestions kept in .gitmodules were propagated to the
config when the submodule was initialized, and at runtime we read only
from the config from then on without reading from .gitmodules, so that
once the user decides to follow what the project suggests (or customize
that away), the preference would stick to the repository.

That arrangement does not cater well to people who want to follow along
whatever the project's suggestion of the day, so we might want to change
things so that we if we find it in the config, we stop there and use what
we found, otherwise we use what is in the in-tree gitmodules; I suspect we
might require some changes to "submodule init" not to copy certain things
to the config for that to work, though...

Re: [PATCH v3 2/3] Add the 'fetch.recurseSubmodules' config setting

[Jonathan Nieder]

Junio C Hamano wrote:

> I think the motivation behind having a way to read it from .gitmodules is
> so that project can suggest the default for convenience (e.g. "almost
> everybody who interacts with this project wants these submodules checked
> out and kept updated").

Yes, that makes some sense to me.  Except wouldn't it be a single
configuration item?  "These submodules should be checked out in all
but unusual situations, so check them out automatically and keep them
updated."

Maybe a person setting this to false actually means "This submodule
has its url set to a repository that is updated very frequently, and
most updates are not relevant to the superproject."  Unfortunately, I
think the result would be a poor user experience: when an update comes
that _is_ important to the superproject, what happens?

 $ git fetch
 ... go on plane ...
 $ git merge @{u} && git submodule update --no-fetch --recursive
 [...]
 fatal: reference is not a tree: f1c596a3895643d0969a15b8e945bf0c0072e470

Hmm.  I think in that scenario a better solution would be to point the
submodule url point to a project-specific clone that is updated less
frequently.

What am I missing?

Jonathan

Re: [PATCH v3 2/3] Add the 'fetch.recurseSubmodules' config setting

[Jens Lehmann]

Am 11.11.2010 19:31, schrieb Junio C Hamano:
> Jonathan Nieder <jrnieder@gmail.com> writes:
> 
>>> "fetch.recurseSubmodules" is only read from .git/config. The one read
>>> first from .gitmodules and then from .git/config is the per-submodule
>>> setting "submodule.<name>.fetchRecurseSubmodules" added in 3/3.
>>
>> Sorry for the nonsense.  Would it be easy (or desirable) to make
>> _that_ one not be read from .gitmodules?
> 
> I think the motivation behind having a way to read it from .gitmodules is
> so that project can suggest the default for convenience (e.g. "almost
> everybody who interacts with this project wants these submodules checked
> out and kept updated").

Yes, and to achieve that it should /not/ be necessary to run a "git
submodule sync" manually afterwards to activate those changes.

Jonathan, I think when we allow upstream to configure which submodules
are to be cloned and checked out by default (which seems an option almost
everyone likes to have), doesn't it make sense to let upstream set a
default which submodules should be fetched too (so that new commits there
can be checked out recursively later)? I would rather not like to do that
implicitly just because a submodule is configured for recursive checkout
...


> Traditionally the suggestions kept in .gitmodules were propagated to the
> config when the submodule was initialized, and at runtime we read only
> from the config from then on without reading from .gitmodules, so that
> once the user decides to follow what the project suggests (or customize
> that away), the preference would stick to the repository.
> 
> That arrangement does not cater well to people who want to follow along
> whatever the project's suggestion of the day, so we might want to change
> things so that we if we find it in the config, we stop there and use what
> we found, otherwise we use what is in the in-tree gitmodules; I suspect we
> might require some changes to "submodule init" not to copy certain things
> to the config for that to work, though...

Right. But this would enable you to have branches where different sets
of submodules are fetched, checked out and/or considered to make the
superproject dirty depending on the topic the branch is for.

Re: [PATCH v3 2/3] Add the 'fetch.recurseSubmodules' config setting

[Jens Lehmann]

Am 11.11.2010 20:00, schrieb Jonathan Nieder:
> Junio C Hamano wrote:
> 
>> I think the motivation behind having a way to read it from .gitmodules is
>> so that project can suggest the default for convenience (e.g. "almost
>> everybody who interacts with this project wants these submodules checked
>> out and kept updated").
> 
> Yes, that makes some sense to me.  Except wouldn't it be a single
> configuration item?  "These submodules should be checked out in all
> but unusual situations, so check them out automatically and keep them
> updated."

Hmm, but we have at least three modes of how to update them:

1) Never fetch the submodule (to get new commits the user has to run
   "git fetch --recurse-submodules" by hand)

2) Fetch the submodule each time you fetch the superproject (Which is
   really handy when you do development in the submodule too but can
   be really inconvenient when you don't)

3) Update submodules only when new recorded commits are fetched in
   the superproject (This mode is not added with the current patch
   series but will be in one of the next)

So you would need a config option for that anyway, no? And that is why
I'd rather like to have a separate fetch option to control that behavior
instead of an implicit "if-it's-to-be-checked-out-fetch-it-too" approach.


> Maybe a person setting this to false actually means "This submodule
> has its url set to a repository that is updated very frequently, and
> most updates are not relevant to the superproject."  Unfortunately, I
> think the result would be a poor user experience: when an update comes
> that _is_ important to the superproject, what happens?
> 
>  $ git fetch
>  ... go on plane ...
>  $ git merge @{u} && git submodule update --no-fetch --recursive
>  [...]
>  fatal: reference is not a tree: f1c596a3895643d0969a15b8e945bf0c0072e470
> 
> Hmm.  I think in that scenario a better solution would be to point the
> submodule url point to a project-specific clone that is updated less
> frequently.
> 
> What am I missing?

That situation should be handled by method 3) above which was proposed
for such a use case.

[PATCH v4 1/3] fetch/pull: Add the --recurse-submodules option

[Jens Lehmann]

Until now you had to call "git submodule update" (without -N|--no-fetch
option) or something like "git submodule foreach git fetch" to fetch
new commits in populated submodules from their remote.

This could lead to "(commits not present)" messages in the output of
"git diff --submodule" (which is used by "git gui" and "gitk") after
fetching or pulling new commits in the superproject and is an obstacle for
implementing recursive checkout of submodules. Also "git submodule
update" cannot fetch changes when disconnected, so it was very easy to
forget to fetch the submodule changes before disconnecting only to
discover later that they are needed.

This patch adds the "--recurse-submodules" option to recursively fetch
each populated submodule from the url configured in the .git/config of the
submodule at the end of each "git fetch" or during "git pull" in the
superproject. The submodule paths are taken from the index.

The hidden option "--submodule-prefix" is added to "git fetch" to be able
to print out the full paths of nested submodules.

Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de>
---

Am 11.11.2010 01:05, schrieb Jonathan Nieder:
> Jens Lehmann wrote:
> 
>> * Should the "--submodule-prefix" option - which is only used internally
>>   now - be a hidden option to "git fetch"?
> 
> Yes.  (Any option that is useless outside scripts should be, imho.)

Ok, here is the updated version of patch 1/3 making "--submodule-prefix" a
hidden option which is not documented in Documentation/fetch-options.txt .


 Documentation/fetch-options.txt |    3 +
 builtin/fetch.c                 |   53 ++++++++++++++-----
 git-pull.sh                     |    7 ++-
 submodule.c                     |   66 +++++++++++++++++++++++-
 submodule.h                     |    3 +
 t/t5526-fetch-submodules.sh     |  109 +++++++++++++++++++++++++++++++++++++++
 6 files changed, 224 insertions(+), 17 deletions(-)
 create mode 100755 t/t5526-fetch-submodules.sh

diff --git a/Documentation/fetch-options.txt b/Documentation/fetch-options.txt
index 5ce1e72..85b7f84 100644
--- a/Documentation/fetch-options.txt
+++ b/Documentation/fetch-options.txt
@@ -66,6 +66,9 @@ ifndef::git-pull[]
 	linkgit:git-config[1].
 endif::git-pull[]

+--recurse-submodules::
+	Use this option to fetch new commits of all populated submodules too.
+
 -u::
 --update-head-ok::
 	By default 'git fetch' refuses to update the head which
diff --git a/builtin/fetch.c b/builtin/fetch.c
index d35f000..db3fba3 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -12,6 +12,7 @@
 #include "parse-options.h"
 #include "sigchain.h"
 #include "transport.h"
+#include "submodule.h"

 static const char * const builtin_fetch_usage[] = {
 	"git fetch [<options>] [<repository> [<refspec>...]]",
@@ -28,12 +29,13 @@ enum {
 };

 static int all, append, dry_run, force, keep, multiple, prune, update_head_ok, verbosity;
-static int progress;
+static int progress, recurse_submodules;
 static int tags = TAGS_DEFAULT;
 static const char *depth;
 static const char *upload_pack;
 static struct strbuf default_rla = STRBUF_INIT;
 static struct transport *transport;
+static const char *submodule_prefix = "";

 static struct option builtin_fetch_options[] = {
 	OPT__VERBOSITY(&verbosity),
@@ -53,6 +55,8 @@ static struct option builtin_fetch_options[] = {
 		    "do not fetch all tags (--no-tags)", TAGS_UNSET),
 	OPT_BOOLEAN('p', "prune", &prune,
 		    "prune tracking branches no longer on remote"),
+	OPT_BOOLEAN(0, "recurse-submodules", &recurse_submodules,
+		    "control recursive fetching of submodules"),
 	OPT_BOOLEAN(0, "dry-run", &dry_run,
 		    "dry run"),
 	OPT_BOOLEAN('k', "keep", &keep, "keep downloaded pack"),
@@ -61,6 +65,8 @@ static struct option builtin_fetch_options[] = {
 	OPT_BOOLEAN(0, "progress", &progress, "force progress reporting"),
 	OPT_STRING(0, "depth", &depth, "DEPTH",
 		   "deepen history of shallow clone"),
+	{ OPTION_STRING, 0, "submodule-prefix", &submodule_prefix, "DIR",
+		   "prepend this to submodule path output", PARSE_OPT_HIDDEN },
 	OPT_END()
 };

@@ -784,28 +790,36 @@ static int add_remote_or_group(const char *name, struct string_list *list)
 	return 1;
 }

-static int fetch_multiple(struct string_list *list)
+static void add_options_to_argv(int *argc, const char **argv)
 {
-	int i, result = 0;
-	const char *argv[11] = { "fetch", "--append" };
-	int argc = 2;
-
 	if (dry_run)
-		argv[argc++] = "--dry-run";
+		argv[(*argc)++] = "--dry-run";
 	if (prune)
-		argv[argc++] = "--prune";
+		argv[(*argc)++] = "--prune";
 	if (update_head_ok)
-		argv[argc++] = "--update-head-ok";
+		argv[(*argc)++] = "--update-head-ok";
 	if (force)
-		argv[argc++] = "--force";
+		argv[(*argc)++] = "--force";
 	if (keep)
-		argv[argc++] = "--keep";
+		argv[(*argc)++] = "--keep";
+	if (recurse_submodules)
+		argv[(*argc)++] = "--recurse-submodules";
 	if (verbosity >= 2)
-		argv[argc++] = "-v";
+		argv[(*argc)++] = "-v";
 	if (verbosity >= 1)
-		argv[argc++] = "-v";
+		argv[(*argc)++] = "-v";
 	else if (verbosity < 0)
-		argv[argc++] = "-q";
+		argv[(*argc)++] = "-q";
+
+}
+
+static int fetch_multiple(struct string_list *list)
+{
+	int i, result = 0;
+	const char *argv[12] = { "fetch", "--append" };
+	int argc = 2;
+
+	add_options_to_argv(&argc, argv);

 	if (!append && !dry_run) {
 		int errcode = truncate_fetch_head();
@@ -926,6 +940,17 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)
 		}
 	}

+	if (!result && recurse_submodules) {
+		const char *options[10];
+		int num_options = 0;
+		gitmodules_config();
+		git_config(submodule_config, NULL);
+		add_options_to_argv(&num_options, options);
+		result = fetch_populated_submodules(num_options, options,
+						    submodule_prefix,
+						    verbosity < 0);
+	}
+
 	/* All names were strdup()ed or strndup()ed */
 	list.strdup_strings = 1;
 	string_list_clear(&list, 0);
diff --git a/git-pull.sh b/git-pull.sh
index 8eb74d4..5804c62 100755
--- a/git-pull.sh
+++ b/git-pull.sh
@@ -38,7 +38,7 @@ test -z "$(git ls-files -u)" || die_conflict
 test -f "$GIT_DIR/MERGE_HEAD" && die_merge

 strategy_args= diffstat= no_commit= squash= no_ff= ff_only=
-log_arg= verbosity= progress=
+log_arg= verbosity= progress= recurse_submodules=
 merge_args=
 curr_branch=$(git symbolic-ref -q HEAD)
 curr_branch_short="${curr_branch#refs/heads/}"
@@ -105,6 +105,9 @@ do
 	--no-r|--no-re|--no-reb|--no-reba|--no-rebas|--no-rebase)
 		rebase=false
 		;;
+	--recurse-submodules)
+		recurse_submodules=--recurse-submodules
+		;;
 	--d|--dr|--dry|--dry-|--dry-r|--dry-ru|--dry-run)
 		dry_run=--dry-run
 		;;
@@ -220,7 +223,7 @@ test true = "$rebase" && {
 	done
 }
 orig_head=$(git rev-parse -q --verify HEAD)
-git fetch $verbosity $progress $dry_run --update-head-ok "$@" || exit 1
+git fetch $verbosity $progress $dry_run $recurse_submodules --update-head-ok "$@" || exit 1
 test -z "$dry_run" || exit 0

 curr_head=$(git rev-parse -q --verify HEAD)
diff --git a/submodule.c b/submodule.c
index 91a4758..4d9b774 100644
--- a/submodule.c
+++ b/submodule.c
@@ -63,7 +63,7 @@ void set_diffopt_flags_from_submodule_config(struct diff_options *diffopt,
 	}
 }

-static int submodule_config(const char *var, const char *value, void *cb)
+int submodule_config(const char *var, const char *value, void *cb)
 {
 	if (!prefixcmp(var, "submodule."))
 		return parse_submodule_config_option(var, value);
@@ -229,6 +229,70 @@ void show_submodule_summary(FILE *f, const char *path,
 	strbuf_release(&sb);
 }

+int fetch_populated_submodules(int num_options, const char **options,
+			       const char *prefix, int quiet)
+{
+	int i, result = 0, argc = 0;
+	struct child_process cp;
+	const char **argv;
+	struct string_list_item *name_for_path;
+	const char *work_tree = get_git_work_tree();
+	if (!work_tree)
+		return 0;
+
+	if (!the_index.initialized)
+		if (read_cache() < 0)
+			die("index file corrupt");
+
+	argv = xcalloc(num_options + 5, sizeof(const char *));
+	argv[argc++] = "fetch";
+	for (i = 0; i < num_options; i++)
+		argv[argc++] = options[i];
+	argv[argc++] = "--submodule-prefix";
+
+	memset(&cp, 0, sizeof(cp));
+	cp.argv = argv;
+	cp.env = local_repo_env;
+	cp.git_cmd = 1;
+	cp.no_stdin = 1;
+
+	for (i = 0; i < active_nr; i++) {
+		struct strbuf submodule_path = STRBUF_INIT;
+		struct strbuf submodule_git_dir = STRBUF_INIT;
+		struct strbuf submodule_prefix = STRBUF_INIT;
+		struct cache_entry *ce = active_cache[i];
+		const char *git_dir, *name;
+
+		if (!S_ISGITLINK(ce->ce_mode))
+			continue;
+
+		name = ce->name;
+		name_for_path = unsorted_string_list_lookup(&config_name_for_path, ce->name);
+		if (name_for_path)
+			name = name_for_path->util;
+
+		strbuf_addf(&submodule_path, "%s/%s", work_tree, ce->name);
+		strbuf_addf(&submodule_git_dir, "%s/.git", submodule_path.buf);
+		strbuf_addf(&submodule_prefix, "%s%s/", prefix, ce->name);
+		git_dir = read_gitfile_gently(submodule_git_dir.buf);
+		if (!git_dir)
+			git_dir = submodule_git_dir.buf;
+		if (is_directory(git_dir)) {
+			if (!quiet)
+				printf("Fetching submodule %s%s\n", prefix, ce->name);
+			cp.dir = submodule_path.buf;
+			argv[argc] = submodule_prefix.buf;
+			if (run_command(&cp))
+				result = 1;
+		}
+		strbuf_release(&submodule_path);
+		strbuf_release(&submodule_git_dir);
+		strbuf_release(&submodule_prefix);
+	}
+	free(argv);
+	return result;
+}
+
 unsigned is_submodule_modified(const char *path, int ignore_untracked)
 {
 	ssize_t len;
diff --git a/submodule.h b/submodule.h
index 386f410..b39d7a1 100644
--- a/submodule.h
+++ b/submodule.h
@@ -5,6 +5,7 @@ struct diff_options;

 void set_diffopt_flags_from_submodule_config(struct diff_options *diffopt,
 		const char *path);
+int submodule_config(const char *var, const char *value, void *cb);
 void gitmodules_config();
 int parse_submodule_config_option(const char *var, const char *value);
 void handle_ignore_submodules_arg(struct diff_options *diffopt, const char *);
@@ -12,6 +13,8 @@ void show_submodule_summary(FILE *f, const char *path,
 		unsigned char one[20], unsigned char two[20],
 		unsigned dirty_submodule,
 		const char *del, const char *add, const char *reset);
+int fetch_populated_submodules(int num_options, const char **options,
+			       const char *prefix, int quiet);
 unsigned is_submodule_modified(const char *path, int ignore_untracked);
 int merge_submodule(unsigned char result[20], const char *path, const unsigned char base[20],
 		    const unsigned char a[20], const unsigned char b[20]);
diff --git a/t/t5526-fetch-submodules.sh b/t/t5526-fetch-submodules.sh
new file mode 100755
index 0000000..107317a
--- /dev/null
+++ b/t/t5526-fetch-submodules.sh
@@ -0,0 +1,109 @@
+#!/bin/sh
+# Copyright (c) 2010, Jens Lehmann
+
+test_description='Recursive "git fetch" for submodules'
+
+. ./test-lib.sh
+
+pwd=$(pwd)
+
+add_upstream_commit() {
+	(
+		cd submodule &&
+		head1=$(git rev-parse --short HEAD) &&
+		echo new >> subfile &&
+		test_tick &&
+		git add subfile &&
+		git commit -m new subfile &&
+		head2=$(git rev-parse --short HEAD) &&
+		echo "From $pwd/submodule" > ../expect.err &&
+		echo "   $head1..$head2  master     -> origin/master" >> ../expect.err
+	)
+	(
+		cd deepsubmodule &&
+		head1=$(git rev-parse --short HEAD) &&
+		echo new >> deepsubfile &&
+		test_tick &&
+		git add deepsubfile &&
+		git commit -m new deepsubfile &&
+		head2=$(git rev-parse --short HEAD) &&
+		echo "From $pwd/deepsubmodule" >> ../expect.err &&
+		echo "   $head1..$head2  master     -> origin/master" >> ../expect.err
+	)
+}
+
+test_expect_success setup '
+	mkdir deepsubmodule &&
+	(
+		cd deepsubmodule &&
+		git init &&
+		echo deepsubcontent > deepsubfile &&
+		git add deepsubfile &&
+		git commit -m new deepsubfile
+	) &&
+	mkdir submodule &&
+	(
+		cd submodule &&
+		git init &&
+		echo subcontent > subfile &&
+		git add subfile &&
+		git submodule add "$pwd/deepsubmodule" deepsubmodule &&
+		git commit -a -m new
+	) &&
+	git submodule add "$pwd/submodule" submodule &&
+	git commit -am initial &&
+	git clone . downstream &&
+	(
+		cd downstream &&
+		git submodule update --init --recursive
+	) &&
+	echo "Fetching submodule submodule" > expect.out &&
+	echo "Fetching submodule submodule/deepsubmodule" >> expect.out
+'
+
+test_expect_success "fetch --recurse-submodules recurses into submodules" '
+	add_upstream_commit &&
+	(
+		cd downstream &&
+		git fetch --recurse-submodules >../actual.out 2>../actual.err
+	) &&
+	test_cmp expect.out actual.out &&
+	test_cmp expect.err actual.err
+'
+
+test_expect_success "fetch alone only fetches superproject" '
+	add_upstream_commit &&
+	(
+		cd downstream &&
+		git fetch >../actual.out 2>../actual.err
+	) &&
+	! test -s actual.out &&
+	! test -s actual.err
+'
+
+test_expect_success "--quiet propagates to submodules" '
+	(
+		cd downstream &&
+		git fetch --recurse-submodules --quiet >../actual.out 2>../actual.err
+	) &&
+	! test -s actual.out &&
+	! test -s actual.err
+'
+
+test_expect_success "--dry-run propagates to submodules" '
+	add_upstream_commit &&
+	(
+		cd downstream &&
+		git fetch --recurse-submodules --dry-run >../actual.out 2>../actual.err
+	) &&
+	test_cmp expect.out actual.out &&
+	test_cmp expect.err actual.err &&
+	(
+		cd downstream &&
+		git fetch --recurse-submodules >../actual.out 2>../actual.err
+	) &&
+	test_cmp expect.out actual.out &&
+	test_cmp expect.err actual.err
+'
+
+test_done
-- 
1.7.3.2.335.g2e3006

Re: [PATCH v3 2/3] Add the 'fetch.recurseSubmodules' config setting

[Jonathan Nieder]

Jens Lehmann wrote:
> Am 11.11.2010 20:00, schrieb Jonathan Nieder:

>> Yes, that makes some sense to me.  Except wouldn't it be a single
>> configuration item?  "These submodules should be checked out in all
>> but unusual situations, so check them out automatically and keep them
>> updated."
>
> Hmm, but we have at least three modes of how to update them:
>
> 1) Never fetch the submodule (to get new commits the user has to run
>    "git fetch --recurse-submodules" by hand)
>
> 2) Fetch the submodule each time you fetch the superproject (Which is
>    really handy when you do development in the submodule too but can
>    be really inconvenient when you don't)
>
> 3) Update submodules only when new recorded commits are fetched in
>    the superproject (This mode is not added with the current patch
>    series but will be in one of the next)
>
> So you would need a config option for that anyway, no? And that is why
> I'd rather like to have a separate fetch option to control that behavior
> instead of an implicit "if-it's-to-be-checked-out-fetch-it-too" approach.

I still think I am missing something.

Traditionally, git has been a _content_ tracker.  The configuration that
gets transmitted (.gitignore, .gitattributes, .gitmodules) would only
represent basic information needed for that content to remain usable and
sensible.

In the case of .gitmodules, it seems that two concepts are being
conflated:

 A. Configuration based on the user's preferences.  Absolutely, a person
    deserves to be able to easily choose between (1), (2), and (3) as
    described above.

 B. Metadata about the content that should be shared.  For example, "this
    submodule would be checked out in all but unusual circumstances" is a
    useful thing to be able to declare.

Probably I am missing something big, but fetchsubmodules as currently
defined seem like something in category A and not in category B.
Partially because if we ever implement option (3), that is what almost
_every_ casual consumer will want.  So why should they be stuck with these
configuration files specifying (1) and (2) when they check out old
revisions?

All that said, I do not think it is unreasonable in some situations for a
project to want to share configuration of type (A) between members of a
project; for example, lots of projects share hooks and that's great.  I
just don't think git should set it up automatically --- better to require
an explicit user action like "sh useprojectconfiguration.sh" (an action
more explicit than "please initialize and checkout all relevant
submodules") for such cases.

Hoping that is clearer,
Jonathan

Re: [PATCH v3 2/3] Add the 'fetch.recurseSubmodules' config setting

[Jens Lehmann]

Am 12.11.2010 16:52, schrieb Jonathan Nieder:
> Jens Lehmann wrote:
>> Am 11.11.2010 20:00, schrieb Jonathan Nieder:
> 
>>> Yes, that makes some sense to me.  Except wouldn't it be a single
>>> configuration item?  "These submodules should be checked out in all
>>> but unusual situations, so check them out automatically and keep them
>>> updated."
>>
>> Hmm, but we have at least three modes of how to update them:
>>
>> 1) Never fetch the submodule (to get new commits the user has to run
>>    "git fetch --recurse-submodules" by hand)
>>
>> 2) Fetch the submodule each time you fetch the superproject (Which is
>>    really handy when you do development in the submodule too but can
>>    be really inconvenient when you don't)
>>
>> 3) Update submodules only when new recorded commits are fetched in
>>    the superproject (This mode is not added with the current patch
>>    series but will be in one of the next)
>>
>> So you would need a config option for that anyway, no? And that is why
>> I'd rather like to have a separate fetch option to control that behavior
>> instead of an implicit "if-it's-to-be-checked-out-fetch-it-too" approach.
> 
> I still think I am missing something.

Ok, I'll try harder to explain my view ;-)

> Traditionally, git has been a _content_ tracker.  The configuration that
> gets transmitted (.gitignore, .gitattributes, .gitmodules) would only
> represent basic information needed for that content to remain usable and
> sensible.

Ack.

> In the case of .gitmodules, it seems that two concepts are being
> conflated:
> 
>  A. Configuration based on the user's preferences.  Absolutely, a person
>     deserves to be able to easily choose between (1), (2), and (3) as
>     described above.
> 
>  B. Metadata about the content that should be shared.  For example, "this
>     submodule would be checked out in all but unusual circumstances" is a
>     useful thing to be able to declare.

No, I think these concepts aren't conflated at all:

- Category A is handled by .git/config

- Category B is handled by the .gitmodules file

Category A should not be handled by a file in the work tree while
category B stuff belongs there to be able to share it. That is why git
searches all new options in both files with the values from .git/config
overriding those from .gitmodules: to provide useful defaults and let
the user choose otherwise if he wants to.

> Probably I am missing something big, but fetchsubmodules as currently
> defined seem like something in category A and not in category B.
> Partially because if we ever implement option (3), that is what almost
> _every_ casual consumer will want.  So why should they be stuck with these
> configuration files specifying (1) and (2) when they check out old
> revisions?

With submodules there never is a one-size-fits-all solution :-)
(And we are currently working on option (3), so it will exist rather
sooner than later)

There are people putting lots of large files in submodules for better
performance and they almost always never want to fetch (or even stat)
them, so (1) is for them and it's cool that their upstream can configure
that, avoiding to have every developer to repeat their "obvious" choice
after each clone again (and that might only be needed for some submodules,
so a repo-wide config doesn't necessarily help them).

And when you are on a superproject branch actively developing inside a
submodule, you may want to increase fetch-activity to fetch all new
commits in the submodule even if they aren't referenced in the
superproject (yet), as that might be just what your fellow developers
are about to do. And the person setting up that branch could do that
once for all users so they don't have to repeat it in every clone. And
when switching away from that branch all those developers cannot forget
to reconfigure to fetch-on-demand, so not having that in .git/config is
a plus here too.

And right, "almost _every_ casual consumer" should start with (3), and
that is why it will be the default. So most people will get along
without having ever to configure fetch behavior in .gitmodules because
git does The Right Thing for them. But in my opinion we should allow
that for those other users ...

> All that said, I do not think it is unreasonable in some situations for a
> project to want to share configuration of type (A) between members of a
> project; for example, lots of projects share hooks and that's great.  I
> just don't think git should set it up automatically --- better to require
> an explicit user action like "sh useprojectconfiguration.sh" (an action
> more explicit than "please initialize and checkout all relevant
> submodules") for such cases.

You have no other choice for hooks because of security concerns. But I
can't see any downsides in leaving upstream *the choice* to configure
default submodule behavior. Lots of people - including me - want that for
clone and checkout. And to be consistent the same configuration abilities
should be available for fetch too, and if only to follow the principle of
least surprise.


Thanks
Jens

Re: [PATCH v4 1/3] fetch/pull: Add the --recurse-submodules option

[Jonathan Nieder]

Jens Lehmann wrote:

> Ok, here is the updated version of patch 1/3 making "--submodule-prefix" a
> hidden option which is not documented in Documentation/fetch-options.txt .

Oh, I missed that you were proposing leaving it undocumented.  On one hand,
it is nice when debugging if all options are documented; but on the other
hand, there is precedent in --rebasing for hiding implementation details
like this.  Unlike -h output, the manual is not so space-constrainted, so...

> --- /dev/null
> +++ b/t/t5526-fetch-submodules.sh
> @@ -0,0 +1,109 @@
> +#!/bin/sh
> +# Copyright (c) 2010, Jens Lehmann
> +
> +test_description='Recursive "git fetch" for submodules'
> +
> +. ./test-lib.sh
> +
> +pwd=$(pwd)
> +
> +add_upstream_commit() {
> +	(
> +		cd submodule &&
> +		head1=$(git rev-parse --short HEAD) &&
> +		echo new >> subfile &&
> +		test_tick &&
> +		git add subfile &&
> +		git commit -m new subfile &&
> +		head2=$(git rev-parse --short HEAD) &&
> +		echo "From $pwd/submodule" > ../expect.err &&
> +		echo "   $head1..$head2  master     -> origin/master" >> ../expect.err
> +	)
> +	(

Missing &&?

Hope that helps.  These are just tiny nitpicks; I am happy with the patch
with or without them.

diff --git a/Documentation/fetch-options.txt b/Documentation/fetch-options.txt
index 594f7ce..a9cb8ab 100644
--- a/Documentation/fetch-options.txt
+++ b/Documentation/fetch-options.txt
@@ -67,6 +67,13 @@ endif::git-pull[]
 --recurse-submodules::
 	Use this option to fetch new commits of all populated submodules too.
 
+ifndef::git-pull[]
+--submodule-prefix=<path>::
+	Prepend <path> to paths printed in informative messages
+	such as "Fetching submodule foo".  This option is used
+	internally when recursing over submodules.
+endif::git-pull[]
+
 -u::
 --update-head-ok::
 	By default 'git fetch' refuses to update the head which
diff --git a/t/t5526-fetch-submodules.sh b/t/t5526-fetch-submodules.sh
index 107317a..9c1d3a0 100755
--- a/t/t5526-fetch-submodules.sh
+++ b/t/t5526-fetch-submodules.sh
@@ -18,7 +18,7 @@ add_upstream_commit() {
 		head2=$(git rev-parse --short HEAD) &&
 		echo "From $pwd/submodule" > ../expect.err &&
 		echo "   $head1..$head2  master     -> origin/master" >> ../expect.err
-	)
+	) &&
 	(
 		cd deepsubmodule &&
 		head1=$(git rev-parse --short HEAD) &&

Re: [PATCH v3 2/3] Add the 'fetch.recurseSubmodules' config setting

[Jonathan Nieder]

Jens Lehmann wrote:

> No, I think these concepts aren't conflated at all:
> 
> - Category A is handled by .git/config
> 
> - Category B is handled by the .gitmodules file

I meant that the two files currently support the same submodule.*
options.

> There are people putting lots of large files in submodules for better
> performance and they almost always never want to fetch (or even stat)
> them, so (1) is for them and it's cool that their upstream can configure
> that, avoiding to have every developer to repeat their "obvious" choice
> after each clone again (and that might only be needed for some submodules,
> so a repo-wide config doesn't necessarily help them).

Wouldn't (3) work for these people, too?

I think we are getting closer to an explanation.  I can look into
adding documentation for this on top when finished.

> And when you are on a superproject branch actively developing inside a
> submodule, you may want to increase fetch-activity to fetch all new
> commits in the submodule even if they aren't referenced in the
> superproject (yet), as that might be just what your fellow developers
> are about to do. And the person setting up that branch could do that
> once for all users so they don't have to repeat it in every clone.

This one seems less reasonable to me.  It seems like a way to
remotely help developers get a nice setup, rather than a declaration
about the content.

Let me take an unrelated example to illustrate what I mean.  Some
projects might want all their developers to use branch.autosetuprebase,
to avoid confusion since the update hook is going to reject mergy
history anyway.  That seems like a perfectly reasonable desire to me,
and I'd encourage them to add a script that sets everything up
following the policies of their project.

Now git could even learn to read a .gitconfig file including settings
like that one that do not have a security impact.  It would make lots
of people happy, and individuals could override settings they really
dislike in ~/.gitconfig.  Should we do it?

I think no, for reasons of intuitiveness and predictability.

On the other hand, scenarios like (1) might mean we have to support
such things despite that downside.

> And
> when switching away from that branch all those developers cannot forget
> to reconfigure to fetch-on-demand, so not having that in .git/config is
> a plus here too.

Yes, the "read .gitmodules first and then .git/config" is a very nice
enhancement --- thanks!

> You have no other choice for hooks because of security concerns. But I
> can't see any downsides in leaving upstream *the choice* to configure
> default submodule behavior. Lots of people - including me - want that for
> clone and checkout.

There is one setting that it is obvious to me for upstream should be
able to set:

	"these submodules are a necessary part of the project;
	 always (at clone time, fetch time, checkout, etc) make
	 sure they are available"

I could easily be convinced about others, but there ought to be a use
case to outweigh the "subtle behavior changing behind my back" syndrome.

And again: thanks for doing all this work.  It's inspiring.  (Next step
recursive push?)
Jonathan

Re: [PATCH v4 1/3] fetch/pull: Add the --recurse-submodules option

[Jens Lehmann]

Am 12.11.2010 20:54, schrieb Jonathan Nieder:
> Jens Lehmann wrote:
> 
>> Ok, here is the updated version of patch 1/3 making "--submodule-prefix" a
>> hidden option which is not documented in Documentation/fetch-options.txt .
> 
> Oh, I missed that you were proposing leaving it undocumented.  On one hand,
> it is nice when debugging if all options are documented; but on the other
> hand, there is precedent in --rebasing for hiding implementation details
> like this.  Unlike -h output, the manual is not so space-constrainted, so...

Whoops, my question was about hiding the option and so I have been a bit
overeager to remove the documentation too ...

>> --- /dev/null
>> +++ b/t/t5526-fetch-submodules.sh
>> @@ -0,0 +1,109 @@
>> +#!/bin/sh
>> +# Copyright (c) 2010, Jens Lehmann
>> +
>> +test_description='Recursive "git fetch" for submodules'
>> +
>> +. ./test-lib.sh
>> +
>> +pwd=$(pwd)
>> +
>> +add_upstream_commit() {
>> +	(
>> +		cd submodule &&
>> +		head1=$(git rev-parse --short HEAD) &&
>> +		echo new >> subfile &&
>> +		test_tick &&
>> +		git add subfile &&
>> +		git commit -m new subfile &&
>> +		head2=$(git rev-parse --short HEAD) &&
>> +		echo "From $pwd/submodule" > ../expect.err &&
>> +		echo "   $head1..$head2  master     -> origin/master" >> ../expect.err
>> +	)
>> +	(
> 
> Missing &&?

Good eyes!

> Hope that helps.  These are just tiny nitpicks; I am happy with the patch
> with or without them.

Thanks!

Junio, do you want me to send an updated version of this patch or are you ok
with squashing this in?


> diff --git a/Documentation/fetch-options.txt b/Documentation/fetch-options.txt
> index 594f7ce..a9cb8ab 100644
> --- a/Documentation/fetch-options.txt
> +++ b/Documentation/fetch-options.txt
> @@ -67,6 +67,13 @@ endif::git-pull[]
>  --recurse-submodules::
>  	Use this option to fetch new commits of all populated submodules too.
>  
> +ifndef::git-pull[]
> +--submodule-prefix=<path>::
> +	Prepend <path> to paths printed in informative messages
> +	such as "Fetching submodule foo".  This option is used
> +	internally when recursing over submodules.
> +endif::git-pull[]
> +
>  -u::
>  --update-head-ok::
>  	By default 'git fetch' refuses to update the head which
> diff --git a/t/t5526-fetch-submodules.sh b/t/t5526-fetch-submodules.sh
> index 107317a..9c1d3a0 100755
> --- a/t/t5526-fetch-submodules.sh
> +++ b/t/t5526-fetch-submodules.sh
> @@ -18,7 +18,7 @@ add_upstream_commit() {
>  		head2=$(git rev-parse --short HEAD) &&
>  		echo "From $pwd/submodule" > ../expect.err &&
>  		echo "   $head1..$head2  master     -> origin/master" >> ../expect.err
> -	)
> +	) &&
>  	(
>  		cd deepsubmodule &&
>  		head1=$(git rev-parse --short HEAD) &&
> 

Re: [PATCH v3 2/3] Add the 'fetch.recurseSubmodules' config setting

[Jens Lehmann]

Am 12.11.2010 21:16, schrieb Jonathan Nieder:
> Jens Lehmann wrote:
>> There are people putting lots of large files in submodules for better
>> performance and they almost always never want to fetch (or even stat)
>> them, so (1) is for them and it's cool that their upstream can configure
>> that, avoiding to have every developer to repeat their "obvious" choice
>> after each clone again (and that might only be needed for some submodules,
>> so a repo-wide config doesn't necessarily help them).
> 
> Wouldn't (3) work for these people, too?

As I understand there are people who use submodules as something rather
unrelated to the superproject line of development and thus recursive on
demand fetch (and recursive checkout) would not be what they want (and
could take considerable time they don't want to spend).

> I think we are getting closer to an explanation.  I can look into
> adding documentation for this on top when finished.

That would be great! I had the same thing in mind so I would be happy
to help you with that if you want.

>> And when you are on a superproject branch actively developing inside a
>> submodule, you may want to increase fetch-activity to fetch all new
>> commits in the submodule even if they aren't referenced in the
>> superproject (yet), as that might be just what your fellow developers
>> are about to do. And the person setting up that branch could do that
>> once for all users so they don't have to repeat it in every clone.
> 
> This one seems less reasonable to me.  It seems like a way to
> remotely help developers get a nice setup, rather than a declaration
> about the content.

Yeah, I know this example is not as convincing as (1). But it was the
best I could come up with trying to illustrate the "treat-submodules-
as-if-everything-were-in-one-tree" model which some people use.
(And we use this model at my dayjob, but we would be fine setting the
global config option once on every developers computer. But hey, adding
that to the .gitmodules file would get rid of that too ;-)

> Let me take an unrelated example to illustrate what I mean.  Some
> projects might want all their developers to use branch.autosetuprebase,
> to avoid confusion since the update hook is going to reject mergy
> history anyway.  That seems like a perfectly reasonable desire to me,
> and I'd encourage them to add a script that sets everything up
> following the policies of their project.
> 
> Now git could even learn to read a .gitconfig file including settings
> like that one that do not have a security impact.  It would make lots
> of people happy, and individuals could override settings they really
> dislike in ~/.gitconfig.  Should we do it?

Hmm, I never thought of that. But you are right, this is the same
principle I'm using for .gitmodules.

> I think no, for reasons of intuitiveness and predictability.
> 
> On the other hand, scenarios like (1) might mean we have to support
> such things despite that downside.

Yup.

>> And
>> when switching away from that branch all those developers cannot forget
>> to reconfigure to fetch-on-demand, so not having that in .git/config is
>> a plus here too.
> 
> Yes, the "read .gitmodules first and then .git/config" is a very nice
> enhancement --- thanks!

You are welcome!

>> You have no other choice for hooks because of security concerns. But I
>> can't see any downsides in leaving upstream *the choice* to configure
>> default submodule behavior. Lots of people - including me - want that for
>> clone and checkout.
> 
> There is one setting that it is obvious to me for upstream should be
> able to set:
> 
> 	"these submodules are a necessary part of the project;
> 	 always (at clone time, fetch time, checkout, etc) make
> 	 sure they are available"
> 
> I could easily be convinced about others, but there ought to be a use
> case to outweigh the "subtle behavior changing behind my back" syndrome.

I don't have a problem with dropping 3/3 from this series if use case (1)
doesn't convince people. I added it to be consistent with the behavior of
the 'ignore' flag I added earlier and because I it is needed to support
more than one use case for submodules in the same superproject. But we
could wait if someone complains and add it if that happens ... I dunno.

> And again: thanks for doing all this work.  It's inspiring.  (Next step
> recursive push?)

Thanks a lot! (And nope, it's recursive checkout - thats why I started
recursive fetch, so that I do have something to check out ;-).

Jens

Re: [PATCH v4 1/3] fetch/pull: Add the --recurse-submodules option

[Junio C Hamano]

Jens Lehmann <Jens.Lehmann@web.de> writes:

> Until now you had to call "git submodule update" (without -N|--no-fetch
> option) or something like "git submodule foreach git fetch" to fetch
> new commits in populated submodules from their remote.
> ...
> diff --git a/builtin/fetch.c b/builtin/fetch.c
> index d35f000..db3fba3 100644
> --- a/builtin/fetch.c
> +++ b/builtin/fetch.c
> ...
> @@ -784,28 +790,36 @@ static int add_remote_or_group(const char *name, struct string_list *list)
>  	return 1;
>  }
>
> -static int fetch_multiple(struct string_list *list)
> +static void add_options_to_argv(int *argc, const char **argv)
>  {
> -	int i, result = 0;
> -	const char *argv[11] = { "fetch", "--append" };
> -...
> +static int fetch_multiple(struct string_list *list)
> +{
> +	int i, result = 0;
> +	const char *argv[12] = { "fetch", "--append" };

This used to be 11; are we adding something new?  Ahh, possibly
"--recurse_submodules".

> diff --git a/submodule.c b/submodule.c
> index 91a4758..4d9b774 100644
> --- a/submodule.c
> +++ b/submodule.c
> @@ -63,7 +63,7 @@ void set_diffopt_flags_from_submodule_config(struct diff_options *diffopt,
>  	}
>  }
>
> -static int submodule_config(const char *var, const char *value, void *cb)
> +int submodule_config(const char *var, const char *value, void *cb)
>  {
>  	if (!prefixcmp(var, "submodule."))
>  		return parse_submodule_config_option(var, value);
> @@ -229,6 +229,70 @@ void show_submodule_summary(FILE *f, const char *path,
>  	strbuf_release(&sb);
>  }
>
> +int fetch_populated_submodules(int num_options, const char **options,
> +			       const char *prefix, int quiet)
> +{
> +	int i, result = 0, argc = 0;
> +	struct child_process cp;
> +	const char **argv;
> +	struct string_list_item *name_for_path;
> +	const char *work_tree = get_git_work_tree();
> +	if (!work_tree)
> +		return 0;
> +
> +	if (!the_index.initialized)
> +		if (read_cache() < 0)
> +			die("index file corrupt");
> +
> +	argv = xcalloc(num_options + 5, sizeof(const char *));

Where is this '5' coming from?  "fetch" "--submodule-prefix", the prefix,
and the terminating NULL?  What did I miss?

Re: [PATCH v4 1/3] fetch/pull: Add the --recurse-submodules option

[Jens Lehmann]

Am 09.12.2010 22:16, schrieb Junio C Hamano:
> Jens Lehmann <Jens.Lehmann@web.de> writes:
> 
>> Until now you had to call "git submodule update" (without -N|--no-fetch
>> option) or something like "git submodule foreach git fetch" to fetch
>> new commits in populated submodules from their remote.
>> ...
>> diff --git a/builtin/fetch.c b/builtin/fetch.c
>> index d35f000..db3fba3 100644
>> --- a/builtin/fetch.c
>> +++ b/builtin/fetch.c
>> ...
>> @@ -784,28 +790,36 @@ static int add_remote_or_group(const char *name, struct string_list *list)
>>  	return 1;
>>  }
>>
>> -static int fetch_multiple(struct string_list *list)
>> +static void add_options_to_argv(int *argc, const char **argv)
>>  {
>> -	int i, result = 0;
>> -	const char *argv[11] = { "fetch", "--append" };
>> -...
>> +static int fetch_multiple(struct string_list *list)
>> +{
>> +	int i, result = 0;
>> +	const char *argv[12] = { "fetch", "--append" };
> 
> This used to be 11; are we adding something new?  Ahh, possibly
> "--recurse_submodules".

Yup, that's why! (see a few lines down between "--keep" and "-v").

>> diff --git a/submodule.c b/submodule.c
>> index 91a4758..4d9b774 100644
>> --- a/submodule.c
>> +++ b/submodule.c
>> @@ -63,7 +63,7 @@ void set_diffopt_flags_from_submodule_config(struct diff_options *diffopt,
>>  	}
>>  }
>>
>> -static int submodule_config(const char *var, const char *value, void *cb)
>> +int submodule_config(const char *var, const char *value, void *cb)
>>  {
>>  	if (!prefixcmp(var, "submodule."))
>>  		return parse_submodule_config_option(var, value);
>> @@ -229,6 +229,70 @@ void show_submodule_summary(FILE *f, const char *path,
>>  	strbuf_release(&sb);
>>  }
>>
>> +int fetch_populated_submodules(int num_options, const char **options,
>> +			       const char *prefix, int quiet)
>> +{
>> +	int i, result = 0, argc = 0;
>> +	struct child_process cp;
>> +	const char **argv;
>> +	struct string_list_item *name_for_path;
>> +	const char *work_tree = get_git_work_tree();
>> +	if (!work_tree)
>> +		return 0;
>> +
>> +	if (!the_index.initialized)
>> +		if (read_cache() < 0)
>> +			die("index file corrupt");
>> +
>> +	argv = xcalloc(num_options + 5, sizeof(const char *));
> 
> Where is this '5' coming from?  "fetch" "--submodule-prefix", the prefix,
> and the terminating NULL?  What did I miss?

No, you didn't miss anything but I have been off by one ... '4' is
sufficient here.

Re: [PATCH v4 1/3] fetch/pull: Add the --recurse-submodules option

[Junio C Hamano]

Jens Lehmann <Jens.Lehmann@web.de> writes:

> Am 09.12.2010 22:16, schrieb Junio C Hamano:
> ...
>>> +int fetch_populated_submodules(int num_options, const char **options,
>>> +			       const char *prefix, int quiet)
>>> +{
>>> +	int i, result = 0, argc = 0;
>>> +	struct child_process cp;
>>> +	const char **argv;
>>> +	struct string_list_item *name_for_path;
>>> +	const char *work_tree = get_git_work_tree();
>>> +	if (!work_tree)
>>> +		return 0;
>>> +
>>> +	if (!the_index.initialized)
>>> +		if (read_cache() < 0)
>>> +			die("index file corrupt");
>>> +
>>> +	argv = xcalloc(num_options + 5, sizeof(const char *));
>> 
>> Where is this '5' coming from?  "fetch" "--submodule-prefix", the prefix,
>> and the terminating NULL?  What did I miss?
>
> No, you didn't miss anything but I have been off by one ... '4' is
> sufficient here.

Ok, thanks for double checking.

-- >8 --
Subject: [PATCH] fetch_populated_submodules(): document dynamic allocation size

... while fixing a miscounting.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 submodule.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/submodule.c b/submodule.c
index 4e62900..6f1c107 100644
--- a/submodule.c
+++ b/submodule.c
@@ -264,7 +264,8 @@ int fetch_populated_submodules(int num_options, const char **options,
 		if (read_cache() < 0)
 			die("index file corrupt");
 
-	argv = xcalloc(num_options + 5, sizeof(const char *));
+	/* 4: "fetch" (options) "--submodule-prefix" prefix NULL */
+	argv = xcalloc(num_options + 4, sizeof(const char *));
 	argv[argc++] = "fetch";
 	for (i = 0; i < num_options; i++)
 		argv[argc++] = options[i];

Re: [PATCH v4 1/3] fetch/pull: Add the --recurse-submodules option

[Jens Lehmann]

Am 10.12.2010 18:30, schrieb Junio C Hamano:
> Jens Lehmann <Jens.Lehmann@web.de> writes:
>> No, you didn't miss anything but I have been off by one ... '4' is
>> sufficient here.
> 
> Ok, thanks for double checking.
> 
> -- >8 --
> Subject: [PATCH] fetch_populated_submodules(): document dynamic allocation size

Thanks and ack.

(This was a leftover from the time the "--recurse-submodules" option had been
added in this function before I moved that into add_options_to_argv())