Skip to contentSkip to conversation

Teach fetch and pull to recursively fetch submodules too

RFC PATCH Superseded Jens Lehmann 9 participants 55 messages Jun 15, 2016 Documentation

v1 of 4 Superseded Latest is v4 →

lore
Share & tools
lore
#0 lore raw

cover Teach fetch and pull to recursively fetch submodules too

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
Signature · 1 line
1.7.2.2.527.gdf3084
#1 lore raw

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

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.

---
 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
Documentation/fetch-options.txt +5 -0
index 470ac31..1d875be 100644
@@ -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
builtin/fetch.c +10 -1
index fab3fce..da5fc9a 100644
@@ -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);
git-pull.sh +8 -2
index 8eb74d4..4bc8a60 100755
@@ -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)
submodule.c +42 -1

55 lines not on this page · raw message

submodule.h +2 -0
index 386f410..380878c 100644
@@ -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]);
t/t5526-fetch-submodules.sh new file +64 -0

64 lines not on this page · raw message

t/t7403-submodule-sync.sh +1 -1
index 02522f9..6f3e966 100755
@@ -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 \
Signature · 1 line
1.7.2.2.527.gdf3084
#2 lore raw

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

In reply to 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".

---
 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(-)
Documentation/config.txt +6 -0
index 0510ac7..048465f 100644
@@ -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
Documentation/fetch-options.txt +2 -1
index 1d875be..aff4daf 100644
@@ -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::
Documentation/gitmodules.txt +8 -0
index bcffd95..febfef4 100644
@@ -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
builtin/fetch.c +10 -4
index da5fc9a..17759b5 100644
@@ -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 */
submodule.c +18 -1
index e4f2419..8c98fad 100644
@@ -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);
submodule.h +1 -1
index 380878c..9e6257e 100644
@@ -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]);
t/t5526-fetch-submodules.sh +40 -0
index da5d5fd..489ef1a 100755
@@ -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 &&
Signature · 1 line
1.7.2.2.527.gdf3084
#3 lore raw

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

Quoted 5 lines · Jens Lehmann
submodule.c +0 -0
index e4f2419..8c98fad 100644

submodule.c int parse_submodule_config_option(const char *var, const char *value) · line 110

Quoted 10 lines
submodule.c +7 -0
@@ -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

#4 lore raw

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

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

---

Am 30.08.2010 09:34, schrieb Junio C Hamano:

Quoted 3 lines

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(-)
Documentation/config.txt +6 -0
index 0510ac7..048465f 100644
@@ -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
Documentation/fetch-options.txt +2 -1
index 1d875be..aff4daf 100644
@@ -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::
Documentation/gitmodules.txt +8 -0
index bcffd95..febfef4 100644
@@ -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
builtin/fetch.c +10 -4
index da5fc9a..17759b5 100644
@@ -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 */
submodule.c +18 -1
index e4f2419..2380638 100644
@@ -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);
submodule.h +1 -1
index 380878c..9e6257e 100644
@@ -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]);
t/t5526-fetch-submodules.sh +40 -0
index da5d5fd..489ef1a 100755
@@ -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 &&
Signature · 1 line
1.7.2.2.527.gacca9
#5 lore raw
In reply to Jens Lehmann
Quoted 9 lines · Jens Lehmann

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?

#6 lore raw

Am 30.08.2010 07:58, schrieb Junio C Hamano:

Quoted 4 lines

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.

#7 lore raw
In reply to Junio C Hamano
Quoted 7 lines · Junio C Hamano
Quoted 5 lines · Jens Lehmann

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.

Quoted 7 lines
Quoted 4 lines

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

#8 lore raw

Am 15.09.2010 02:18, schrieb Kevin Ballard:

Quoted 1 line

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

Quoted 1 line

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?

Quoted 1 line

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.

Quoted 1 line

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!

#9 lore raw
Quoted 7 lines · Jens Lehmann

Am 15.09.2010 02:18, schrieb Kevin Ballard:

Quoted 1 line

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.

Quoted 6 lines
Quoted 1 line

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.

Quoted 10 lines
Quoted 1 line

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

Quoted 10 lines

.fetch doesn't exist.

Quoted 1 line

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

#10 lore raw
In reply to 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.

Quoted 12 lines · Kevin Ballard
Quoted 10 lines · Junio C Hamano
Quoted 8 lines · Jens Lehmann

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?

Quoted 8 lines
Quoted 4 lines
Quoted 1 line

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.

