[RFC PATCH 0/2] Teach fetch and pull to recursively fetch submodules too

[RFC PATCH 0/2] Teach fetch and pull to recursively fetch submodules too

[Jens Lehmann]

Before we can take advantage of a recursive checkout for
submodules, we have to make sure that new commits are
present in the submodules. And even now after a fetch in the
superproject one sees "(commits not present)" output from
"git diff --submodule" (and thus in "git gui" and "gitk")
when the superproject recorded new submodule commits that
are not yet fetched there. Currently the time they do get
fetched is when the user does a "git submodule update". But
that makes it really hard to see beforehand what changes in
the submodule you will get by running that command, you have
to do something like "git submodule foreach git fetch" to
achieve that.

So I extended the fetch command to fetch populated submodules
too. I also added a command line option to fetch and pull and
the second patch introduces a per submodule config option to
give users the chance to control that behavior.

And maybe we need a config option to customize that behavior
for all submodules or all repos too?

Opinions?


Jens Lehmann (2):
  fetch/pull: Recursively fetch populated submodules
  Submodules: Add the new "fetch" config option for fetch and pull

 Documentation/config.txt        |    6 ++
 Documentation/fetch-options.txt |    6 ++
 Documentation/gitmodules.txt    |    8 +++
 builtin/fetch.c                 |   17 ++++++-
 git-pull.sh                     |   10 +++-
 submodule.c                     |   60 ++++++++++++++++++++++-
 submodule.h                     |    2 +
 t/t5526-fetch-submodules.sh     |  104 +++++++++++++++++++++++++++++++++++++++
 t/t7403-submodule-sync.sh       |    2 +-
 9 files changed, 210 insertions(+), 5 deletions(-)
 create mode 100755 t/t5526-fetch-submodules.sh

-- 
1.7.2.2.527.gdf3084

[PATCH 1/2] fetch/pull: Recursively fetch populated submodules

[Jens Lehmann]

Until now you had to call "git submodule update" (without the using the
-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" (and in "git gui" and "gitk") after fetching/pulling new
commits in the superproject and is an obstacle for implementing recursive
checkout of submodules.

This patch recursively fetches 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. This new behavior can be
disabled by using the new --no-recursive option.

t7403 had to be changed to use the --no-recursive option for pull.

Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de>
---
 Documentation/fetch-options.txt |    5 +++
 builtin/fetch.c                 |   11 ++++++-
 git-pull.sh                     |   10 +++++-
 submodule.c                     |   43 +++++++++++++++++++++++++-
 submodule.h                     |    2 +
 t/t5526-fetch-submodules.sh     |   64 +++++++++++++++++++++++++++++++++++++++
 t/t7403-submodule-sync.sh       |    2 +-
 7 files changed, 132 insertions(+), 5 deletions(-)
 create mode 100755 t/t5526-fetch-submodules.sh

diff --git a/Documentation/fetch-options.txt b/Documentation/fetch-options.txt
index 470ac31..1d875be 100644
--- a/Documentation/fetch-options.txt
+++ b/Documentation/fetch-options.txt
@@ -64,6 +64,11 @@ endif::git-pull[]
 	specified with the remote.<name>.tagopt setting. See
 	linkgit:git-config[1].

+--[no-]recursive::
+	By default new commits of all populated submodules will be fetched
+	too. This option can be used to disable/enable recursive fetching of
+	submodules.
+
 -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 fab3fce..da5fc9a 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>...]]",
@@ -27,7 +28,7 @@ enum {
 	TAGS_SET = 2
 };

-static int all, append, dry_run, force, keep, multiple, prune, update_head_ok, verbosity;
+static int all, append, dry_run, force, keep, multiple, prune, recursive = -1, update_head_ok, verbosity;
 static int progress;
 static int tags = TAGS_DEFAULT;
 static const char *depth;
@@ -53,6 +54,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, "recursive", &recursive,
+		    "control recursive fetching of submodules"),
 	OPT_BOOLEAN(0, "dry-run", &dry_run,
 		    "dry run"),
 	OPT_BOOLEAN('k', "keep", &keep, "keep downloaded pack"),
@@ -921,6 +924,12 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)
 		}
 	}

+	if (!result && recursive) {
+		gitmodules_config();
+		git_config(submodule_config, NULL);
+		result = fetch_populated_submodules();
+	}
+
 	/* 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..4bc8a60 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= recursive=
 merge_args=
 curr_branch=$(git symbolic-ref -q HEAD)
 curr_branch_short="${curr_branch#refs/heads/}"
@@ -105,6 +105,12 @@ do
 	--no-r|--no-re|--no-reb|--no-reba|--no-rebas|--no-rebase)
 		rebase=false
 		;;
+	--recursive)
+		recursive=--recursive
+		;;
+	--no-recursive)
+		recursive=--no-recursive
+		;;
 	--d|--dr|--dry|--dry-|--dry-r|--dry-ru|--dry-run)
 		dry_run=--dry-run
 		;;
@@ -220,7 +226,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 $recursive --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..e4f2419 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,47 @@ void show_submodule_summary(FILE *f, const char *path,
 	strbuf_release(&sb);
 }

+int fetch_populated_submodules()
+{
+	int result = 0;
+	struct child_process cp;
+	const char *argv[] = {
+		"fetch",
+		NULL,
+	};
+	struct string_list_item *name_for_path;
+	const char *work_tree = get_git_work_tree();
+	if (!work_tree)
+		return 0;
+
+	memset(&cp, 0, sizeof(cp));
+	cp.argv = argv;
+	cp.env = local_repo_env;
+	cp.git_cmd = 1;
+	cp.no_stdin = 1;
+	cp.out = -1;
+
+	for_each_string_list_item(name_for_path, &config_name_for_path) {
+		struct strbuf submodule_path = STRBUF_INIT;
+		struct strbuf submodule_git_dir = STRBUF_INIT;
+		const char *git_dir;
+		strbuf_addf(&submodule_path, "%s/%s", work_tree, name_for_path->string);
+		strbuf_addf(&submodule_git_dir, "%s/.git", submodule_path.buf);
+		git_dir = read_gitfile_gently(submodule_git_dir.buf);
+		if (!git_dir)
+			git_dir = submodule_git_dir.buf;
+		if (is_directory(git_dir)) {
+			printf("Fetching submodule %s\n", name_for_path->string);
+			cp.dir = submodule_path.buf;
+			if (run_command(&cp))
+				result = 1;
+		}
+		strbuf_release(&submodule_path);
+		strbuf_release(&submodule_git_dir);
+	}
+	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..380878c 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,7 @@ 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();
 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..da5d5fd
--- /dev/null
+++ b/t/t5526-fetch-submodules.sh
@@ -0,0 +1,64 @@
+#!/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
+	)
+}
+
+test_expect_success setup '
+	mkdir submodule &&
+	(
+		cd submodule &&
+		git init &&
+		echo subcontent > subfile &&
+		git add subfile &&
+		git commit -m new subfile
+	) &&
+	git submodule add "$pwd/submodule" submodule &&
+	git commit -am initial &&
+	git clone . downstream &&
+	(
+		cd downstream &&
+		git submodule init &&
+		git submodule update
+	) &&
+	echo "Fetching submodule submodule" > expect.out
+'
+
+test_expect_success "fetch recurses into submodules" '
+	add_upstream_commit &&
+	(
+		cd downstream &&
+		git fetch >../actual.out 2>../actual.err
+	) &&
+	test_cmp expect.out actual.out &&
+	test_cmp expect.err actual.err
+'
+
+test_expect_success "fetch --no-recursive only fetches superproject" '
+	add_upstream_commit &&
+	(
+		cd downstream &&
+		git fetch --no-recursive >../actual.out 2>../actual.err
+	) &&
+	! test -s actual.out &&
+	! test -s actual.err
+'
+
+test_done
diff --git a/t/t7403-submodule-sync.sh b/t/t7403-submodule-sync.sh
index 02522f9..6f3e966 100755
--- a/t/t7403-submodule-sync.sh
+++ b/t/t7403-submodule-sync.sh
@@ -50,7 +50,7 @@ test_expect_success 'change submodule url' '

 test_expect_success '"git submodule sync" should update submodule URLs' '
 	(cd super-clone &&
-	 git pull &&
+	 git pull --no-recursive &&
 	 git submodule sync
 	) &&
 	test -d "$(git config -f super-clone/submodule/.git/config \
-- 
1.7.2.2.527.gdf3084

[PATCH 2/2] Submodules: Add the new "fetch" config option

[Jens Lehmann]

The new boolean "fetch" config option controls the default 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.

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

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

Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de>
---
 Documentation/config.txt        |    6 +++++
 Documentation/fetch-options.txt |    3 +-
 Documentation/gitmodules.txt    |    8 +++++++
 builtin/fetch.c                 |   14 +++++++++---
 submodule.c                     |   19 +++++++++++++++++-
 submodule.h                     |    2 +-
 t/t5526-fetch-submodules.sh     |   40 +++++++++++++++++++++++++++++++++++++++
 7 files changed, 85 insertions(+), 7 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 0510ac7..048465f 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1769,6 +1769,12 @@ 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>.fetch::
+	A boolean to enable/disable recursive fetching of this submodule. It can
+	be overriden by using the --[no-]recursive command line option to "git
+	fetch" and "git pull".	This setting overrides any setting made in
+	.gitmodules for this submodule.
+
 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 1d875be..aff4daf 100644
--- a/Documentation/fetch-options.txt
+++ b/Documentation/fetch-options.txt
@@ -67,7 +67,8 @@ endif::git-pull[]
 --[no-]recursive::
 	By default new commits of all populated submodules will be fetched
 	too. This option can be used to disable/enable recursive fetching of
-	submodules.
+	submodules regardless of the 'fetch' configuration setting (see
+	linkgit:git-config[1] or linkgit:gitmodules[5]).

 -u::
 --update-head-ok::
diff --git a/Documentation/gitmodules.txt b/Documentation/gitmodules.txt
index bcffd95..febfef4 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>.fetch::
+	A boolean 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-]recursive" 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/builtin/fetch.c b/builtin/fetch.c
index da5fc9a..17759b5 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -28,7 +28,13 @@ enum {
 	TAGS_SET = 2
 };

-static int all, append, dry_run, force, keep, multiple, prune, recursive = -1, update_head_ok, verbosity;
+enum {
+	RECURSIVE_UNSET = 0,
+	RECURSIVE_DEFAULT = 1,
+	RECURSIVE_SET = 2
+};
+
+static int all, append, dry_run, force, keep, multiple, prune, recursive = RECURSIVE_DEFAULT, update_head_ok, verbosity;
 static int progress;
 static int tags = TAGS_DEFAULT;
 static const char *depth;
@@ -54,8 +60,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, "recursive", &recursive,
-		    "control recursive fetching of submodules"),
+	OPT_SET_INT(0, "recursive", &recursive,
+		    "control recursive fetching of submodules", RECURSIVE_SET),
 	OPT_BOOLEAN(0, "dry-run", &dry_run,
 		    "dry run"),
 	OPT_BOOLEAN('k', "keep", &keep, "keep downloaded pack"),
@@ -927,7 +933,7 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)
 	if (!result && recursive) {
 		gitmodules_config();
 		git_config(submodule_config, NULL);
-		result = fetch_populated_submodules();
+		result = fetch_populated_submodules(recursive == RECURSIVE_SET);
 	}

 	/* All names were strdup()ed or strndup()ed */
diff --git a/submodule.c b/submodule.c
index e4f2419..8c98fad 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_for_name;
 struct string_list config_ignore_for_name;

 static int add_submodule_odb(const char *path)
