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

DORMANTno replies

5 messages, 2 authors, 2016-06-15 · open the first message on its own page

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

From: Jens Lehmann <hidden>
Date: 2016-06-15 22:49:24

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

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

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

Opinions?


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

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

-- 
1.7.2.2.527.gdf3084

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

From: Jens Lehmann <hidden>
Date: 2016-06-15 22:49:24

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

This could lead to "(commits not present)" messages in the output of "git
diff --submodule" (and in "git gui" and "gitk") after fetching/pulling new
commits in the superproject and is an obstacle for implementing recursive
checkout of submodules.

This patch recursively fetches each populated submodule from the url
configured in the .git/config of the submodule at the end of each "git
fetch" or during "git pull" in the superproject. This new behavior can be
disabled by using the new --no-recursive option.

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

Signed-off-by: Jens Lehmann <redacted>
---
 Documentation/fetch-options.txt |    5 +++
 builtin/fetch.c                 |   11 ++++++-
 git-pull.sh                     |   10 +++++-
 submodule.c                     |   43 +++++++++++++++++++++++++-
 submodule.h                     |    2 +
 t/t5526-fetch-submodules.sh     |   64 +++++++++++++++++++++++++++++++++++++++
 t/t7403-submodule-sync.sh       |    2 +-
 7 files changed, 132 insertions(+), 5 deletions(-)
 create mode 100755 t/t5526-fetch-submodules.sh
diff --git a/Documentation/fetch-options.txt b/Documentation/fetch-options.txt
index 470ac31..1d875be 100644
--- a/Documentation/fetch-options.txt
+++ b/Documentation/fetch-options.txt
@@ -64,6 +64,11 @@ endif::git-pull[]
 	specified with the remote.<name>.tagopt setting. See
 	linkgit:git-config[1].

+--[no-]recursive::
+	By default new commits of all populated submodules will be fetched
+	too. This option can be used to disable/enable recursive fetching of
+	submodules.
+
 -u::
 --update-head-ok::
 	By default 'git fetch' refuses to update the head which
diff --git a/builtin/fetch.c b/builtin/fetch.c
index fab3fce..da5fc9a 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -12,6 +12,7 @@
 #include "parse-options.h"
 #include "sigchain.h"
 #include "transport.h"
+#include "submodule.h"

 static const char * const builtin_fetch_usage[] = {
 	"git fetch [<options>] [<repository> [<refspec>...]]",
@@ -27,7 +28,7 @@ enum {
 	TAGS_SET = 2
 };

-static int all, append, dry_run, force, keep, multiple, prune, update_head_ok, verbosity;
+static int all, append, dry_run, force, keep, multiple, prune, recursive = -1, update_head_ok, verbosity;
 static int progress;
 static int tags = TAGS_DEFAULT;
 static const char *depth;
@@ -53,6 +54,8 @@ static struct option builtin_fetch_options[] = {
 		    "do not fetch all tags (--no-tags)", TAGS_UNSET),
 	OPT_BOOLEAN('p', "prune", &prune,
 		    "prune tracking branches no longer on remote"),
+	OPT_BOOLEAN(0, "recursive", &recursive,
+		    "control recursive fetching of submodules"),
 	OPT_BOOLEAN(0, "dry-run", &dry_run,
 		    "dry run"),
 	OPT_BOOLEAN('k', "keep", &keep, "keep downloaded pack"),
@@ -921,6 +924,12 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)
 		}
 	}

+	if (!result && recursive) {
+		gitmodules_config();
+		git_config(submodule_config, NULL);
+		result = fetch_populated_submodules();
+	}
+
 	/* All names were strdup()ed or strndup()ed */
 	list.strdup_strings = 1;
 	string_list_clear(&list, 0);
diff --git a/git-pull.sh b/git-pull.sh
index 8eb74d4..4bc8a60 100755
--- a/git-pull.sh
+++ b/git-pull.sh
@@ -38,7 +38,7 @@ test -z "$(git ls-files -u)" || die_conflict
 test -f "$GIT_DIR/MERGE_HEAD" && die_merge

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

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

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