Quoted 2 lines

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

#11 lore raw

PATCH 1/1 fetch: Get submodule paths from index and not from .gitmodules

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.

---

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(-)
submodule.c +19 -6
index 05661e2..13a694b 100644
@@ -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;
Signature · 1 line
1.7.3.rc2.232.g3328
#12 lore raw

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

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

Quoted 84 lines · 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.

---

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(-)
submodule.c +19 -6
index 05661e2..13a694b 100644
@@ -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;
Signature · 1 line
1.7.3.rc2.232.g3328
#13 lore raw

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

Am 16.09.2010 21:29, schrieb Kevin Ballard:

Quoted 2 lines

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:

submodule.c +2 -1
index e2c3bae..4fb1071 100644
@@ -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;
#14 lore raw

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

Am 9/17/2010 13:31, schrieb Jens Lehmann:

Quoted 10 lines

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:

submodule.c +0 -0
index e2c3bae..4fb1071 100644

submodule.c int fetch_populated_submodules(int forced) · line 264

Quoted 6 lines
submodule.c +2 -1
@@ -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

#15 lore raw

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

Am 17.09.2010 14:06, schrieb Johannes Sixt:

Quoted 6 lines

Am 9/17/2010 13:31, schrieb Jens Lehmann:

Quoted 5 lines

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:

Quoted 15 lines

submodule.c int fetch_populated_submodules(int forced) · line 264

Quoted 11 lines
submodule.c +2 -1
index e2c3bae..4fb1071 100644
@@ -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.

Quoted 4 lines

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:

submodule.c +0 -1
index e2c3bae..209efa4 100644
@@ -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;
#16 lore raw

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

Quoted 16 lines · Jens Lehmann
Quoted 4 lines

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:

submodule.c +0 -0
index e2c3bae..209efa4 100644

submodule.c int fetch_populated_submodules(int forced) · line 265

Quoted 7 lines
submodule.c +0 -1
@@ -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

#17 lore raw

cover fix problems with recursive submodule fetching

Am 18.09.2010 02:29, schrieb Kevin Ballard:

Quoted 1 line

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?

Quoted 1 line

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.

Quoted 1 line

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(-)
Signature · 1 line
1.7.3.rc2.234.g00c0
#18 lore raw

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

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.

---
 submodule.c                 |    1 -
 t/t5526-fetch-submodules.sh |   30 +++++++++++++++++++++++++-----
 2 files changed, 25 insertions(+), 6 deletions(-)
submodule.c +0 -1
index 2380638..a62cae5 100644
@@ -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;
t/t5526-fetch-submodules.sh +25 -5
index 489ef1a..cabf118 100755
@@ -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" '
Signature · 1 line
1.7.3.rc2.234.g00c0
#19 lore raw

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

In reply to 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.

---
 submodule.c |   25 +++++++++++++++++++------
 1 files changed, 19 insertions(+), 6 deletions(-)
submodule.c +19 -6
index a62cae5..209efa4 100644
@@ -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;
Signature · 1 line
1.7.3.rc2.234.g00c0
#20 lore raw

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

In reply to Jens Lehmann
Quoted 16 lines · Jens Lehmann
Quoted 1 line

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.

Quoted 3 lines

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

#21 lore raw

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

Am 19.09.2010 05:54, schrieb Kevin Ballard:

Quoted 7 lines

Here's the submodule layout:

.--Root-.
/ / /|\ \ \
A B C D E F G

    |
    H
Quoted 8 lines

/|\
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 ...

#22 lore raw

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

Quoted 15 lines · Jens Lehmann

Am 19.09.2010 05:54, schrieb Kevin Ballard:

Quoted 14 lines

Here's the submodule layout:

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

Quoted 5 lines
Quoted 1 line

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.

Quoted 3 lines

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

Quoted 3 lines

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

#23 lore raw

v2 cover Teach fetch and pull to recursively fetch submodules too

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
Signature · 1 line
1.7.3.1.108.gb6303
#24 lore raw

PATCH v2 1/3 fetch/pull: Recursively fetch populated submodules

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.

---
 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
Documentation/fetch-options.txt +14 -0
index 470ac31..4eb8c90 100644
@@ -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
builtin/fetch.c +37 -14

102 lines not on this page · raw message

git-pull.sh +8 -2
index 8eb74d4..4bc8a60 100755
@@ -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)
submodule.c +65 -1

78 lines not on this page · raw message