@@ -100,6 +101,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 > 5) && !strcmp(var + len - 6, ".fetch")) {
+		strbuf_add(&submodname, var, len - 6);
+		config = unsorted_string_list_lookup(&config_fetch_for_name, submodname.buf);
+		if (!config)
+			config = string_list_append(&config_fetch_for_name,
+						    strbuf_detach(&submodname, NULL));
+		config->util = (void *)git_config_bool(var, value);
+		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")) {
@@ -229,7 +238,7 @@ void show_submodule_summary(FILE *f, const char *path,
 	strbuf_release(&sb);
 }

-int fetch_populated_submodules()
+int fetch_populated_submodules(int forced)
 {
 	int result = 0;
 	struct child_process cp;
@@ -253,6 +262,14 @@ int fetch_populated_submodules()
 		struct strbuf submodule_path = STRBUF_INIT;
 		struct strbuf submodule_git_dir = STRBUF_INIT;
 		const char *git_dir;
+
+		if (!forced) {
+			struct string_list_item *fetch_option;
+			fetch_option = unsorted_string_list_lookup(&config_fetch_for_name, name_for_path->util);
+			if (fetch_option && !fetch_option->util)
+				continue;
+		}
+
 		strbuf_addf(&submodule_path, "%s/%s", work_tree, name_for_path->string);
 		strbuf_addf(&submodule_git_dir, "%s/.git", submodule_path.buf);
 		git_dir = read_gitfile_gently(submodule_git_dir.buf);
diff --git a/submodule.h b/submodule.h
index 380878c..9e6257e 100644
--- a/submodule.h
+++ b/submodule.h
@@ -13,7 +13,7 @@ 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 fetch_populated_submodules(int forced);
 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 da5d5fd..489ef1a 100755
--- a/t/t5526-fetch-submodules.sh
+++ b/t/t5526-fetch-submodules.sh
@@ -52,6 +52,46 @@ test_expect_success "fetch recurses into submodules" '
 '

 test_expect_success "fetch --no-recursive only fetches superproject" '
+	(
+		cd downstream &&
+		git fetch --no-recursive >../actual.out 2>../actual.err
+	) &&
+	! test -s actual.out &&
+	! test -s actual.err
+'
+
+test_expect_success "using fetch=false in .gitmodules only fetches superproject" '
+	(
+		cd downstream &&
+		git config -f .gitmodules submodule.submodule.fetch false &&
+		git fetch >../actual.out 2>../actual.err
+	) &&
+	! test -s actual.out &&
+	! test -s actual.err
+'
+
+test_expect_success "--recursive overrides .gitmodules config" '
+	add_upstream_commit &&
+	(
+		cd downstream &&
+		git fetch --recursive >../actual.out 2>../actual.err
+	) &&
+	test_cmp expect.out actual.out &&
+	test_cmp expect.err actual.err
+'
+
+test_expect_success "using fetch=true in .git/config overrides setting in .gitmodules" '
+	add_upstream_commit &&
+	(
+		cd downstream &&
+		git config submodule.submodule.fetch true &&
+		git fetch >../actual.out 2>../actual.err
+	) &&
+	test_cmp expect.out actual.out &&
+	test_cmp expect.err actual.err
+'
+
+test_expect_success "--no-recursive overrides fetch setting from .git/config" '
 	add_upstream_commit &&
 	(
 		cd downstream &&
-- 
1.7.2.2.527.gdf3084

Re: [PATCH 2/2] Submodules: Add the new "fetch" config option

[Junio C Hamano]

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

> diff --git a/submodule.c b/submodule.c
> index e4f2419..8c98fad 100644
> --- a/submodule.c
> +++ b/submodule.c
> @@ -100,6 +101,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 > 5) && !strcmp(var + len - 6, ".fetch")) {
> +		strbuf_add(&submodname, var, len - 6);
> +		config = unsorted_string_list_lookup(&config_fetch_for_name, submodname.buf);
> +		if (!config)
> +			config = string_list_append(&config_fetch_for_name,
> +						    strbuf_detach(&submodname, NULL));
> +		config->util = (void *)git_config_bool(var, value);

Hmm?  We get this here...

    submodule.c:110: error: cast to pointer from integer of different size

[PATCH 2/2 v2] Submodules: Add the new "fetch" config option for fetch and pull

[Jens Lehmann]

The new boolean "fetch" config option controls the default 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.

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

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

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

Am 30.08.2010 09:34, schrieb Junio C Hamano:
> Hmm?  We get this here...
>
>     submodule.c:110: error: cast to pointer from integer of different size

Grmpf, sorry for messing up 64bit builds. This version of the patch
compiles cleanly for me under 32 and 64 bits.


 Documentation/config.txt        |    6 +++++
 Documentation/fetch-options.txt |    3 +-
 Documentation/gitmodules.txt    |    8 +++++++
 builtin/fetch.c                 |   14 +++++++++---
 submodule.c                     |   19 +++++++++++++++++-
 submodule.h                     |    2 +-
 t/t5526-fetch-submodules.sh     |   40 +++++++++++++++++++++++++++++++++++++++
 7 files changed, 85 insertions(+), 7 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 0510ac7..048465f 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1769,6 +1769,12 @@ 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>.fetch::
+	A boolean to enable/disable recursive fetching of this submodule. It can
+	be overriden by using the --[no-]recursive command line option to "git
+	fetch" and "git pull".	This setting overrides any setting made in
+	.gitmodules for this submodule.
+
 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 1d875be..aff4daf 100644
--- a/Documentation/fetch-options.txt
+++ b/Documentation/fetch-options.txt
@@ -67,7 +67,8 @@ endif::git-pull[]
 --[no-]recursive::
 	By default new commits of all populated submodules will be fetched
 	too. This option can be used to disable/enable recursive fetching of
-	submodules.
+	submodules regardless of the 'fetch' configuration setting (see
+	linkgit:git-config[1] or linkgit:gitmodules[5]).

 -u::
 --update-head-ok::
diff --git a/Documentation/gitmodules.txt b/Documentation/gitmodules.txt
index bcffd95..febfef4 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>.fetch::
+	A boolean 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-]recursive" 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/builtin/fetch.c b/builtin/fetch.c
index da5fc9a..17759b5 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -28,7 +28,13 @@ enum {
 	TAGS_SET = 2
 };

-static int all, append, dry_run, force, keep, multiple, prune, recursive = -1, update_head_ok, verbosity;
+enum {
+	RECURSIVE_UNSET = 0,
+	RECURSIVE_DEFAULT = 1,
+	RECURSIVE_SET = 2
+};
+
+static int all, append, dry_run, force, keep, multiple, prune, recursive = RECURSIVE_DEFAULT, update_head_ok, verbosity;
 static int progress;
 static int tags = TAGS_DEFAULT;
 static const char *depth;
@@ -54,8 +60,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, "recursive", &recursive,
-		    "control recursive fetching of submodules"),
+	OPT_SET_INT(0, "recursive", &recursive,
+		    "control recursive fetching of submodules", RECURSIVE_SET),
 	OPT_BOOLEAN(0, "dry-run", &dry_run,
 		    "dry run"),
 	OPT_BOOLEAN('k', "keep", &keep, "keep downloaded pack"),
@@ -927,7 +933,7 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)
 	if (!result && recursive) {
 		gitmodules_config();
 		git_config(submodule_config, NULL);
-		result = fetch_populated_submodules();
+		result = fetch_populated_submodules(recursive == RECURSIVE_SET);
 	}

 	/* All names were strdup()ed or strndup()ed */
diff --git a/submodule.c b/submodule.c
index e4f2419..2380638 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_for_name;
 struct string_list config_ignore_for_name;

 static int add_submodule_odb(const char *path)