+int fetch_populated_submodules()
+{
+	int result = 0;
+	struct child_process cp;
+	const char *argv[] = {
+		"fetch",
+		NULL,
+	};
+	struct string_list_item *name_for_path;
+	const char *work_tree = get_git_work_tree();
+	if (!work_tree)
+		return 0;
+
+	memset(&cp, 0, sizeof(cp));
+	cp.argv = argv;
+	cp.env = local_repo_env;
+	cp.git_cmd = 1;
+	cp.no_stdin = 1;
+	cp.out = -1;
+
+	for_each_string_list_item(name_for_path, &config_name_for_path) {
+		struct strbuf submodule_path = STRBUF_INIT;
+		struct strbuf submodule_git_dir = STRBUF_INIT;
+		const char *git_dir;
+		strbuf_addf(&submodule_path, "%s/%s", work_tree, name_for_path->string);
+		strbuf_addf(&submodule_git_dir, "%s/.git", submodule_path.buf);
+		git_dir = read_gitfile_gently(submodule_git_dir.buf);
+		if (!git_dir)
+			git_dir = submodule_git_dir.buf;
+		if (is_directory(git_dir)) {
+			printf("Fetching submodule %s\n", name_for_path->string);
+			cp.dir = submodule_path.buf;
+			if (run_command(&cp))
+				result = 1;
+		}
+		strbuf_release(&submodule_path);
+		strbuf_release(&submodule_git_dir);
+	}
+	return result;
+}
+
 unsigned is_submodule_modified(const char *path, int ignore_untracked)
 {
 	ssize_t len;
diff --git a/submodule.h b/submodule.h
index 386f410..380878c 100644
--- a/submodule.h
+++ b/submodule.h
@@ -5,6 +5,7 @@ struct diff_options;

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

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

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

From: Jens Lehmann <hidden>
Date: 2016-06-15 22:49:24

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

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

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

Signed-off-by: Jens Lehmann <redacted>
---
 Documentation/config.txt        |    6 +++++
 Documentation/fetch-options.txt |    3 +-
 Documentation/gitmodules.txt    |    8 +++++++
 builtin/fetch.c                 |   14 +++++++++---
 submodule.c                     |   19 +++++++++++++++++-
 submodule.h                     |    2 +-
 t/t5526-fetch-submodules.sh     |   40 +++++++++++++++++++++++++++++++++++++++
 7 files changed, 85 insertions(+), 7 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 0510ac7..048465f 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1769,6 +1769,12 @@ submodule.<name>.update::
 	URL and other values found in the `.gitmodules` file.  See
 	linkgit:git-submodule[1] and linkgit:gitmodules[5] for details.

+submodule.<name>.fetch::
+	A boolean to enable/disable recursive fetching of this submodule. It can
+	be overriden by using the --[no-]recursive command line option to "git
+	fetch" and "git pull".	This setting overrides any setting made in
+	.gitmodules for this submodule.
+
 submodule.<name>.ignore::
 	Defines under what circumstances "git status" and the diff family show
 	a submodule as modified. When set to "all", it will never be considered
diff --git a/Documentation/fetch-options.txt b/Documentation/fetch-options.txt
index 1d875be..aff4daf 100644
--- a/Documentation/fetch-options.txt
+++ b/Documentation/fetch-options.txt
@@ -67,7 +67,8 @@ endif::git-pull[]
 --[no-]recursive::
 	By default new commits of all populated submodules will be fetched
 	too. This option can be used to disable/enable recursive fetching of
-	submodules.
+	submodules regardless of the 'fetch' configuration setting (see
+	linkgit:git-config[1] or linkgit:gitmodules[5]).

 -u::
 --update-head-ok::
diff --git a/Documentation/gitmodules.txt b/Documentation/gitmodules.txt
index bcffd95..febfef4 100644
--- a/Documentation/gitmodules.txt
+++ b/Documentation/gitmodules.txt
@@ -44,6 +44,14 @@ submodule.<name>.update::
 	This config option is overridden if 'git submodule update' is given
 	the '--merge' or '--rebase' options.

+submodule.<name>.fetch::
+	A boolean to enable/disable recursive fetching of this submodule.
+	If this option is also present in the submodules entry in .git/config of
+	the superproject, the setting there will override the one found in
+	.gitmodules.
+	Both settings can be overriden on the command line by using the
+	"--[no-]recursive" option to "git fetch" and "git pull"..
+
 submodule.<name>.ignore::
 	Defines under what circumstances "git status" and the diff family show
 	a submodule as modified. When set to "all", it will never be considered
diff --git a/builtin/fetch.c b/builtin/fetch.c
index da5fc9a..17759b5 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -28,7 +28,13 @@ enum {
 	TAGS_SET = 2
 };

-static int all, append, dry_run, force, keep, multiple, prune, recursive = -1, update_head_ok, verbosity;
+enum {
+	RECURSIVE_UNSET = 0,
+	RECURSIVE_DEFAULT = 1,
+	RECURSIVE_SET = 2
+};
+
+static int all, append, dry_run, force, keep, multiple, prune, recursive = RECURSIVE_DEFAULT, update_head_ok, verbosity;
 static int progress;
 static int tags = TAGS_DEFAULT;
 static const char *depth;
@@ -54,8 +60,8 @@ static struct option builtin_fetch_options[] = {
 		    "do not fetch all tags (--no-tags)", TAGS_UNSET),
 	OPT_BOOLEAN('p', "prune", &prune,
 		    "prune tracking branches no longer on remote"),
-	OPT_BOOLEAN(0, "recursive", &recursive,
-		    "control recursive fetching of submodules"),
+	OPT_SET_INT(0, "recursive", &recursive,
+		    "control recursive fetching of submodules", RECURSIVE_SET),
 	OPT_BOOLEAN(0, "dry-run", &dry_run,
 		    "dry run"),
 	OPT_BOOLEAN('k', "keep", &keep, "keep downloaded pack"),
@@ -927,7 +933,7 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)
 	if (!result && recursive) {
 		gitmodules_config();
 		git_config(submodule_config, NULL);
-		result = fetch_populated_submodules();
+		result = fetch_populated_submodules(recursive == RECURSIVE_SET);
 	}

 	/* All names were strdup()ed or strndup()ed */