submodule.h +3 -0
index 386f410..b39d7a1 100644
@@ -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]);
t/t5526-fetch-submodules.sh new file +109 -0

109 lines not on this page · raw message

t/t7403-submodule-sync.sh +1 -1
index 02522f9..6f3e966 100755
@@ -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 \
Signature · 1 line
1.7.3.1.108.gb6303
#25 lore raw

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

In reply to 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".

---
 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(-)
Documentation/config.txt +6 -0
index 69d91fa..bc9b768 100644
@@ -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
Documentation/fetch-options.txt +2 -1
index 4eb8c90..f238e3c 100644
@@ -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::
Documentation/gitmodules.txt +8 -0
index bcffd95..febfef4 100644
@@ -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
builtin/fetch.c +10 -3
index d41f8cc..d20d022 100644
@@ -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);
 	}
 
submodule.c +19 -1
index 4d9b774..e4437b4 100644
@@ -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);
submodule.h +1 -1
index b39d7a1..08b422a 100644
@@ -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]);
t/t5526-fetch-submodules.sh +54 -0

64 lines not on this page · raw message

Signature · 1 line
1.7.3.1.108.gb6303
#26 lore raw

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

Quoted 6 lines

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.

#27 lore raw

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

Am 07.10.2010 15:33, schrieb Jon Seymour:

Quoted 12 lines
Quoted 6 lines

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

Quoted 4 lines

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.

#28 lore raw

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

Quoted 7 lines · Jens Lehmann

Am 07.10.2010 15:33, schrieb Jon Seymour:

Quoted 5 lines

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

Quoted 8 lines
Quoted 4 lines

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.

#29 lore raw

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

Am 09.10.2010 21:54, schrieb Jonathan Nieder:

Quoted 5 lines
Quoted 3 lines · Jens Lehmann

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.

Quoted 7 lines

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.

#30 lore raw

PATCH v2 3/3 Add the 'fetch.recursive' config setting

In reply to 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.

---
 Documentation/config.txt    |    4 +++
 submodule.c                 |    7 +++++
 t/t5526-fetch-submodules.sh |   56 +++++++++++++++++++++++++++++++++++++++----
 3 files changed, 62 insertions(+), 5 deletions(-)
Documentation/config.txt +4 -0
index bc9b768..6f259c1 100644
@@ -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
submodule.c +7 -0
index e4437b4..d4adb37 100644
@@ -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);
t/t5526-fetch-submodules.sh +51 -5

78 lines not on this page · raw message

Signature · 1 line
1.7.3.1.108.gb6303
#31 lore raw
In reply to Jens Lehmann
Quoted 6 lines · Jens Lehmann
   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.

Quoted 9 lines

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

#32 lore raw
In reply to Jens Lehmann
Quoted 11 lines · Jens Lehmann

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.

Quoted 10 lines · Junio C Hamano
Quoted 6 lines
  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.

Quoted 28 lines
Quoted 9 lines

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

Quoted 5 lines