@@ -100,6 +101,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 > 5) && !strcmp(var + len - 6, ".fetch")) {
+		strbuf_add(&submodname, var, len - 6);
+		config = unsorted_string_list_lookup(&config_fetch_for_name, submodname.buf);
+		if (!config)
+			config = string_list_append(&config_fetch_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")) {
@@ -229,7 +238,7 @@ void show_submodule_summary(FILE *f, const char *path,
 	strbuf_release(&sb);
 }

-int fetch_populated_submodules()
+int fetch_populated_submodules(int forced)
 {
 	int result = 0;
 	struct child_process cp;
@@ -253,6 +262,14 @@ int fetch_populated_submodules()
 		struct strbuf submodule_path = STRBUF_INIT;
 		struct strbuf submodule_git_dir = STRBUF_INIT;
 		const char *git_dir;
+
+		if (!forced) {
+			struct string_list_item *fetch_option;
+			fetch_option = unsorted_string_list_lookup(&config_fetch_for_name, name_for_path->util);
+			if (fetch_option && !fetch_option->util)
+				continue;
+		}
+
 		strbuf_addf(&submodule_path, "%s/%s", work_tree, name_for_path->string);
 		strbuf_addf(&submodule_git_dir, "%s/.git", submodule_path.buf);
 		git_dir = read_gitfile_gently(submodule_git_dir.buf);
diff --git a/submodule.h b/submodule.h
index 380878c..9e6257e 100644
--- a/submodule.h
+++ b/submodule.h
@@ -13,7 +13,7 @@ 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 fetch_populated_submodules(int forced);
 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 da5d5fd..489ef1a 100755
--- a/t/t5526-fetch-submodules.sh
+++ b/t/t5526-fetch-submodules.sh
@@ -52,6 +52,46 @@ test_expect_success "fetch recurses into submodules" '
 '

 test_expect_success "fetch --no-recursive only fetches superproject" '
+	(
+		cd downstream &&
+		git fetch --no-recursive >../actual.out 2>../actual.err
+	) &&
+	! test -s actual.out &&
+	! test -s actual.err
+'
+
+test_expect_success "using fetch=false in .gitmodules only fetches superproject" '
+	(
+		cd downstream &&
+		git config -f .gitmodules submodule.submodule.fetch false &&
+		git fetch >../actual.out 2>../actual.err
+	) &&
+	! test -s actual.out &&
+	! test -s actual.err
+'
+
+test_expect_success "--recursive overrides .gitmodules config" '
+	add_upstream_commit &&
+	(
+		cd downstream &&
+		git fetch --recursive >../actual.out 2>../actual.err
+	) &&
+	test_cmp expect.out actual.out &&
+	test_cmp expect.err actual.err
+'
+
+test_expect_success "using fetch=true in .git/config overrides setting in .gitmodules" '
+	add_upstream_commit &&
+	(
+		cd downstream &&
+		git config submodule.submodule.fetch true &&
+		git fetch >../actual.out 2>../actual.err
+	) &&
+	test_cmp expect.out actual.out &&
+	test_cmp expect.err actual.err
+'
+
+test_expect_success "--no-recursive overrides fetch setting from .git/config" '
 	add_upstream_commit &&
 	(
 		cd downstream &&
-- 
1.7.2.2.527.gacca9

Re: [RFC PATCH 0/2] Teach fetch and pull to recursively fetch submodules too

[Ævar Arnfjörð Bjarmason]

On Sun, Aug 29, 2010 at 15:49, Jens Lehmann <Jens.Lehmann@web.de> wrote:

> Opinions?

I'm not familiar with this area, but I couldn't see anything wrong
with it. However I wonder if we're going to make --recursive the
default for pull/fetch whether it shouldn't be made the default for
clone while we're at it.

Re: [RFC PATCH 0/2] Teach fetch and pull to recursively fetch submodules too

[Jens Lehmann]

Am 29.08.2010 19:29, schrieb Ævar Arnfjörð Bjarmason:
> However I wonder if we're going to make --recursive the
> default for pull/fetch whether it shouldn't be made the default for
> clone while we're at it.

Hm, while that would work for me (and for you obviously ;-), it
wouldn't make sense for other use cases where submodules are used
as a kind of sparse checkout (and then some of them are never meant
to be checked out, at least by some users).

I don't think recursive fetch would hurt there, as that only
affects already populated submodules, so turning that on by
default sounds safe.

After finishing the recursive checkout of populated submodules
one of my next goals is to teach clone to recurse too, but only
for those submodules where this is wanted (aka configured via
.gitmodules).

Re: [RFC PATCH 0/2] Teach fetch and pull to recursively fetch submodules too

[Junio C Hamano]

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

> So I extended the fetch command to fetch populated submodules too. I
> also added a command line option to fetch and pull and the second patch
> introduces a per submodule config option to give users the chance to
> control that behavior.
>
> And maybe we need a config option to customize that behavior
> for all submodules or all repos too?
>
> Opinions?

I think your "if a particular submodule is already initialized, recursing
into it is more likely than not what the user wants" is a sound heuristic.

I am not sure what you mean by "all submodules" nor "all repos", though.
In order to trigger the feature this series introduces, users have a means
to initialize all submodules in one go (update --recursive --init), and
wouldn't that be sufficient?

Re: [RFC PATCH 0/2] Teach fetch and pull to recursively fetch submodules too

[Jens Lehmann]

Am 30.08.2010 07:58, schrieb Junio C Hamano:
> I am not sure what you mean by "all submodules" nor "all repos", though.
> In order to trigger the feature this series introduces, users have a means
> to initialize all submodules in one go (update --recursive --init), and
> wouldn't that be sufficient?

I was thinking about adding a "fetch.recursive" config option which
would allow setting the - now hardcoded - default in .git/config or
~/.gitconfig, but I'm not quite sure that is needed.

Re: [RFC PATCH 0/2] Teach fetch and pull to recursively fetch submodules too

[Kevin Ballard]

On Aug 29, 2010, at 10:58 PM, Junio C Hamano wrote:

> Jens Lehmann <Jens.Lehmann@web.de> writes:
> 
>> So I extended the fetch command to fetch populated submodules too. I
>> also added a command line option to fetch and pull and the second patch
>> introduces a per submodule config option to give users the chance to
>> control that behavior.
>> 
>> And maybe we need a config option to customize that behavior
>> for all submodules or all repos too?
>> 
>> Opinions?
> 
> I think your "if a particular submodule is already initialized, recursing
> into it is more likely than not what the user wants" is a sound heuristic.

This was just merged onto the next branch a few days ago, so I finally got a chance to test it out, and it's not pretty. The project I'm working in has a bunch of submodules, and this makes a simple `git fetch` take about 20 seconds. Even worse, it makes `git remote update` take significantly longer, as it apparently fetches all submodules twice. As a comparison, with all the submodule fetching turned off, a normal `git fetch` takes about 1.5 seconds. Oddly enough, it doesn't seem to be fetching submodules recursively - one of my submodules has a whole tree of about 6 different submodules, but those embedded submodules aren't being fetched.

There seems to be 3 issues here. The first is `git remote update`, which I am fond of using, causes buggy behavior where it fetches all submodules twice. The second is this submodule fetch doesn't appear to be recursive. The third issue is `git fetch` doesn't have any business fetching submodules when the submodule reference was never changed as part of the fetch, especially if the main fetch itself didn't even find any changes. It seems to me that the correct behavior would be to look at all the fetched commits to see if any of them change the submodule reference, and only in that case should it automatically fetch any submodules whose references were modified. The stated desire of avoiding "(commits not present)" when doing a diff will still be met, but it will avoid slowing down the nor
 mal case of a `git fetch`.

It also seems like there ought to be a config variable one can set for the default behavior if submodule.<name>.fetch is not present in .gitmodules or in .git/config.

-Kevin Ballard

Re: [RFC PATCH 0/2] Teach fetch and pull to recursively fetch submodules too

[Kevin Ballard]

To those CC'd on the original message, sorry for the repeat send to the list but the original was rejected due to not being text/plain.

On Sep 14, 2010, at 5:18 PM, Kevin Ballard wrote:

> On Aug 29, 2010, at 10:58 PM, Junio C Hamano wrote:
> 
>> Jens Lehmann <Jens.Lehmann@web.de> writes:
>> 
>>> So I extended the fetch command to fetch populated submodules too. I
>>> also added a command line option to fetch and pull and the second patch
>>> introduces a per submodule config option to give users the chance to
>>> control that behavior.
>>> 
>>> And maybe we need a config option to customize that behavior
>>> for all submodules or all repos too?
>>> 
>>> Opinions?
>> 
>> I think your "if a particular submodule is already initialized, recursing
>> into it is more likely than not what the user wants" is a sound heuristic.
> 
> This was just merged onto the next branch a few days ago, so I finally got a chance to test it out, and it's not pretty. The project I'm working in has a bunch of submodules, and this makes a simple `git fetch` take about 20 seconds. Even worse, it makes `git remote update` take significantly longer, as it apparently fetches all submodules twice. As a comparison, with all the submodule fetching turned off, a normal `git fetch` takes about 1.5 seconds. Oddly enough, it doesn't seem to be fetching submodules recursively - one of my submodules has a whole tree of about 6 different submodules, but those embedded submodules aren't being fetched.
> 
> There seems to be 3 issues here. The first is `git remote update`, which I am fond of using, causes buggy behavior where it fetches all submodules twice. The second is this submodule fetch doesn't appear to be recursive. The third issue is `git fetch` doesn't have any business fetching submodules when the submodule reference was never changed as part of the fetch, especially if the main fetch itself didn't even find any changes. It seems to me that the correct behavior would be to look at all the fetched commits to see if any of them change the submodule reference, and only in that case should it automatically fetch any submodules whose references were modified. The stated desire of avoiding "(commits not present)" when doing a diff will still be met, but it will avoid slowing down the n
 ormal case of a `git fetch`.
> 
> It also seems like there ought to be a config variable one can set for the default behavior if submodule.<name>.fetch is not present in .gitmodules or in .git/config.

Turns out the recursive fetch behavior is a bit more subtle than I thought. It actually recurses one level deep into submodules, but no further. I have a submodule foo, which has a submodule bar, which has a submodule baz, and when I run `git fetch` I get

Fetching submodule foo
Fetching submodule bar

There's two problems here. The first is it doesn't tell me that bar is actually foo/bar, and the second is it never fetches baz.

-Kevin Ballard

[PATCH] fetch: Get submodule paths from index and not from .gitmodules

[Jens Lehmann]

In the first version the .gitmodules file was parsed and all submodules
found there were recursively fetched. This lead to problems when the
.gitmodules file was not properly set up. "git submodule update" gets
this information from the index via "git ls-files", lets do the same here.

Reported-by: Kevin Ballard <kevin@sb.org>
Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de>
---

Could it be that the information in your .gitmodules files is not
quite right? Then this patch should fix your problems with the
recursion. Please test it and let me know if the submodules are
now recursively fetched as they should.


 submodule.c |   25 +++++++++++++++++++------
 1 files changed, 19 insertions(+), 6 deletions(-)

diff --git a/submodule.c b/submodule.c
index 05661e2..13a694b 100644
--- a/submodule.c
+++ b/submodule.c
@@ -240,7 +240,7 @@ void show_submodule_summary(FILE *f, const char *path,

 int fetch_populated_submodules(int forced)
 {
-	int result = 0;
+	int i, result = 0;
 	struct child_process cp;
 	const char *argv[] = {
 		"fetch",
@@ -251,6 +251,10 @@ int fetch_populated_submodules(int forced)
 	if (!work_tree)
 		return 0;

+	if (!the_index.initialized)
+		if (read_cache() < 0)
+			die("index file corrupt");
+
 	memset(&cp, 0, sizeof(cp));
 	cp.argv = argv;
 	cp.env = local_repo_env;
@@ -258,25 +262,34 @@ int fetch_populated_submodules(int forced)
 	cp.no_stdin = 1;
 	cp.out = -1;

-	for_each_string_list_item(name_for_path, &config_name_for_path) {
+	for (i = 0; i < active_nr; i++) {
 		struct strbuf submodule_path = STRBUF_INIT;
 		struct strbuf submodule_git_dir = STRBUF_INIT;
-		const char *git_dir;
+		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;

 		if (!forced) {
 			struct string_list_item *fetch_option;
-			fetch_option = unsorted_string_list_lookup(&config_fetch_for_name, name_for_path->util);
+			fetch_option = unsorted_string_list_lookup(&config_fetch_for_name, name);
 			if (fetch_option && !fetch_option->util)
 				continue;
 		}

-		strbuf_addf(&submodule_path, "%s/%s", work_tree, name_for_path->string);
+		strbuf_addf(&submodule_path, "%s/%s", work_tree, ce->name);
 		strbuf_addf(&submodule_git_dir, "%s/.git", submodule_path.buf);
 		git_dir = read_gitfile_gently(submodule_git_dir.buf);
 		if (!git_dir)
 			git_dir = submodule_git_dir.buf;
 		if (is_directory(git_dir)) {
-			printf("Fetching submodule %s\n", name_for_path->string);
+			printf("Fetching submodule %s\n", ce->name);
 			cp.dir = submodule_path.buf;
 			if (run_command(&cp))
 				result = 1;
-- 
1.7.3.rc2.232.g3328

Re: [PATCH] fetch: Get submodule paths from index and not from .gitmodules

[Kevin Ballard]

Unfortunately, the only effect this had was to change the order of fetches (it now appears to be case-sensitive alphabetical). After applying this patch, this is what I saw (using the same terms as my previous email):

Fetching submodule G
Fetching submodule B
Fetching submodule F
Fetching submodule C
Fetching submodule H
Fetching submodule E
Fetching submodule A
Fetching submodule D

Note that it's still telling me that it's fetching submodule H even though this inside of submodule C rather than at the root level. I also verified this by running `git ls-tree -r HEAD | grep commit` and observed that only submodules A-G show up in that list.

-Kevin Ballard

On Sep 16, 2010, at 6:55 AM, Jens Lehmann wrote:

> In the first version the .gitmodules file was parsed and all submodules
> found there were recursively fetched. This lead to problems when the
> .gitmodules file was not properly set up. "git submodule update" gets
> this information from the index via "git ls-files", lets do the same here.
> 
> Reported-by: Kevin Ballard <kevin@sb.org>
> Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de>
> ---
> 
> Could it be that the information in your .gitmodules files is not
> quite right? Then this patch should fix your problems with the
> recursion. Please test it and let me know if the submodules are
> now recursively fetched as they should.
> 
> 
> submodule.c |   25 +++++++++++++++++++------
> 1 files changed, 19 insertions(+), 6 deletions(-)
> 
> diff --git a/submodule.c b/submodule.c
> index 05661e2..13a694b 100644
> --- a/submodule.c
> +++ b/submodule.c
> @@ -240,7 +240,7 @@ void show_submodule_summary(FILE *f, const char *path,
> 
> int fetch_populated_submodules(int forced)
> {
> -	int result = 0;
> +	int i, result = 0;
> 	struct child_process cp;
> 	const char *argv[] = {
> 		"fetch",
> @@ -251,6 +251,10 @@ int fetch_populated_submodules(int forced)
> 	if (!work_tree)
> 		return 0;
> 
> +	if (!the_index.initialized)
> +		if (read_cache() < 0)
> +			die("index file corrupt");
> +
> 	memset(&cp, 0, sizeof(cp));
> 	cp.argv = argv;
> 	cp.env = local_repo_env;
> @@ -258,25 +262,34 @@ int fetch_populated_submodules(int forced)
> 	cp.no_stdin = 1;
> 	cp.out = -1;
> 
> -	for_each_string_list_item(name_for_path, &config_name_for_path) {
> +	for (i = 0; i < active_nr; i++) {
> 		struct strbuf submodule_path = STRBUF_INIT;
> 		struct strbuf submodule_git_dir = STRBUF_INIT;
> -		const char *git_dir;
> +		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;
> 
> 		if (!forced) {
> 			struct string_list_item *fetch_option;
> -			fetch_option = unsorted_string_list_lookup(&config_fetch_for_name, name_for_path->util);
> +			fetch_option = unsorted_string_list_lookup(&config_fetch_for_name, name);
> 			if (fetch_option && !fetch_option->util)
> 				continue;
> 		}
> 
> -		strbuf_addf(&submodule_path, "%s/%s", work_tree, name_for_path->string);
> +		strbuf_addf(&submodule_path, "%s/%s", work_tree, ce->name);
> 		strbuf_addf(&submodule_git_dir, "%s/.git", submodule_path.buf);
> 		git_dir = read_gitfile_gently(submodule_git_dir.buf);
> 		if (!git_dir)
> 			git_dir = submodule_git_dir.buf;
> 		if (is_directory(git_dir)) {
> -			printf("Fetching submodule %s\n", name_for_path->string);
> +			printf("Fetching submodule %s\n", ce->name);
> 			cp.dir = submodule_path.buf;
> 			if (run_command(&cp))
> 				result = 1;
> -- 
> 1.7.3.rc2.232.g3328

Re: [PATCH] fetch: Get submodule paths from index and not from .gitmodules

[Jens Lehmann]

Am 16.09.2010 21:29, schrieb Kevin Ballard:
> Unfortunately, the only effect this had was to change the order of fetches
> (it now appears to be case-sensitive alphabetical).

Yup, this is a side effect of using the index instead of the .gitmodules
file.

But I think I found the real issue, the stdout of the forked "git fetch"
was swallowed due to a copy & paste bug while the actual fetch commands
were executed nonetheless. Please try the following change:


diff --git a/submodule.c b/submodule.c
index e2c3bae..4fb1071 100644
--- a/submodule.c
+++ b/submodule.c
@@ -260,7 +260,8 @@ int fetch_populated_submodules(int forced)
        cp.env = local_repo_env;
        cp.git_cmd = 1;
        cp.no_stdin = 1;
-       cp.out = -1;
+       cp.out = 1;
+       cp.err = 1;

        for (i = 0; i < active_nr; i++) {
                struct strbuf submodule_path = STRBUF_INIT;

Re: [PATCH] fetch: Get submodule paths from index and not from .gitmodules

[Johannes Sixt]

Am 9/17/2010 13:31, schrieb Jens Lehmann:
> But I think I found the real issue, the stdout of the forked "git fetch"
> was swallowed due to a copy & paste bug while the actual fetch commands
> were executed nonetheless. Please try the following change:
> 
> 
> diff --git a/submodule.c b/submodule.c
> index e2c3bae..4fb1071 100644
> --- a/submodule.c
> +++ b/submodule.c
> @@ -260,7 +260,8 @@ int fetch_populated_submodules(int forced)
>         cp.env = local_repo_env;
>         cp.git_cmd = 1;
>         cp.no_stdin = 1;
> -       cp.out = -1;
> +       cp.out = 1;
> +       cp.err = 1;

This cannot be correct. Subsequent code reads the stdout of the child
process, i.e., you want a pipe; hence, cp.out = -1 is correct (this
requests a pipe; later code correctly closes cp.out).

As far as stderr of the child is concerned, if you only want to re-use the
standard error of the parent, then not assigning anything to cp.err is
correct (it was set to 0 in the memset before this hunk). But perhaps you
want to achieve something else?

-- Hannes

Re: [PATCH] fetch: Get submodule paths from index and not from .gitmodules

[Jens Lehmann]

Am 17.09.2010 14:06, schrieb Johannes Sixt:
> Am 9/17/2010 13:31, schrieb Jens Lehmann:
>> But I think I found the real issue, the stdout of the forked "git fetch"
>> was swallowed due to a copy & paste bug while the actual fetch commands
>> were executed nonetheless. Please try the following change:
>>
>>
>> diff --git a/submodule.c b/submodule.c
>> index e2c3bae..4fb1071 100644
>> --- a/submodule.c
>> +++ b/submodule.c
>> @@ -260,7 +260,8 @@ int fetch_populated_submodules(int forced)
>>         cp.env = local_repo_env;
>>         cp.git_cmd = 1;
>>         cp.no_stdin = 1;
>> -       cp.out = -1;
>> +       cp.out = 1;
>> +       cp.err = 1;
> 
> This cannot be correct. Subsequent code reads the stdout of the child
> process, i.e., you want a pipe; hence, cp.out = -1 is correct (this
> requests a pipe; later code correctly closes cp.out).

Thanks for catching this! I copied this code from a spot where stdout
is read via a pipe (and is then closed afterwards), but that isn't the
case here.


> As far as stderr of the child is concerned, if you only want to re-use the
> standard error of the parent, then not assigning anything to cp.err is
> correct (it was set to 0 in the memset before this hunk). But perhaps you
> want to achieve something else?

Nope. You are right, setting both to 0 (via the memset) to inherit the
channel from the parent is just what is needed here.

So the correct fix should look like this:


diff --git a/submodule.c b/submodule.c
index e2c3bae..209efa4 100644
--- a/submodule.c
+++ b/submodule.c
@@ -260,7 +260,6 @@ int fetch_populated_submodules(int forced)
        cp.env = local_repo_env;
        cp.git_cmd = 1;
        cp.no_stdin = 1;
-       cp.out = -1;

        for (i = 0; i < active_nr; i++) {
                struct strbuf submodule_path = STRBUF_INIT;

Re: [PATCH] fetch: Get submodule paths from index and not from .gitmodules

[Johannes Sixt]

Am 9/17/2010 14:22, schrieb Jens Lehmann:
> Am 17.09.2010 14:06, schrieb Johannes Sixt:
>> Am 9/17/2010 13:31, schrieb Jens Lehmann:
>>> But I think I found the real issue, the stdout of the forked "git fetch"
>>> was swallowed due to a copy & paste bug while the actual fetch commands
>>> were executed nonetheless. Please try the following change:
>>>
>>>
>>> diff --git a/submodule.c b/submodule.c
>>> index e2c3bae..4fb1071 100644
>>> --- a/submodule.c
>>> +++ b/submodule.c
>>> @@ -260,7 +260,8 @@ int fetch_populated_submodules(int forced)
>>>         cp.env = local_repo_env;
>>>         cp.git_cmd = 1;
>>>         cp.no_stdin = 1;
>>> -       cp.out = -1;
>>> +       cp.out = 1;
>>> +       cp.err = 1;
>>
>> This cannot be correct. Subsequent code reads the stdout of the child
>> process, i.e., you want a pipe; hence, cp.out = -1 is correct (this
>> requests a pipe; later code correctly closes cp.out).
> 
> Thanks for catching this! I copied this code from a spot where stdout
> is read via a pipe (and is then closed afterwards), but that isn't the
> case here.
> 
> 
>> As far as stderr of the child is concerned, if you only want to re-use the
>> standard error of the parent, then not assigning anything to cp.err is
>> correct (it was set to 0 in the memset before this hunk). But perhaps you
>> want to achieve something else?
> 
> Nope. You are right, setting both to 0 (via the memset) to inherit the
> channel from the parent is just what is needed here.
> 
> So the correct fix should look like this:
> 
> 
> diff --git a/submodule.c b/submodule.c
> index e2c3bae..209efa4 100644
> --- a/submodule.c
> +++ b/submodule.c
> @@ -260,7 +260,6 @@ int fetch_populated_submodules(int forced)
>         cp.env = local_repo_env;
>         cp.git_cmd = 1;
>         cp.no_stdin = 1;
> -       cp.out = -1;
> 
>         for (i = 0; i < active_nr; i++) {
>                 struct strbuf submodule_path = STRBUF_INIT;

That cannot be correct, either. Look further down. There you have:

	len = strbuf_read(&buf, cp.out, 1024);

and later

	close(cp.out);

You can do neither when you do not request a pipe for the child's stdout.
You must set cp.out = -1 if you want to keep these two lines.

-- Hannes

Re: [PATCH] fetch: Get submodule paths from index and not from .gitmodules

[Jens Lehmann]

Am 17.09.2010 14:32, schrieb Johannes Sixt:
> Am 9/17/2010 14:22, schrieb Jens Lehmann:
>> So the correct fix should look like this:
>>
>>
>> diff --git a/submodule.c b/submodule.c
>> index e2c3bae..209efa4 100644
>> --- a/submodule.c
>> +++ b/submodule.c
>> @@ -260,7 +260,6 @@ int fetch_populated_submodules(int forced)
>>         cp.env = local_repo_env;
>>         cp.git_cmd = 1;
>>         cp.no_stdin = 1;
>> -       cp.out = -1;
>>
>>         for (i = 0; i < active_nr; i++) {
>>                 struct strbuf submodule_path = STRBUF_INIT;
> 
> That cannot be correct, either. Look further down. There you have:
> 
> 	len = strbuf_read(&buf, cp.out, 1024);
> 
> and later
> 
> 	close(cp.out);
> 
> You can do neither when you do not request a pipe for the child's stdout.
> You must set cp.out = -1 if you want to keep these two lines.

Aah, you must be looking at submodule.c from current master. Then you'll
find is_submodule_modified() at that location, where of course the -1 is
needed for cp.out because later read() and close() are used on it.

But I was trying to fix a problem introduced by 496b35e7 in current next,
where the new function fetch_populated_submodules() is located there. And
in that function the output is not read and so cp.out = 0 is the right
thing to do here.

But thanks for double checking!

Re: [PATCH] fetch: Get submodule paths from index and not from .gitmodules

[Johannes Sixt]

Am 9/17/2010 16:01, schrieb Jens Lehmann:
> But I was trying to fix a problem introduced by 496b35e7 in current next,
> where the new function fetch_populated_submodules() is located there. And
> in that function the output is not read and so cp.out = 0 is the right
> thing to do here.

Got it! You are right.

-- Hannes

Re: [PATCH] fetch: Get submodule paths from index and not from .gitmodules

[Kevin Ballard]

On Sep 17, 2010, at 5:22 AM, Jens Lehmann wrote:
>> As far as stderr of the child is concerned, if you only want to re-use the
>> standard error of the parent, then not assigning anything to cp.err is
>> correct (it was set to 0 in the memset before this hunk). But perhaps you
>> want to achieve something else?
> 
> Nope. You are right, setting both to 0 (via the memset) to inherit the
> channel from the parent is just what is needed here.
> 
> So the correct fix should look like this:
> 
> 
> diff --git a/submodule.c b/submodule.c
> index e2c3bae..209efa4 100644
> --- a/submodule.c
> +++ b/submodule.c
> @@ -260,7 +260,6 @@ int fetch_populated_submodules(int forced)
>        cp.env = local_repo_env;
>        cp.git_cmd = 1;
>        cp.no_stdin = 1;
> -       cp.out = -1;
> 
>        for (i = 0; i < active_nr; i++) {
>                struct strbuf submodule_path = STRBUF_INIT;

Yep, this patch fixes the output issue. Now all submodules are reported. However, they're still not prefixed with the previous layers, so it appears like all submodules are rooted in the current directory, which is obviously not true. And of course the performance issue is still relevant.

-Kevin Ballard

[PATCH 0/2] fix problems with recursive submodule fetching

[Jens Lehmann]

Am 18.09.2010 02:29, schrieb Kevin Ballard:
> Yep, this patch fixes the output issue. Now all submodules are reported.

Thanks again for testing this, the first patch of this series fixes
that and adds the necessary tests to not let that happen again.

The second patch lets "git fetch" use the index instead of .gitmodules
to get the list of submodules to fetch. I think this makes sense, even
if fetch didn't need to read the index until now, but the gitlink
entries are not susceptible to configuration errors like those from
.gitmodules (And "git submodule update" uses the index via "git
ls-files" too, so we'll also avoid regressions that way).

Any objections to the second patch?


> However, they're still not prefixed with the previous layers, so it appears like all submodules are rooted in the current directory, which is obviously not true.

Yes, as promised I will address this in a patch soon to come. And I
will also post a patch to add a configuration option to control the
recursive fetching.

Junio: IMO these future patches are needed before this feature can be
merged into master.


> And of course the performance issue is still relevant.

Hmm, I think it still the right thing to do to a full fetch of all
submodule changes, as you can't possibly know at fetch time what you'll
need later. But I have to admit that "git submodule update" does some
kind of optimization, as it only fetches submodules where the commit
recorded in the superproject has changed. But IMO this aims a bit too
short, as it still can lead to commits missing in submodules (which are
on branches you didn't check out and ran "git submodule update" on).

The aim of these changes is to make you able to do a fetch before you go
onto a plane and then check out every branch of the superproject without
having a commit missing from a submodule. And even more, you should be
able to use all new submodule commits not yet committed inside the
superproject without being able to forget to fetch them before you leave.

But maybe we can reduce this performance impact by running the submodule
fetches in threads. Dunno yet, I'll look into that as soon as I have the
other two patches ready.


These patches apply to current next or jl/fetch-submodule-recursive.


Jens Lehmann (2):
  fetch: Fix a bug swallowing the output of recursive submodule
    fetching
  fetch: Get submodule paths from index and not from .gitmodules

 submodule.c                 |   26 +++++++++++++++++++-------
 t/t5526-fetch-submodules.sh |   30 +++++++++++++++++++++++++-----
 2 files changed, 44 insertions(+), 12 deletions(-)

-- 
1.7.3.rc2.234.g00c0

[PATCH 1/2] fetch: Fix a bug swallowing the output of recursive submodule fetching

[Jens Lehmann]

Due to a copy and paste bug only the first level of recursion was seen
when doing a "git fetch" on a nested submodule tree. Fix that and extend
t5526 to find such breakage in the future.

Reported-by: Kevin Ballard <kevin@sb.org>
Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de>
---
 submodule.c                 |    1 -
 t/t5526-fetch-submodules.sh |   30 +++++++++++++++++++++++++-----
 2 files changed, 25 insertions(+), 6 deletions(-)

diff --git a/submodule.c b/submodule.c
index 2380638..a62cae5 100644
--- a/submodule.c
+++ b/submodule.c
@@ -256,7 +256,6 @@ int fetch_populated_submodules(int forced)
 	cp.env = local_repo_env;
 	cp.git_cmd = 1;
 	cp.no_stdin = 1;
-	cp.out = -1;

 	for_each_string_list_item(name_for_path, &config_name_for_path) {
 		struct strbuf submodule_path = STRBUF_INIT;
diff --git a/t/t5526-fetch-submodules.sh b/t/t5526-fetch-submodules.sh
index 489ef1a..cabf118 100755
--- a/t/t5526-fetch-submodules.sh
+++ b/t/t5526-fetch-submodules.sh
@@ -16,29 +16,49 @@ add_upstream_commit() {
 		git add subfile &&
 		git commit -m new subfile &&
 		head2=$(git rev-parse --short HEAD) &&
-		echo "From $pwd/submodule" > ../expect.err
+		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 commit -m new 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 init &&
-		git submodule update
+		git submodule update --init --recursive
 	) &&
-	echo "Fetching submodule submodule" > expect.out
+	echo "Fetching submodule submodule" > expect.out &&
+	echo "Fetching submodule deepsubmodule" >> expect.out
 '

 test_expect_success "fetch recurses into submodules" '
-- 
1.7.3.rc2.234.g00c0

[PATCH 2/2] fetch: Get submodule paths from index and not from .gitmodules

[Jens Lehmann]

In the first version the .gitmodules file was parsed and all submodules
found there were recursively fetched. This can lead to problems when the
.gitmodules file is not properly set up. "git submodule update" gets
this information from the index via "git ls-files", lets do the same here.

Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de>
---
 submodule.c |   25 +++++++++++++++++++------
 1 files changed, 19 insertions(+), 6 deletions(-)

diff --git a/submodule.c b/submodule.c
index a62cae5..209efa4 100644
--- a/submodule.c
+++ b/submodule.c
@@ -240,7 +240,7 @@ void show_submodule_summary(FILE *f, const char *path,

 int fetch_populated_submodules(int forced)
 {
-	int result = 0;
+	int i, result = 0;
 	struct child_process cp;
 	const char *argv[] = {
 		"fetch",
@@ -251,31 +251,44 @@ int fetch_populated_submodules(int forced)
 	if (!work_tree)
 		return 0;

+	if (!the_index.initialized)
+		if (read_cache() < 0)
+			die("index file corrupt");
+
 	memset(&cp, 0, sizeof(cp));
 	cp.argv = argv;
 	cp.env = local_repo_env;
 	cp.git_cmd = 1;
 	cp.no_stdin = 1;

-	for_each_string_list_item(name_for_path, &config_name_for_path) {
+	for (i = 0; i < active_nr; i++) {
 		struct strbuf submodule_path = STRBUF_INIT;
 		struct strbuf submodule_git_dir = STRBUF_INIT;
-		const char *git_dir;
+		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;

 		if (!forced) {
 			struct string_list_item *fetch_option;
-			fetch_option = unsorted_string_list_lookup(&config_fetch_for_name, name_for_path->util);
+			fetch_option = unsorted_string_list_lookup(&config_fetch_for_name, name);
 			if (fetch_option && !fetch_option->util)
 				continue;
 		}

-		strbuf_addf(&submodule_path, "%s/%s", work_tree, name_for_path->string);
+		strbuf_addf(&submodule_path, "%s/%s", work_tree, ce->name);
 		strbuf_addf(&submodule_git_dir, "%s/.git", submodule_path.buf);
 		git_dir = read_gitfile_gently(submodule_git_dir.buf);
 		if (!git_dir)
 			git_dir = submodule_git_dir.buf;
 		if (is_directory(git_dir)) {
-			printf("Fetching submodule %s\n", name_for_path->string);
+			printf("Fetching submodule %s\n", ce->name);
 			cp.dir = submodule_path.buf;
 			if (run_command(&cp))
 				result = 1;
-- 
1.7.3.rc2.234.g00c0

Re: [PATCH 0/2] fix problems with recursive submodule fetching

[Kevin Ballard]

On Sep 18, 2010, at 3:32 PM, Jens Lehmann wrote:

>> And of course the performance issue is still relevant.
> 
> Hmm, I think it still the right thing to do to a full fetch of all
> submodule changes, as you can't possibly know at fetch time what you'll
> need later. But I have to admit that "git submodule update" does some
> kind of optimization, as it only fetches submodules where the commit
> recorded in the superproject has changed. But IMO this aims a bit too
> short, as it still can lead to commits missing in submodules (which are
> on branches you didn't check out and ran "git submodule update" on).
> 
> The aim of these changes is to make you able to do a fetch before you go
> onto a plane and then check out every branch of the superproject without
> having a commit missing from a submodule. And even more, you should be
> able to use all new submodule commits not yet committed inside the
> superproject without being able to forget to fetch them before you leave.
> 
> But maybe we can reduce this performance impact by running the submodule
> fetches in threads. Dunno yet, I'll look into that as soon as I have the
> other two patches ready.

In a situation like mine, with 17 nested submodules, you're still going to significantly increase the time for a fetch. I understand the desire to be able to run something like `git fetch` and have everything be there, but I don't agree it should be the default behavior. Perhaps it should be a flag, like `git fetch --update-submodules` (or `git fetch --recursive`). That's friendly enough to let people know about (much friendlier than `git submodule update --recursive`), but wouldn't cause any changes to the common case. And a config variable to control whether recursive is the default would not be amiss (though I would make that config variable default to false).

I should also note that fetching all submodules may be not desired in the slightest. If I use submodules to pull in specific tagged builds of another project, I really have no desire at all to regularly update the branches in that repository. Similarly, if I pull in another project that's out of my control, I have no desire to update any submodules it may have unless the project itself changes. That's certainly the case in my project. Here's the submodule layout:


 .--Root-.
/ / /|\ \ \  
A B C D E F G
    |
    H
   /|\
  I J K
   /|\
  L M N
   /|\
  O P Q

I control the root, and I control most of the first-level of submodules. I would also not mind seeing updates to other submodules at that level, though that doesn't generally concern me. However, I don't control submodule H, and I have absolutely no desire whatsoever to see updates to I-Q unless needed by submodule H. In this layout, regularly running `git submodule update --init --recursive` works perfectly. Having `git fetch` fetch submodules I-Q on every invocation is a complete waste of my time.

-Kevin Ballard

Re: [PATCH 0/2] fix problems with recursive submodule fetching

[Jens Lehmann]

Am 19.09.2010 05:54, schrieb Kevin Ballard:
> Here's the submodule layout:
> 
>  .--Root-.
> / / /|\ \ \  
> A B C D E F G
>     |
>     H
>    /|\
>   I J K
>    /|\
>   L M N
>    /|\
>   O P Q
> 
> I control the root, and I control most of the first-level of submodules. I would also not mind seeing updates to other submodules at that level, though that doesn't generally concern me. However, I don't control submodule H, and I have absolutely no desire whatsoever to see updates to I-Q unless needed by submodule H. In this layout, regularly running `git submodule update --init --recursive` works perfectly. Having `git fetch` fetch submodules I-Q on every invocation is a complete waste of my time.

Sounds like adding  the "fetch=no" option to the .gitmodules entry
for submodule H would help you. That would tell "git fetch" to not
recurse into H and deeper.

But as you seem to be fine with running "git submodule update --init
--recursive" when needed you might be even better off by setting the
upcoming global config option to control recursive fetching to false.

But IMHO it makes more sense to let checkout take care of submodules
too and get rid of the necessity to call "git submodule update" every
time a submodule needs to be updated ...

Re: [PATCH 0/2] fix problems with recursive submodule fetching

[Kevin Ballard]

On Sep 19, 2010, at 9:40 AM, Jens Lehmann wrote:

> Am 19.09.2010 05:54, schrieb Kevin Ballard:
>> Here's the submodule layout:
>> 
>> .--Root-.
>> / / /|\ \ \  
>> A B C D E F G
>>    |
>>    H
>>   /|\
>>  I J K
>>   /|\
>>  L M N
>>   /|\
>>  O P Q
>> 
>> I control the root, and I control most of the first-level of submodules. I would also not mind seeing updates to other submodules at that level, though that doesn't generally concern me. However, I don't control submodule H, and I have absolutely no desire whatsoever to see updates to I-Q unless needed by submodule H. In this layout, regularly running `git submodule update --init --recursive` works perfectly. Having `git fetch` fetch submodules I-Q on every invocation is a complete waste of my time.
> 
> Sounds like adding  the "fetch=no" option to the .gitmodules entry
> for submodule H would help you. That would tell "git fetch" to not
> recurse into H and deeper.

The problem is that while I want to get updates from H, I don't control H so I can't add anything to its .gitmodules. I can go in and edit H's .git/config to handle this, but then everybody else that clones this project will have the exact same problem.

> But as you seem to be fine with running "git submodule update --init
> --recursive" when needed you might be even better off by setting the
> upcoming global config option to control recursive fetching to false.

Perhaps, but that negates any benefit from this patchset whatsoever. You're optimizing for the case where you control the project and every level of submodules beneath it, and you want to be notified the moment there's a commit available on any of them to pull into the parent. My argument is that two extremely common cases are you don't control all the submodules beneath yourself, and your submodules are pegged to versioned tags. In both these cases, you don't want to recursively fetch for changes every time. I think fetching everything every time has merit, but I don't think it's a worthwhile default. A far better default would be to only fetch submodules if a gitlink entry is updated in the parent repository. Given that this may be complicated, a reasonable compromise might be to update 
 submodules only if the parent repository had changes, and then this can be overridden to either always update or never update submodules.

I should also like to point out that, although the original motivation for this fetching (judging from the commit messages) seems to be to prevent you from having problems diffing the current submodule with the value stored in the gitlink, this doesn't actually do anything to help the fact that, if you have a clean tree with no changes and all submodules are up-to-date, you can run `git pull` and end up with a dirty tree as submodules are not automatically updated when their gitlink changes. I hope someone is considering modifying pull/merge behavior to update submodules automatically if their gitlink changes and they were already in a clean state with a detached HEAD pointing at the old value of the gitlink (e.g. if you checkout master, or if you make changes, don't update it).

> But IMHO it makes more sense to let checkout take care of submodules
> too and get rid of the necessity to call "git submodule update" every
> time a submodule needs to be updated ...

I agree. Checkout should be perfectly capable of handling this. Submodules are a powerful feature, but they are one of the least user-friendly aspects of git in my opinion.

-Kevin Ballard

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

[Jens Lehmann]

So here is the next iteration of the recursive fetching for submodules.

Changes to the first patchset and its fixes:

* Squashed all fixes into the first patch
* Added the new config setting 'fetch.recursive' to set the default
  for all submodules

IMO this patchset should replace the one currently in pu.


Still unresolved issues:

1) Performance

   Kevin reported the fetch time went up from 1.5s to 20s for him
   because of the recursion. Kevin, could you please test the branch
   "parallel-submodule-fetch" from my github repository:

      http://github.com/jlehmann/git-submod-enhancements.git

   It has these three patches based on next plus a preliminary
   commit fetching submodules in parallel (but note that a limit
   of 128 submodules is hardcoded and the output might be mixed
   between the fetch threads, I'll fix that when you confirm the
   performance benefit I expect).

   (It took me some time to get that working, the DNS server in my
   DSL router seems to silently drop IPv6 requests. This lead to a
   15 second timeout in getaddrinfo() when forking fetch commands
   in quick succession, turning the intended speedup into a
   massive slowdown. If you happen to run into the same issue,
   please try the "broken-dns-server-hack" branch which contains a
   workaround for that issue).


2) The proper default

   Kevin argues that submodule fetching should only happen when new
   commits for the submodule have been fetched (which is kind of
   similar to what "git submodule update" does now), while I argue
   that submodules should always be fetched by default, no matter
   if new commits from submodules are committed inside the
   superproject or not.

   Ok, lets look at the basic ways you can run the fetch command and
   look at possible defaults:

   a) "git fetch --all"

      The user wanted to fetch new commits from all remotes. I think
      git should also fetch all submodules then, no matter if new
      commits from them are fetched in the superproject, as the user
      explicitly said he wants everything. Objections?

   b) "git fetch [<repository>]"

      The user wants to fetch from the default [or a single repo]. I
      think all submodules should be fetched too, Kevin thinks this
      should happen only when it is necessary (at least for his 'H'
      repository). While I think fetching all submodules too is
      consistent with the fact that you get all branches in the
      superproject too, whether you need them or not, we can't seem
      to agree on this one (also see my proposal below).

   c) "git fetch [<repository>] <refspec>"

      If the user wants to fetch only commits from a certain refspec,
      I think that it is sane to let git fetch only those submodules
      where new commits were fetched in the superproject as he was
      explicitly asking only for a subset of available commits.

   Assuming we agree on a) and - the still to be implemented - c), It
   looks like we need a new config setting for 'fetch.recursive' and
   'submodule.<name>.fetch', say 'changed'? This would allow Kevin to
   set 'submodule.H.fetch' to 'changed' in the .gitmodules describing
   it. Then "git fetch" would only then recurse further when new
   commits from 'H' have been used in the commits fetched in its
   superproject. Would that be an acceptable solution?


Jens Lehmann (3):
  fetch/pull: Recursively fetch populated submodules
  Submodules: Add the new "fetch" config option for fetch and pull
  Add the 'fetch.recursive' config setting

 Documentation/config.txt        |   10 ++
 Documentation/fetch-options.txt |   15 +++
 Documentation/gitmodules.txt    |    8 ++
 builtin/fetch.c                 |   58 ++++++++---
 git-pull.sh                     |   10 ++-
 submodule.c                     |   91 +++++++++++++++++-
 submodule.h                     |    3 +
 t/t5526-fetch-submodules.sh     |  209 +++++++++++++++++++++++++++++++++++++++
 t/t7403-submodule-sync.sh       |    2 +-
 9 files changed, 388 insertions(+), 18 deletions(-)
 create mode 100755 t/t5526-fetch-submodules.sh

-- 
1.7.3.1.108.gb6303

[PATCH 1/3] fetch/pull: Recursively fetch populated submodules

[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" (and in "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 recursively fetches 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.

This new behavior can be disabled by using the new "--no-recursive"
option. Also the option "--submodule-prefix" is added to be able to
print out the full paths of nested submodules.

t7403 had to be changed to use the --no-recursive option for pull.

Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de>
---
 Documentation/fetch-options.txt |   14 +++++
 builtin/fetch.c                 |   51 +++++++++++++-----
 git-pull.sh                     |   10 +++-
 submodule.c                     |   66 +++++++++++++++++++++++-
 submodule.h                     |    3 +
 t/t5526-fetch-submodules.sh     |  109 +++++++++++++++++++++++++++++++++++++++
 t/t7403-submodule-sync.sh       |    2 +-
 7 files changed, 237 insertions(+), 18 deletions(-)
 create mode 100755 t/t5526-fetch-submodules.sh

diff --git a/Documentation/fetch-options.txt b/Documentation/fetch-options.txt
index 470ac31..4eb8c90 100644
--- a/Documentation/fetch-options.txt
+++ b/Documentation/fetch-options.txt
@@ -64,6 +64,20 @@ endif::git-pull[]
 	specified with the remote.<name>.tagopt setting. See
 	linkgit:git-config[1].

+--[no-]recursive::
+	By default new commits of all populated submodules will be fetched
+	too. This option can be used to disable/enable recursive fetching of
+	submodules.
+
+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 6fc5047..d41f8cc 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>...]]",
@@ -27,13 +28,14 @@ enum {
 	TAGS_SET = 2
 };

-static int all, append, dry_run, force, keep, multiple, prune, update_head_ok, verbosity;
+static int all, append, dry_run, force, keep, multiple, prune, recursive = -1, update_head_ok, verbosity;
 static int progress;
 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, "recursive", &recursive,
+		    "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,34 @@ 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 (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 +938,17 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)
 		}
 	}

+	if (!result && recursive) {
+		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..4bc8a60 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= recursive=
 merge_args=
 curr_branch=$(git symbolic-ref -q HEAD)
 curr_branch_short="${curr_branch#refs/heads/}"
@@ -105,6 +105,12 @@ do
 	--no-r|--no-re|--no-reb|--no-reba|--no-rebas|--no-rebase)
 		rebase=false
 		;;
+	--recursive)
+		recursive=--recursive
+		;;
+	--no-recursive)
+		recursive=--no-recursive
+		;;
 	--d|--dr|--dry|--dry-|--dry-r|--dry-ru|--dry-run)
 		dry_run=--dry-run
 		;;
@@ -220,7 +226,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 $recursive --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..e0230a2
--- /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 recurses into submodules" '
+	add_upstream_commit &&
+	(
+		cd downstream &&
+		git fetch >../actual.out 2>../actual.err
+	) &&
+	test_cmp expect.out actual.out &&
+	test_cmp expect.err actual.err
+'
+
+test_expect_success "fetch --no-recursive only fetches superproject" '
+	add_upstream_commit &&
+	(
+		cd downstream &&
+		git fetch --no-recursive >../actual.out 2>../actual.err
+	) &&
+	! test -s actual.out &&
+	! test -s actual.err
+'
+
+test_expect_success "--quiet propagates to submodules" '
+	(
+		cd downstream &&
+		git fetch --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 --dry-run >../actual.out 2>../actual.err
+	) &&
+	test_cmp expect.out actual.out &&
+	test_cmp expect.err actual.err &&
+	(
+		cd downstream &&
+		git fetch >../actual.out 2>../actual.err
+	) &&
+	test_cmp expect.out actual.out &&
+	test_cmp expect.err actual.err
+'
+
+test_done
diff --git a/t/t7403-submodule-sync.sh b/t/t7403-submodule-sync.sh
index 02522f9..6f3e966 100755
--- a/t/t7403-submodule-sync.sh
+++ b/t/t7403-submodule-sync.sh
@@ -50,7 +50,7 @@ test_expect_success 'change submodule url' '

 test_expect_success '"git submodule sync" should update submodule URLs' '
 	(cd super-clone &&
-	 git pull &&
+	 git pull --no-recursive &&
 	 git submodule sync
 	) &&
 	test -d "$(git config -f super-clone/submodule/.git/config \
-- 
1.7.3.1.108.gb6303

[PATCH 2/3] Submodules: Add the new "fetch" config option for fetch and pull

[Jens Lehmann]

The new boolean "fetch" config option controls the default 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.

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

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

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

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 69d91fa..bc9b768 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1791,6 +1791,12 @@ 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>.fetch::
+	A boolean to enable/disable recursive fetching of this submodule. It can
+	be overriden by using the --[no-]recursive command line option to "git
+	fetch" and "git pull".	This setting overrides any setting made in
+	.gitmodules for this submodule.
+
 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 4eb8c90..f238e3c 100644
--- a/Documentation/fetch-options.txt
+++ b/Documentation/fetch-options.txt
@@ -67,7 +67,8 @@ endif::git-pull[]
 --[no-]recursive::
 	By default new commits of all populated submodules will be fetched
 	too. This option can be used to disable/enable recursive fetching of
-	submodules.
+	submodules regardless of the 'fetch' configuration setting (see
+	linkgit:git-config[1] or linkgit:gitmodules[5]).

 ifndef::git-pull[]
 --submodule-prefix::
diff --git a/Documentation/gitmodules.txt b/Documentation/gitmodules.txt
index bcffd95..febfef4 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>.fetch::
+	A boolean 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-]recursive" 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/builtin/fetch.c b/builtin/fetch.c
index d41f8cc..d20d022 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -28,7 +28,13 @@ enum {
 	TAGS_SET = 2
 };

-static int all, append, dry_run, force, keep, multiple, prune, recursive = -1, update_head_ok, verbosity;
+enum {
+	RECURSIVE_UNSET = 0,
+	RECURSIVE_DEFAULT = 1,
+	RECURSIVE_SET = 2
+};
+
+static int all, append, dry_run, force, keep, multiple, prune, recursive = RECURSIVE_DEFAULT, update_head_ok, verbosity;
 static int progress;
 static int tags = TAGS_DEFAULT;
 static const char *depth;
@@ -55,8 +61,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, "recursive", &recursive,
-		    "control recursive fetching of submodules"),
+	OPT_SET_INT(0, "recursive", &recursive,
+		    "control recursive fetching of submodules", RECURSIVE_SET),
 	OPT_BOOLEAN(0, "dry-run", &dry_run,
 		    "dry run"),
 	OPT_BOOLEAN('k', "keep", &keep, "keep downloaded pack"),
@@ -946,6 +952,7 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)
 		add_options_to_argv(&num_options, options);
 		result = fetch_populated_submodules(num_options, options,
 						    submodule_prefix,
+						    recursive == RECURSIVE_SET,
 						    verbosity < 0);
 	}

diff --git a/submodule.c b/submodule.c
index 4d9b774..e4437b4 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_for_name;
 struct string_list config_ignore_for_name;

 static int add_submodule_odb(const char *path)
@@ -100,6 +101,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 > 5) && !strcmp(var + len - 6, ".fetch")) {
+		strbuf_add(&submodname, var, len - 6);
+		config = unsorted_string_list_lookup(&config_fetch_for_name, submodname.buf);
+		if (!config)
+			config = string_list_append(&config_fetch_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")) {
@@ -230,7 +239,7 @@ void show_submodule_summary(FILE *f, const char *path,
 }

 int fetch_populated_submodules(int num_options, const char **options,
-			       const char *prefix, int quiet)
+			       const char *prefix, int forced, int quiet)
 {
 	int i, result = 0, argc = 0;
 	struct child_process cp;
@@ -248,6 +257,8 @@ int fetch_populated_submodules(int num_options, const char **options,
 	argv[argc++] = "fetch";
 	for (i = 0; i < num_options; i++)
 		argv[argc++] = options[i];
+	if (forced)
+		argv[argc++] = "--recursive";
 	argv[argc++] = "--submodule-prefix";

 	memset(&cp, 0, sizeof(cp));
@@ -271,6 +282,13 @@ int fetch_populated_submodules(int num_options, const char **options,
 		if (name_for_path)
 			name = name_for_path->util;

+		if (!forced) {
+			struct string_list_item *fetch_option;
+			fetch_option = unsorted_string_list_lookup(&config_fetch_for_name, name);
+			if (fetch_option && !fetch_option->util)
+				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..08b422a 100644
--- a/submodule.h
+++ b/submodule.h
@@ -14,7 +14,7 @@ void show_submodule_summary(FILE *f, const char *path,
 		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);
+			       const char *prefix, int forced, 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 e0230a2..f4e3157 100755
--- a/t/t5526-fetch-submodules.sh
+++ b/t/t5526-fetch-submodules.sh
@@ -72,6 +72,46 @@ test_expect_success "fetch recurses into submodules" '
 '

 test_expect_success "fetch --no-recursive only fetches superproject" '
+	(
+		cd downstream &&
+		git fetch --no-recursive >../actual.out 2>../actual.err
+	) &&
+	! test -s actual.out &&
+	! test -s actual.err
+'
+
+test_expect_success "using fetch=false in .gitmodules only fetches superproject" '
+	(
+		cd downstream &&
+		git config -f .gitmodules submodule.submodule.fetch false &&
+		git fetch >../actual.out 2>../actual.err
+	) &&
+	! test -s actual.out &&
+	! test -s actual.err
+'
+
+test_expect_success "--recursive overrides .gitmodules config" '
+	add_upstream_commit &&
+	(
+		cd downstream &&
+		git fetch --recursive >../actual.out 2>../actual.err
+	) &&
+	test_cmp expect.out actual.out &&
+	test_cmp expect.err actual.err
+'
+
+test_expect_success "using fetch=true in .git/config overrides setting in .gitmodules" '
+	add_upstream_commit &&
+	(
+		cd downstream &&
+		git config submodule.submodule.fetch true &&
+		git fetch >../actual.out 2>../actual.err
+	) &&
+	test_cmp expect.out actual.out &&
+	test_cmp expect.err actual.err
+'
+
+test_expect_success "--no-recursive overrides fetch setting from .git/config" '
 	add_upstream_commit &&
 	(
 		cd downstream &&
@@ -106,4 +146,18 @@ test_expect_success "--dry-run propagates to submodules" '
 	test_cmp expect.err actual.err
 '

+test_expect_success "--recursive propagates to submodules" '
+	add_upstream_commit &&
+	(
+		cd downstream &&
+		(
+			cd submodule &&
+			git config -f .gitmodules submodule.deepsubmodule.fetch false
+		) &&
+		git fetch --recursive >../actual.out 2>../actual.err
+	) &&
+	test_cmp expect.out actual.out &&
+	test_cmp expect.err actual.err
+'
+
 test_done
-- 
1.7.3.1.108.gb6303

Re: [PATCH 2/3] Submodules: Add the new "fetch" config option for fetch and pull

[Jon Seymour]

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

I wonder if the name is a little too general for the function of this
configuration variable and if it might not be better to qualify it a
little further, perhaps
with the recursive suffix, e.g.:

    submodule.<name>.fetch.recursive

This would allow us to define other attributes that configure fetch on
a per submodule basis later should the need arise.

jon.

Re: [PATCH 2/3] Submodules: Add the new "fetch" config option for fetch and pull

[Jens Lehmann]

Am 07.10.2010 15:33, schrieb Jon Seymour:
>> The .gitmodules file is parsed for "submodule.<name>.fetch" entries before
>> looking for them in .git/config. Thus settings found in .git/config will
>> override those from .gitmodules, thereby allowing the local developer to
>> ignore settings given by the remote side while also letting upstream set
>> defaults for those users who don't have special needs.
>>
> 
> I wonder if the name is a little too general for the function of this
> configuration variable and if it might not be better to qualify it a
> little further, perhaps
> with the recursive suffix, e.g.:
> 
>     submodule.<name>.fetch.recursive
> 
> This would allow us to define other attributes that configure fetch on
> a per submodule basis later should the need arise.

I think that's a valid point (especially as we might add the --recursive
option to other commands too). I will change the option name as proposed
in the next version.

Re: [PATCH 2/3] Submodules: Add the new "fetch" config option for fetch and pull

[Jonathan Nieder]

Jens Lehmann wrote:
> Am 07.10.2010 15:33, schrieb Jon Seymour:

>> I wonder if the name is a little too general for the function of this
>> configuration variable and if it might not be better to qualify it a
>> little further, perhaps
>> with the recursive suffix, e.g.:
>> 
>>     submodule.<name>.fetch.recursive
>> 
>> This would allow us to define other attributes that configure fetch on
>> a per submodule basis later should the need arise.
>
> I think that's a valid point (especially as we might add the --recursive
> option to other commands too). I will change the option name as proposed
> in the next version.

That would look like

	[submodule "<name>"]
		fetch.recursive = ...

in files with .git/config syntax.  Maybe fetchRecursive or similar
would be more consistent with existing variables?

I'm not personally invested in any name; just thought that reminder
might be useful.

Re: [PATCH 2/3] Submodules: Add the new "fetch" config option for fetch and pull

[Jens Lehmann]

Am 09.10.2010 21:54, schrieb Jonathan Nieder:
> Jens Lehmann wrote:
>> I think that's a valid point (especially as we might add the --recursive
>> option to other commands too). I will change the option name as proposed
>> in the next version.
> 
> That would look like
> 
> 	[submodule "<name>"]
> 		fetch.recursive = ...
> 
> in files with .git/config syntax.  Maybe fetchRecursive or similar
> would be more consistent with existing variables?

Yes, it looks like fetchRecursive would be a better choice.

[PATCH 3/3] Add the 'fetch.recursive' config setting

[Jens Lehmann]

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

Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de>
---
 Documentation/config.txt    |    4 +++
 submodule.c                 |    7 +++++
 t/t5526-fetch-submodules.sh |   56 +++++++++++++++++++++++++++++++++++++++----
 3 files changed, 62 insertions(+), 5 deletions(-)

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

+fetch.recursive::
+	A boolean value which overrides the default behavior of fetch and
+	pull, which is to recursively fetch populated sumodules too.
+
 fetch.unpackLimit::
 	If the number of objects fetched over the git native
 	transfer is below this
diff --git a/submodule.c b/submodule.c
index e4437b4..d4adb37 100644
--- a/submodule.c
+++ b/submodule.c
@@ -12,6 +12,7 @@
 struct string_list config_name_for_path;
 struct string_list config_fetch_for_name;
 struct string_list config_ignore_for_name;
+static int config_fetch_recursive = 1;

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

@@ -287,6 +292,8 @@ int fetch_populated_submodules(int num_options, const char **options,
 			fetch_option = unsorted_string_list_lookup(&config_fetch_for_name, name);
 			if (fetch_option && !fetch_option->util)
 				continue;
+			if (!fetch_option && !config_fetch_recursive)
+				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 f4e3157..35ef2ff 100755
--- a/t/t5526-fetch-submodules.sh
+++ b/t/t5526-fetch-submodules.sh
@@ -16,8 +16,8 @@ add_upstream_commit() {
 		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
+		echo "From $pwd/submodule" > ../expect_1st.err &&
+		echo "   $head1..$head2  master     -> origin/master" >> ../expect_1st.err
 	)
 	(
 		cd deepsubmodule &&
@@ -27,9 +27,10 @@ add_upstream_commit() {
 		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
-	)
+		echo "From $pwd/deepsubmodule" > ../expect_2nd.err &&
+		echo "   $head1..$head2  master     -> origin/master" >> ../expect_2nd.err
+	) &&
+	cat expect_1st.err expect_2nd.err > expect.err
 }

 test_expect_success setup '
@@ -58,6 +59,7 @@ test_expect_success setup '
 		git submodule update --init --recursive
 	) &&
 	echo "Fetching submodule submodule" > expect.out &&
+	cp expect.out expect_1st.out &&
 	echo "Fetching submodule submodule/deepsubmodule" >> expect.out
 '

@@ -160,4 +162,48 @@ test_expect_success "--recursive propagates to submodules" '
 	test_cmp expect.err actual.err
 '

+test_expect_success "fetch.recursive sets default and --recursive overrides it" '
+	add_upstream_commit &&
+	(
+		cd downstream &&
+		(
+			cd submodule &&
+			git config -f .gitmodules --unset submodule.deepsubmodule.fetch &&
+			git config fetch.recursive false
+		) &&
+		git fetch >../actual.out 2>../actual.err
+	) &&
+	test_cmp expect_1st.out actual.out &&
+	test_cmp expect_1st.err actual.err &&
+	(
+		cd downstream &&
+		git fetch --recursive >../actual.out 2>../actual.err
+	) &&
+	test_cmp expect.out actual.out &&
+	test_cmp expect_2nd.err actual.err
+'
+
+test_expect_success "fetch setting from .git/config overrides fetch.recursive config setting" '
+	add_upstream_commit &&
+	(
+		cd downstream &&
+		git config submodule.submodule.fetch true &&
+		git fetch >../actual.out 2>../actual.err
+	) &&
+	test_cmp expect_1st.out actual.out &&
+	test_cmp expect_1st.err actual.err &&
+	(
+		cd downstream &&
+		(
+			cd submodule &&
+			git config --unset fetch.recursive
+		) &&
+		git config fetch.recursive false &&
+		git config submodule.submodule.fetch true &&
+		git fetch >../actual.out 2>../actual.err
+	) &&
+	test_cmp expect.out actual.out &&
+	test_cmp expect_2nd.err actual.err
+'
+
 test_done
-- 
1.7.3.1.108.gb6303

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

[Junio C Hamano]

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

>    a) "git fetch --all"
>
>       The user wanted to fetch new commits from all remotes. I think
>       git should also fetch all submodules then, no matter if new
>       commits from them are fetched in the superproject, as the user
>       explicitly said he wants everything. Objections?

Why?  I do not see a "--submodules" option on that command line.  The only
thing I asked is to grab all branches for the project I ran "git fetch"
in.

>    b) "git fetch [<repository>]"
>
>       The user wants to fetch from the default [or a single repo]. I
>       think all submodules should be fetched too, Kevin thinks this
>       should happen only when it is necessary (at least for his 'H'
>       repository). While I think fetching all submodules too is
>       consistent with the fact that you get all branches in the
>       superproject too, whether you need them or not, we can't seem
>       to agree on this one (also see my proposal below).

The case with <repository> is a lot more questionable than the case of
fetching implicitly from whereever you usually fetch from.  Imagine that
you fork git.git, and create a separate project that has some nifty
additions to support submodules better.  The additional part is naturally
done as a submodule.  This jens.git repository becomes very popular and
people clone from it.  Your users usually interact with your repository by
saying "git fetch" or "git pull" without any explicit <repository>.  They,
however, would want to fetch/pull from me from time to time to get updates
that you haven't incorporated in jens.git repository. "git fetch junio" is
run.  Why should such a "fetch" go to your repository and slurp the
objects for the submodules?

Perhaps you would want some knobs like these?

        [remote "origin"]
                fetch-submodules = all
                fetch-submodules = changed

        [remote "junio"]
                fetch-submodules = none

I dunno.  I've never been a fan of automatically recursing into submodules
(iow, treating the nested structure as if there is no nesting), so...

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

[Kevin Ballard]

On Oct 5, 2010, at 1:43 PM, Jens Lehmann wrote:

>   Kevin reported the fetch time went up from 1.5s to 20s for him
>   because of the recursion. Kevin, could you please test the branch
>   "parallel-submodule-fetch" from my github repository:
> 
>      http://github.com/jlehmann/git-submod-enhancements.git
> 
>   It has these three patches based on next plus a preliminary
>   commit fetching submodules in parallel (but note that a limit
>   of 128 submodules is hardcoded and the output might be mixed
>   between the fetch threads, I'll fix that when you confirm the
>   performance benefit I expect).

The first `git fetch` still took 20 seconds, but that's because there was data to fetch from one of the deeply-nested submodules (data which, incidentally, I have zero reason to want to fetch). Subsequent fetches took 6.3 seconds. This is contrasted with 1.9s to run `git -c fetch.recursive=false fetch`.

On Oct 5, 2010, at 2:06 PM, Junio C Hamano wrote:

>>   a) "git fetch --all"
>> 
>>      The user wanted to fetch new commits from all remotes. I think
>>      git should also fetch all submodules then, no matter if new
>>      commits from them are fetched in the superproject, as the user
>>      explicitly said he wants everything. Objections?
> 
> Why?  I do not see a "--submodules" option on that command line.  The only
> thing I asked is to grab all branches for the project I ran "git fetch"
> in.

I agree with Junio.

>>   b) "git fetch [<repository>]"
>> 
>>      The user wants to fetch from the default [or a single repo]. I
>>      think all submodules should be fetched too, Kevin thinks this
>>      should happen only when it is necessary (at least for his 'H'
>>      repository). While I think fetching all submodules too is
>>      consistent with the fact that you get all branches in the
>>      superproject too, whether you need them or not, we can't seem
>>      to agree on this one (also see my proposal below).
> 
> The case with <repository> is a lot more questionable than the case of
> fetching implicitly from whereever you usually fetch from.  Imagine that
> you fork git.git, and create a separate project that has some nifty
> additions to support submodules better.  The additional part is naturally
> done as a submodule.  This jens.git repository becomes very popular and
> people clone from it.  Your users usually interact with your repository by
> saying "git fetch" or "git pull" without any explicit <repository>.  They,
> however, would want to fetch/pull from me from time to time to get updates
> that you haven't incorporated in jens.git repository. "git fetch junio" is
> run.  Why should such a "fetch" go to your repository and slurp the
> objects for the submodules?
> 
> Perhaps you would want some knobs like these?
> 
>        [remote "origin"]
>                fetch-submodules = all
>                fetch-submodules = changed
> 
>        [remote "junio"]
>                fetch-submodules = none
> 
> I dunno.  I've never been a fan of automatically recursing into submodules
> (iow, treating the nested structure as if there is no nesting), so...

I agree with this as well.

After thinking on it a bit, I think the best solution is to add a switch --submodules to fetch which will also fetch all submodules, but otherwise fetch will fetch no submodules. This will avoid the problem of detecting changed submodules, while still allowing users to explicitly request all submodules in case they're about to get on a plane flight. And of course we can use a config switch to turn --submodules on or off by default. We should also give some thought to automatically updating submodules when `git pull` is performed. I could imagine `git pull --submodules` effectively doing `git pull && git submodule update --init --recursive`, though this implies submodule updating behavior as part of merge, and it seems harder to justify that.

-Kevin Ballard

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

[Jonathan Nieder]

Kevin Ballard wrote:

> After thinking on it a bit, I think the best solution is to add a switch
> --submodules to fetch which will also fetch all submodules, but otherwise
> fetch will fetch no submodules.

For what it's worth, for my (odd) use cases, what would be most practical
is

	[remote "foo"]
		fetch = ...
		submodules = ...

I could care less about the defaults. :)

Rationale: I shouldn't have to explicitly use --submodules to get
everything I need to hack before dropping connectivity, but I also
don't want to pay the penalty of fetching, say, git-gui when I don't
need it.

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

[Jens Lehmann]

Am 07.10.2010 01:22, schrieb Jonathan Nieder:
> Kevin Ballard wrote:
> 
>> After thinking on it a bit, I think the best solution is to add a switch
>> --submodules to fetch which will also fetch all submodules, but otherwise
>> fetch will fetch no submodules.
> 
> For what it's worth, for my (odd) use cases, what would be most practical
> is
> 
> 	[remote "foo"]
> 		fetch = ...
> 		submodules = ...
> 
> I could care less about the defaults. :)
> 
> Rationale: I shouldn't have to explicitly use --submodules to get
> everything I need to hack before dropping connectivity,

Ack.


> but I also
> don't want to pay the penalty of fetching, say, git-gui when I don't
> need it.

I'm not sure I understand your setup, do you want to configure that for
each remote or for each submodule? Because you can already do the latter
with my patches. And configuring that via .gitmodules has the advantage
that every clone inherits that setting instead of having to set the
options for the remote again after each clone.

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

[Jonathan Nieder]

Jens Lehmann wrote:
> Am 07.10.2010 01:22, schrieb Jonathan Nieder:

>>     I also
>> don't want to pay the penalty of fetching, say, git-gui when I don't
>> need it.
>
> I'm not sure I understand your setup, do you want to configure that for
> each remote or for each submodule?

Without thinking too hard about it, I imagine listing submodules in
the [remote "foo"] stanza, just like I list branches there now.

> Because you can already do the latter
> with my patches. And configuring that via .gitmodules has the advantage
> that every clone inherits that setting

I certainly do _not_ want that property.  Upstream can tell what
submodules to check out by default, but aside from that, the choice of
what to fetch has nothing to do with them.

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

[Jens Lehmann]

Am 09.10.2010 22:02, schrieb Jonathan Nieder:
> Jens Lehmann wrote:
>> Am 07.10.2010 01:22, schrieb Jonathan Nieder:
> 
>>>     I also
>>> don't want to pay the penalty of fetching, say, git-gui when I don't
>>> need it.
>>
>> I'm not sure I understand your setup, do you want to configure that for
>> each remote or for each submodule?
> 
> Without thinking too hard about it, I imagine listing submodules in
> the [remote "foo"] stanza, just like I list branches there now.

Yup, that makes sense.


>> Because you can already do the latter
>> with my patches. And configuring that via .gitmodules has the advantage
>> that every clone inherits that setting
> 
> I certainly do _not_ want that property.  Upstream can tell what
> submodules to check out by default, but aside from that, the choice of
> what to fetch has nothing to do with them.

Ok, I take that as a vote to remove the parsing of .gitmodules in patch
2/3. But I assume you are fine with being able to configure that for
each submodule via .git/config?

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

[Jonathan Nieder]

Jens Lehmann wrote:
> Am 09.10.2010 22:02, schrieb Jonathan Nieder:
>> Jens Lehmann wrote:

>>>                  And configuring that via .gitmodules has the advantage
>>> that every clone inherits that setting
>> 
>> I certainly do _not_ want that property.
[...]
> Ok, I take that as a vote to remove the parsing of .gitmodules in patch
> 2/3. But I assume you are fine with being able to configure that for
> each submodule via .git/config?

I've lost track of the thread, unfortunately.

But here are my thoughts, quickly:

The place of .gitmodules
------------------------
I think of .gitmodules as a place where upstream places information
about the submodules contained in a repository, what they are about,
and how to get them.  So ideally it would look like this:

	[submodule "gnulib"]
		path = lib/gnulib
		description = GNU Portability Library
		url = ../gnulib.git
		checkout = true

meaning the gnulib is stored at lib/gnulib, can be acquired from
that URL, and needs to be checked out in all but unusual
configurations to build this program.

Already the '[submodule "gnulib"] update' setting goes beyond this
expectation, by including information about policy that has nothing
to do with the superproject.  If we were starting over, I think
"git submodule update" might benefit from being split into two
operations:

	git checkout --recursive

for the update = checkout behavior, and

	git submodule update --pull

for the update = merge/rebase behavior.  The choice between merge and
rebase for the latter would use the usual '[branch "master"] rebase'
configuration, with defaults taken from '[branch] autosetuprebase'.
Maybe a superproject could override that choice in .git/config using
[submodule "foo"] configuration.

Well, it should be obvious by now that I think submodule.<foo>.fetch
does not belong in .gitmodules.

Automatic fetching of submodules
--------------------------------
A '[submodule "<foo>"] fetch' tweakable in .git/config like you
propose makes perfect sense.  But what should the default be?

 1. A conservative answer is "never fetch by default".

 2. A simple but less conservative answer is "fetch the modules
    with checkout = true by default".  (Of course I am not attached to
    the names; this is just a sketch.)

 3. There are other more complicated answers, but I'm a strong
    believer in "keep it simple, especially when the code doesn't
    exist yet".

Hope that helps.

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

[Jens Lehmann]

Am 21.10.2010 20:29, schrieb Jonathan Nieder:
> Hope that helps.

Yes, thanks a lot!

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

[Jens Lehmann]

Am 07.10.2010 00:52, schrieb Kevin Ballard:
> On Oct 5, 2010, at 2:06 PM, Junio C Hamano wrote:
>> I dunno.  I've never been a fan of automatically recursing into submodules
>> (iow, treating the nested structure as if there is no nesting), so...
> 
> I agree with this as well.

There are use cases like mine where automatic recursion is just the right
thing to do. But I would be fine with having to turn the recursion on
explicitly in the configuration if most people think recursion is not a
desirable default. It would be really nice to hear from other submodule
users what they think about that ...


> After thinking on it a bit, I think the best solution is to add a switch
> --submodules to fetch which will also fetch all submodules, but otherwise
> fetch will fetch no submodules. This will avoid the problem of detecting
> changed submodules, while still allowing users to explicitly request all
> submodules in case they're about to get on a plane flight.
> And of course we can use a config switch to turn --submodules on or off
> by default.

And apart from the default setting and the name of the option this is
exactly what this patch series does, or am I missing something? I could
rename the option from '--recursive' to '--submodules' if that is
requested (I chose the former for consistency reasons as it is already
used by "git clone" for the same purpose; and IMO we should then use
the same option name for recursive checkout and grep too).


> We should also give some thought to automatically updating submodules
> when `git pull` is performed. I could imagine `git pull --submodules`
> effectively doing `git pull && git submodule update --init --recursive`,
> though this implies submodule updating behavior as part of merge, and
> it seems harder to justify that.

This is the goal of my recursive checkout effort ("update" being the
easier part, "--init" the harder). And if you have recursive checkout
enabled, I think it is justified to update the submodules as part of
the merge.

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

[Marc Branchaud]

On 10-10-09 03:17 PM, Jens Lehmann wrote:
> Am 07.10.2010 00:52, schrieb Kevin Ballard:
>> On Oct 5, 2010, at 2:06 PM, Junio C Hamano wrote:
>>> I dunno.  I've never been a fan of automatically recursing into submodules
>>> (iow, treating the nested structure as if there is no nesting), so...
>>
>> I agree with this as well.
> 
> There are use cases like mine where automatic recursion is just the right
> thing to do. But I would be fine with having to turn the recursion on
> explicitly in the configuration if most people think recursion is not a
> desirable default. It would be really nice to hear from other submodule
> users what they think about that ...

I tend to think that the right default for fetch is to employ the same level
of recursion that was used for the initial clone.  So if the clone was made
with --recursive then fetch should default to using --recursive.

But I'd like to see finer-grained control than that.  For us the set of
submodules to clone depends on what we're trying to build.  Ideally we'd have
a lot of different submodules, and some would be required no matter what the
build target.  It'd be great if clone could be smart enough to recursively
clone those required submodules (i.e. the upstream repo specifies a set of
default submodules -- I believe this is already on Jens's TODO list).  Then
building a particular target could trigger the cloning of ancillary
submodules specific to that target.

In that scenario, the default for later fetches could be to either (a)
retrieve upstream's default set of submodules, or (b) retrieve all populated
submodules.  Either way, a config option is needed to override the default
behaviour, with a third configurable-but-never-default setting to recursively
fetch all submodules, populated or not.

		M.

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

[Jens Lehmann]

Am 13.10.2010 16:48, schrieb Marc Branchaud:
> On 10-10-09 03:17 PM, Jens Lehmann wrote:
>> Am 07.10.2010 00:52, schrieb Kevin Ballard:
>>> On Oct 5, 2010, at 2:06 PM, Junio C Hamano wrote:
>>>> I dunno.  I've never been a fan of automatically recursing into submodules
>>>> (iow, treating the nested structure as if there is no nesting), so...
>>>
>>> I agree with this as well.
>>
>> There are use cases like mine where automatic recursion is just the right
>> thing to do. But I would be fine with having to turn the recursion on
>> explicitly in the configuration if most people think recursion is not a
>> desirable default. It would be really nice to hear from other submodule
>> users what they think about that ...
> 
> I tend to think that the right default for fetch is to employ the same level
> of recursion that was used for the initial clone.  So if the clone was made
> with --recursive then fetch should default to using --recursive.

That's a very interesting idea.


> But I'd like to see finer-grained control than that.  For us the set of
> submodules to clone depends on what we're trying to build.  Ideally we'd have
> a lot of different submodules, and some would be required no matter what the
> build target.  It'd be great if clone could be smart enough to recursively
> clone those required submodules (i.e. the upstream repo specifies a set of
> default submodules -- I believe this is already on Jens's TODO list).

Yup, right now I have new entries in .gitmodules in mind which will
tell clone to clone certain submodules too. I'm just not so sure if
those should be the same I intend to use for recursive checkout ...


>  Then
> building a particular target could trigger the cloning of ancillary
> submodules specific to that target.

And the build could do that using my proposal by adding entries to the
submodule configs in .git/config, which override those from .gitmodules.


> In that scenario, the default for later fetches could be to either (a)
> retrieve upstream's default set of submodules, or (b) retrieve all populated
> submodules.  Either way, a config option is needed to override the default
> behaviour, with a third configurable-but-never-default setting to recursively
> fetch all submodules, populated or not.

If I understand that correctly my proposal should handle all these cases.
(a) could be done by configuring the entries in .gitmodules (even though
there have been votes against that feature), (b) can be achieved by setting
the repo-wide option. But I fear that its impossible to fetch non-populated
submodules, as they lack a .git directory to fetch into.

Thanks for your feedback!

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

[Kevin Ballard]

On Oct 13, 2010, at 12:32 PM, Jens Lehmann wrote:

>>> There are use cases like mine where automatic recursion is just the right
>>> thing to do. But I would be fine with having to turn the recursion on
>>> explicitly in the configuration if most people think recursion is not a
>>> desirable default. It would be really nice to hear from other submodule
>>> users what they think about that ...
>> 
>> I tend to think that the right default for fetch is to employ the same level
>> of recursion that was used for the initial clone.  So if the clone was made
>> with --recursive then fetch should default to using --recursive.
> 
> That's a very interesting idea.

I'm not sure it's correct though. For example, with my scenario every single submodule is required for a correct build, but most submodules should definitely not be updated unless their parent submodule updates its gitlink. So --recursive is recommended for `git clone`, but a non-recursive fetch would be the correct behavior going forward.

-Kevin Ballard

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

[Jens Lehmann]

Am 13.10.2010 21:34, schrieb Kevin Ballard:
> On Oct 13, 2010, at 12:32 PM, Jens Lehmann wrote:
> 
>>>> There are use cases like mine where automatic recursion is just the right
>>>> thing to do. But I would be fine with having to turn the recursion on
>>>> explicitly in the configuration if most people think recursion is not a
>>>> desirable default. It would be really nice to hear from other submodule
>>>> users what they think about that ...
>>>
>>> I tend to think that the right default for fetch is to employ the same level
>>> of recursion that was used for the initial clone.  So if the clone was made
>>> with --recursive then fetch should default to using --recursive.
>>
>> That's a very interesting idea.
> 
> I'm not sure it's correct though. For example, with my scenario every single submodule is required for a correct build, but most submodules should definitely not be updated unless their parent submodule updates its gitlink. So --recursive is recommended for `git clone`, but a non-recursive fetch would be the correct behavior going forward.

For *your* use case it might not be correct, but for others it may very
well be.

We need to get more user stories like that to get an overview about what
config options are useful and what might be reasonable defaults for them.
And then we can decide what set of options and defaults to choose.

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

[Kevin Ballard]

On Oct 13, 2010, at 1:06 PM, Jens Lehmann wrote:

> Am 13.10.2010 21:34, schrieb Kevin Ballard:
>> On Oct 13, 2010, at 12:32 PM, Jens Lehmann wrote:
>> 
>>>>> There are use cases like mine where automatic recursion is just the right
>>>>> thing to do. But I would be fine with having to turn the recursion on
>>>>> explicitly in the configuration if most people think recursion is not a
>>>>> desirable default. It would be really nice to hear from other submodule
>>>>> users what they think about that ...
>>>> 
>>>> I tend to think that the right default for fetch is to employ the same level
>>>> of recursion that was used for the initial clone.  So if the clone was made
>>>> with --recursive then fetch should default to using --recursive.
>>> 
>>> That's a very interesting idea.
>> 
>> I'm not sure it's correct though. For example, with my scenario every single submodule is required for a correct build, but most submodules should definitely not be updated unless their parent submodule updates its gitlink. So --recursive is recommended for `git clone`, but a non-recursive fetch would be the correct behavior going forward.
> 
> For *your* use case it might not be correct, but for others it may very
> well be.

My inclination is to say recursively fetching submodules is a choice many maintainers may want to make, but not something any simple user of a repo will want to do. If I find a neat project out there and `git clone --recursive` it, there's no reason to believe I'll want to track updates to its submodules, as I have no plans on ever modifying this repo myself or creating any commits.

-Kevin Ballard

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

[Chris Packham]

On 13/10/10 13:06, Jens Lehmann wrote:
> 
> For *your* use case it might not be correct, but for others it may very
> well be.
> 
> We need to get more user stories like that to get an overview about what
> config options are useful and what might be reasonable defaults for them.
> And then we can decide what set of options and defaults to choose.

We'll if you want it here's an example of how we use submodules at $dayjob.

Most developers initially clone the project repo and run git submodule
update --init (or git submodule init && git submodule update if their
git version is too old).  This could be replaced by git clone
--recursive, or just git clone if we there was a config for enabling
recursive cloning by default.

A few newbies have been confused by the fact that they run git clone but
don't actually end up with any code (in our case the superprojects are
just containers with no actual code themselves).

We have a continuous integration machine that does a git pull on all
submodules, a few automated tests and updates the superproject if the
tests pass. This would make use of git fetch/pull --recursive to grab
all the latest changes.

After the initial clone developers run git pull on the project. for
modules they aren't working on they run git submodule update <module>.
For modules they are working on they to rebase their working branch to
the SHA1 recorded in the superproject. I think this kind of thing has
already been discussed on the list, not sure that I've seen a solution
that would work for us. For now all of this is is wrapped in a script
for the developers.

Developers would probably want the fetch-if-super-gitlink-has-changed
behaviour. We also need to handle rebasing a submodule's checkedout
branch (if present) against the recorded submodule SHA1. This could
remain "our problem" as long as the worktree of these branches does not
get updated we can simply use the existing rebase logic we have today.

Hope that is a useful use-case.
--
- Chris

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

[Jens Lehmann]

Am 14.10.2010 03:01, schrieb Chris Packham:
> For modules they are working on they to rebase their working branch to
> the SHA1 recorded in the superproject. I think this kind of thing has
> already been discussed on the list, not sure that I've seen a solution
> that would work for us. For now all of this is is wrapped in a script
> for the developers.

I assume you know the "--rebase" option and the "update=rebase" config
setting for "git submodule update" and they don't work for you?


> Developers would probably want the fetch-if-super-gitlink-has-changed
> behaviour.

Yeah, I think we need to support the fetch-if-super-gitlink-has-changed,
always-fetch-all and never-fetch behavior. So in your example you could
set fetch-if-super-gitlink-has-changed in the global config and use
fetch-all for the submodule you are hacking on by changing its config
if you want that.


> Hope that is a useful use-case.

Sure, thanks!

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

[Chris Packham]

On 14/10/10 11:14, Jens Lehmann wrote:
> Am 14.10.2010 03:01, schrieb Chris Packham:
>> For modules they are working on they to rebase their working branch to
>> the SHA1 recorded in the superproject. I think this kind of thing has
>> already been discussed on the list, not sure that I've seen a solution
>> that would work for us. For now all of this is is wrapped in a script
>> for the developers.
> 
> I assume you know the "--rebase" option and the "update=rebase" config
> setting for "git submodule update" and they don't work for you?
>

That's news to me. Our most of our developers are running git 1.6.3 so
it won't work for them. I'll start changing our scripts to at least set
that variable in preparation for our next update (probably won't be for
a while). I can test it out on my PC and see if it suits our workflow.

> 
>> Developers would probably want the fetch-if-super-gitlink-has-changed
>> behaviour.
> 
> Yeah, I think we need to support the fetch-if-super-gitlink-has-changed,
> always-fetch-all and never-fetch behavior. So in your example you could
> set fetch-if-super-gitlink-has-changed in the global config and use
> fetch-all for the submodule you are hacking on by changing its config
> if you want that.
> 
> 
>> Hope that is a useful use-case.
> 
> Sure, thanks!

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

[Marc Branchaud]

On 10-10-13 03:34 PM, Kevin Ballard wrote:
> On Oct 13, 2010, at 12:32 PM, Jens Lehmann wrote:
> 
>>>> There are use cases like mine where automatic recursion is just the
>>>> right thing to do. But I would be fine with having to turn the
>>>> recursion on explicitly in the configuration if most people think
>>>> recursion is not a desirable default. It would be really nice to
>>>> hear from other submodule users what they think about that ...
>>> 
>>> I tend to think that the right default for fetch is to employ the same
>>> level of recursion that was used for the initial clone.  So if the
>>> clone was made with --recursive then fetch should default to using
>>> --recursive.
>> 
>> That's a very interesting idea.
> 
> I'm not sure it's correct though. For example, with my scenario every
> single submodule is required for a correct build, but most submodules
> should definitely not be updated unless their parent submodule updates its
> gitlink. So --recursive is recommended for `git clone`, but a
> non-recursive fetch would be the correct behavior going forward.

What about a "smart" recursive fetch?  One where if *any* new ref in the
superproject changes the superproject's submodule ref, *and* that submodule
ref isn't already in the submodule repo, then fetch updates the submodule.
Recurse as needed.

That way I don't think there'd be any missing commits when checking out
different branches in the superproject, and submodule fetches are minimized.

Not sure how easy that would be to implement though, or what the performance
would be like.

		M.

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

[Kevin Ballard]

On Oct 13, 2010, at 2:27 PM, Marc Branchaud wrote:

>> I'm not sure it's correct though. For example, with my scenario every
>> single submodule is required for a correct build, but most submodules
>> should definitely not be updated unless their parent submodule updates its
>> gitlink. So --recursive is recommended for `git clone`, but a
>> non-recursive fetch would be the correct behavior going forward.
> 
> What about a "smart" recursive fetch?  One where if *any* new ref in the
> superproject changes the superproject's submodule ref, *and* that submodule
> ref isn't already in the submodule repo, then fetch updates the submodule.
> Recurse as needed.
> 
> That way I don't think there'd be any missing commits when checking out
> different branches in the superproject, and submodule fetches are minimized.
> 
> Not sure how easy that would be to implement though, or what the performance
> would be like.

That was actually my original recommendation. I've since backed away from it, due to concerns about difficulty in implementation, but if it can be done then this would be rather ideal for most common cases.

-Kevin Ballard

Re: [RFC PATCH 0/2] Teach fetch and pull to recursively fetch submodules too

[Jens Lehmann]

Am 15.09.2010 02:18, schrieb Kevin Ballard:
> The first is `git remote update`, which I am fond of using, causes buggy behavior where it fetches all submodules twice.

I'll look into that, fetching the same submodule twice should not
happen. (But I'm not sure what you mean by "buggy behavior where it
fetches all submodules twice" though, is there something else going
wrong?)


> The second is this submodule fetch doesn't appear to be recursive.

That is strange as fetch now executes a 'git fetch' inside each
populated submodule, which should automagically recurse. So I assume
that you don't have a .gitmodules file inside your first level of
submodules which describes the deeper nested ones?


> The third issue is `git fetch` doesn't have any business fetching submodules when the submodule reference was never changed as part of the fetch, especially if the main fetch itself didn't even find any changes. It seems to me that the correct behavior would be to look at all the fetched commits to see if any of them change the submodule reference, and only in that case should it automatically fetch any submodules whose references were modified. The stated desire of avoiding "(commits not present)" when doing a diff will still be met, but it will avoid slowing down the normal case of a `git fetch`.

I was thinking about implementing that optimization too but came to
the conclusion that you always have to fetch submodules too no matter
if the superproject has any new commits fetched for them. Because
when you don't do that you wouldn't get a chance to test and commit
e.g. a new feature added by a colleague in a submodule and pushed by
him, as that would not be fetched into your repo before it was
committed inside the superproject. That leads to a 'chicken or the
egg' problem.


> It also seems like there ought to be a config variable one can set for the default behavior if submodule.<name>.fetch is not present in .gitmodules or in .git/config.

I'll take this as a 'yes' to my question if such an option is wanted.


In your other mail you wrote that the output of the recursive fetch
does not prepend the names of the higher level submodules. I did not
think about that and will post a fix for that problem soon.


Thanks for your feedback!

Re: [RFC PATCH 0/2] Teach fetch and pull to recursively fetch submodules too

[Kevin Ballard]

On Sep 15, 2010, at 4:32 AM, Jens Lehmann wrote:

> Am 15.09.2010 02:18, schrieb Kevin Ballard:
>> The first is `git remote update`, which I am fond of using, causes buggy behavior where it fetches all submodules twice.
> 
> I'll look into that, fetching the same submodule twice should not
> happen. (But I'm not sure what you mean by "buggy behavior where it
> fetches all submodules twice" though, is there something else going
> wrong?)

Nothing else wrong, the bug was simply that it fetches all submodules twice. And if it helps, I only have one remote.

>> The second is this submodule fetch doesn't appear to be recursive.
> 
> That is strange as fetch now executes a 'git fetch' inside each
> populated submodule, which should automagically recurse. So I assume
> that you don't have a .gitmodules file inside your first level of
> submodules which describes the deeper nested ones?

I do indeed have a .gitmodules file. My normal approach of `git submodule update --init --recursive` works perfectly fine to update the entire tree of submodules, even on a brand new clone.

Upon further investigation, judging from the time spent on each submodule, it may indeed be recursing. The odd part then, is that a `git fetch` at the top level reports that it's fetching one of the nested submodules, even though a `git fetch` inside that submodule doesn't report any of its nested submodules being fetched.

Here's an equivalent tree for the submodules I have:

  .--Root-.
 / / /|\ \ \  
A B C D E F G
      |
      H
     /|\
    I J K
     /|\--.
    L M N J'
     /|\
    O P Q

J' is listed in .gitmodules but doesn't actually exist in the tree anymore. I'm not sure if this makes a difference.

When I run `git fetch` at the root level, I see the following:

Fetching submodule A
Fetching submodule B
Fetching submodule C
Fetching submodule H
Fetching submodule D
Fetching submodule E
Fetching submodule F
Fetching submodule G

This exactly matches the ordering of the submodules in .gitmodules, except for the stray fetch of H listed after C. If I cd into C and run `git fetch`, I only see

Fetching submodule H

Similarly if I cd into H and fetch, I only see I, J, and K.

I cannot figure out why it is displaying "Fetching submodule H" at this root level. As I said above, going by how long it spends fetching the submodule, I assume it is actually recursing appropriately.

>> The third issue is `git fetch` doesn't have any business fetching submodules when the submodule reference was never changed as part of the fetch, especially if the main fetch itself didn't even find any changes. It seems to me that the correct behavior would be to look at all the fetched commits to see if any of them change the submodule reference, and only in that case should it automatically fetch any submodules whose references were modified. The stated desire of avoiding "(commits not present)" when doing a diff will still be met, but it will avoid slowing down the normal case of a `git fetch`.
> 
> I was thinking about implementing that optimization too but came to
> the conclusion that you always have to fetch submodules too no matter
> if the superproject has any new commits fetched for them. Because
> when you don't do that you wouldn't get a chance to test and commit
> e.g. a new feature added by a colleague in a submodule and pushed by
> him, as that would not be fetched into your repo before it was
> committed inside the superproject. That leads to a 'chicken or the
> egg' problem.

In my scenario, running `git fetch` will fetch a grand total of 17 submodules. This makes fetching extremely slow. And the normal workflow here is if one of my coworkers commits a change to submodule D, and the submodule is in an appropriate state to be updated in the root project, he will go ahead and update it in that root project. Alternatively, if he's making the change to satisfy a request made by me, he will let me know when it's done and I will update it. Otherwise, I don't want to know when he pushes changes. So the ideal behavior for me is only fetch a submodule if the superproject's fetch actually fetched new trees, and one of those trees references a new commit in this submodule that doesn't already exist. If I want to know about changes that aren't referenced by the superprojec
 t, I would simply run `git submodule update --recursive`.

I'm not sure I understand why you think there's a 'chicken or the egg' problem. If a colleague adds a new feature to a submodule, if he updates the superproject for this and commits, then the behavior of `git fetch` won't make any difference. If he just pushes the submodule and leaves it up to you to update the superproject, then there's still no problem, as it cannot be committed into the superproject without you fetching the submodule. It seems the only reason to proactively fetch commits in a submodule that hasn't been updated in the superproject is so you can be aware that there were new commits pushed in the submodule, if you care to go look at them and potentially update the superproject to include the new commits. I am not convinced that this needs to be done on every `git fetch`. I
 t seems likely that most people running `git fetch` aren't responsible for updating the submodule, and often may explicitly not want to update the submodule. And whatever developer is responsible for updating the submodule when necessary should just be in the habit of running something like `git submodule foreach --recursive 'git fetch'`. Alternately, we could turn the submodule.<name>.fetch property into an enum rather than a boolean, and have one value of the enum be the current behavior (where it always fetches), another value be the new default behavior where it only fetches when necessary to resolve commit references in the superproject, and a third value be to never fetch. This mechanism would also benefit from having a config value that sets the default to use when submodule.<name
 >.fetch doesn't exist.

>> It also seems like there ought to be a config variable one can set for the default behavior if submodule.<name>.fetch is not present in .gitmodules or in .git/config.
> 
> I'll take this as a 'yes' to my question if such an option is wanted.
> 
> 
> In your other mail you wrote that the output of the recursive fetch
> does not prepend the names of the higher level submodules. I did not
> think about that and will post a fix for that problem soon.

As illustrated above, the recursive fetch actually isn't printing out any of the nested submodules, except for my outlier submodule H. If it's supposed to be printing them, it should prepend the submodule name with the parent. If it's not supposed to be printing them, then this isn't an issue. That said, I do think it should be printing them, because otherwise you get the weird behavior where you can see several fetches in a row, but without any "Fetching submodule <blah>" header between them. It makes it look like the same repo needed to be fetched several times and is quite confusing.

-Kevin Ballard