diff --git a/submodule.c b/submodule.c
index e4f2419..8c98fad 100644
--- a/submodule.c
+++ b/submodule.c
@@ -10,6 +10,7 @@
 #include "string-list.h"

 struct string_list config_name_for_path;
+struct string_list config_fetch_for_name;
 struct string_list config_ignore_for_name;

 static int add_submodule_odb(const char *path)
@@ -100,6 +101,14 @@ int parse_submodule_config_option(const char *var, const char *value)
 			config = string_list_append(&config_name_for_path, xstrdup(value));
 		config->util = strbuf_detach(&submodname, NULL);
 		strbuf_release(&submodname);
+	} else if ((len > 5) && !strcmp(var + len - 6, ".fetch")) {
+		strbuf_add(&submodname, var, len - 6);
+		config = unsorted_string_list_lookup(&config_fetch_for_name, submodname.buf);
+		if (!config)
+			config = string_list_append(&config_fetch_for_name,
+						    strbuf_detach(&submodname, NULL));
+		config->util = (void *)git_config_bool(var, value);
+		strbuf_release(&submodname);
 	} else if ((len > 7) && !strcmp(var + len - 7, ".ignore")) {
 		if (strcmp(value, "untracked") && strcmp(value, "dirty") &&
 		    strcmp(value, "all") && strcmp(value, "none")) {
@@ -229,7 +238,7 @@ void show_submodule_summary(FILE *f, const char *path,
 	strbuf_release(&sb);
 }

-int fetch_populated_submodules()
+int fetch_populated_submodules(int forced)
 {
 	int result = 0;
 	struct child_process cp;
@@ -253,6 +262,14 @@ int fetch_populated_submodules()
 		struct strbuf submodule_path = STRBUF_INIT;
 		struct strbuf submodule_git_dir = STRBUF_INIT;
 		const char *git_dir;
+
+		if (!forced) {
+			struct string_list_item *fetch_option;
+			fetch_option = unsorted_string_list_lookup(&config_fetch_for_name, name_for_path->util);
+			if (fetch_option && !fetch_option->util)
+				continue;
+		}
+
 		strbuf_addf(&submodule_path, "%s/%s", work_tree, name_for_path->string);
 		strbuf_addf(&submodule_git_dir, "%s/.git", submodule_path.buf);
 		git_dir = read_gitfile_gently(submodule_git_dir.buf);
diff --git a/submodule.h b/submodule.h
index 380878c..9e6257e 100644
--- a/submodule.h
+++ b/submodule.h
@@ -13,7 +13,7 @@ void show_submodule_summary(FILE *f, const char *path,
 		unsigned char one[20], unsigned char two[20],
 		unsigned dirty_submodule,
 		const char *del, const char *add, const char *reset);
-int fetch_populated_submodules();
+int fetch_populated_submodules(int forced);
 unsigned is_submodule_modified(const char *path, int ignore_untracked);
 int merge_submodule(unsigned char result[20], const char *path, const unsigned char base[20],
 		    const unsigned char a[20], const unsigned char b[20]);
diff --git a/t/t5526-fetch-submodules.sh b/t/t5526-fetch-submodules.sh
index da5d5fd..489ef1a 100755
--- a/t/t5526-fetch-submodules.sh
+++ b/t/t5526-fetch-submodules.sh
@@ -52,6 +52,46 @@ test_expect_success "fetch recurses into submodules" '
 '

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

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

From: Ævar Arnfjörð Bjarmason <hidden>
Date: 2016-06-15 22:49:24

On Sun, Aug 29, 2010 at 15:49, Jens Lehmann [off-list ref] wrote:
Opinions?
I'm not familiar with this area, but I couldn't see anything wrong
with it. However I wonder if we're going to make --recursive the
default for pull/fetch whether it shouldn't be made the default for
clone while we're at it.

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

From: Jens Lehmann <hidden>
Date: 2016-06-15 22:49:24

Am 29.08.2010 19:29, schrieb Ævar Arnfjörð Bjarmason:
However I wonder if we're going to make --recursive the
default for pull/fetch whether it shouldn't be made the default for
clone while we're at it.
Hm, while that would work for me (and for you obviously ;-), it
wouldn't make sense for other use cases where submodules are used
as a kind of sparse checkout (and then some of them are never meant
to be checked out, at least by some users).

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

After finishing the recursive checkout of populated submodules
one of my next goals is to teach clone to recurse too, but only
for those submodules where this is wanted (aka configured via
.gitmodules).
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help