[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

#33 lore raw
Quoted 3 lines · Kevin Ballard

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.

#34 lore raw

Am 07.10.2010 01:22, schrieb Jonathan Nieder:

Quoted 9 lines
Quoted 3 lines · Kevin Ballard

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

Quoted 8 lines
	[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.

Quoted 3 lines

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.

#35 lore raw
Quoted 8 lines · Jens Lehmann

Am 07.10.2010 01:22, schrieb Jonathan Nieder:

Quoted 3 lines

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.

Quoted 3 lines

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.

#36 lore raw

Am 09.10.2010 22:02, schrieb Jonathan Nieder:

Quoted 7 lines
Quoted 6 lines · Jens Lehmann

Am 07.10.2010 01:22, schrieb Jonathan Nieder:

Quoted 3 lines

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

Quoted 5 lines
Quoted 2 lines

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.

Quoted 7 lines
Quoted 3 lines

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?

#37 lore raw
Quoted 7 lines · Jens Lehmann

Am 09.10.2010 22:02, schrieb Jonathan Nieder:

Quoted 6 lines
Quoted 2 lines · Jens Lehmann

And configuring that via .gitmodules has the advantage
that every clone inherits that setting

I certainly do _not_ want that property.

[...]

Quoted 3 lines

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.

#38 lore raw

Am 21.10.2010 20:29, schrieb Jonathan Nieder:

Quoted 1 line

Hope that helps.

Yes, thanks a lot!

#39 lore raw
In reply to Kevin Ballard

Am 07.10.2010 00:52, schrieb Kevin Ballard:

Quoted 5 lines · Junio C Hamano
Quoted 2 lines

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

Quoted 7 lines

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

Quoted 5 lines

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.

#40 lore raw
Quoted 5 lines · Jens Lehmann

Am 07.10.2010 00:52, schrieb Kevin Ballard:

Quoted 4 lines · Junio C Hamano
Quoted 2 lines

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

Quoted 7 lines
Quoted 1 line

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.
#41 lore raw

Am 13.10.2010 16:48, schrieb Marc Branchaud:

Quoted 14 lines · Jens Lehmann
Quoted 5 lines

Am 07.10.2010 00:52, schrieb Kevin Ballard:

Quoted 4 lines · Junio C Hamano
Quoted 2 lines

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

Quoted 7 lines
Quoted 1 line

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

Quoted 3 lines

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.

Quoted 6 lines

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

Quoted 3 lines

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.

Quoted 5 lines

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!

#42 lore raw
Quoted 11 lines · Jens Lehmann
Quoted 9 lines
Quoted 5 lines

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

#43 lore raw

Am 13.10.2010 21:34, schrieb Kevin Ballard:

Quoted 8 lines · Jens Lehmann
Quoted 7 lines
Quoted 7 lines
Quoted 6 lines

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

Quoted 7 lines
Quoted 5 lines
Quoted 3 lines

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.

#44 lore raw
Quoted 13 lines · Jens Lehmann

Am 13.10.2010 21:34, schrieb Kevin Ballard:

Quoted 12 lines · Jens Lehmann
Quoted 11 lines
Quoted 10 lines
Quoted 6 lines

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.

Quoted 6 lines
Quoted 3 lines
Quoted 1 line

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

#45 lore raw
In reply to Jens Lehmann
Quoted 7 lines · Jens Lehmann

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

#46 lore raw

Am 14.10.2010 03:01, schrieb Chris Packham:

Quoted 5 lines

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?

Quoted 2 lines

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.

Quoted 1 line

Hope that is a useful use-case.

Sure, thanks!

#47 lore raw
Quoted 10 lines · Jens Lehmann

Am 14.10.2010 03:01, schrieb Chris Packham:

Quoted 5 lines

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.

Quoted 14 lines
Quoted 3 lines

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.

Quoted 1 line

Hope that is a useful use-case.

Sure, thanks!

#48 lore raw
In reply to Kevin Ballard
Quoted 13 lines · Kevin Ballard
Quoted 11 lines · Jens Lehmann
Quoted 10 lines
Quoted 5 lines

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.

Quoted 7 lines
Quoted 1 line

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.
#49 lore raw
Quoted 11 lines · Marc Branchaud
Quoted 5 lines

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.

Quoted 5 lines

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

#50 lore raw

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

In reply to Jens Lehmann

Am 9/17/2010 14:22, schrieb Jens Lehmann:

Quoted 44 lines

Am 17.09.2010 14:06, schrieb Johannes Sixt:

Quoted 6 lines

Am 9/17/2010 13:31, schrieb Jens Lehmann:

Quoted 5 lines

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:

Quoted 15 lines

submodule.c int fetch_populated_submodules(int forced) · line 264

Quoted 11 lines
submodule.c +2 -1
index e2c3bae..4fb1071 100644
@@ -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.

Quoted 4 lines

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:

submodule.c +0 -0
index e2c3bae..209efa4 100644

submodule.c int fetch_populated_submodules(int forced) · line 265

Quoted 7 lines
submodule.c +0 -1
@@ -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

#51 lore raw

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

Am 17.09.2010 14:32, schrieb Johannes Sixt:

Quoted 19 lines

Am 9/17/2010 14:22, schrieb Jens Lehmann:

Quoted 8 lines

So the correct fix should look like this:

submodule.c +0 -0
index e2c3bae..209efa4 100644

submodule.c int fetch_populated_submodules(int forced) · line 265

Quoted 7 lines
submodule.c +0 -1
@@ -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:

Quoted 8 lines
	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!

#52 lore raw

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

Am 9/17/2010 16:01, schrieb Jens Lehmann:

Quoted 4 lines

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

#53 lore raw
In reply to Jens Lehmann
Quoted 1 line · Jens Lehmann

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.

#54 lore raw

Am 29.08.2010 19:29, schrieb Ævar Arnfjörð Bjarmason:

Quoted 3 lines

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