From: Junio C Hamano <gitster@pobox.com>
Date: Mon, 29 May 2017 03:34:41 +0000 (+0900)
Subject: Merge branch 'sb/checkout-recurse-submodules'
X-Git-Tag: v2.14.0-rc0~170
X-Git-Url: https://git.lorimer.id.au/gitweb.git/diff_plain/f1101cefbd32f7ab8250285f122f132d31ff5335?ds=inline;hp=-c

Merge branch 'sb/checkout-recurse-submodules'

"git checkout --recurse-submodules" did not quite work with a
submodule that itself has submodules.

* sb/checkout-recurse-submodules:
submodule: properly recurse for read-tree and checkout
submodule: avoid auto-discovery in new working tree manipulator code
submodule_move_head: reuse child_process structure for futher commands
---

f1101cefbd32f7ab8250285f122f132d31ff5335
diff --combined submodule.c
index 54825100b2,b3ae642f29..268f25fc2c
--- a/submodule.c
+++ b/submodule.c
@@@ -20,7 -20,7 +20,7 @@@
  static int config_fetch_recurse_submodules = RECURSE_SUBMODULES_ON_DEMAND;
  static int config_update_recurse_submodules = RECURSE_SUBMODULES_DEFAULT;
  static int parallel_jobs = 1;
 -static struct string_list changed_submodule_paths = STRING_LIST_INIT_NODUP;
 +static struct string_list changed_submodule_paths = STRING_LIST_INIT_DUP;
  static int initialized_fetch_ref_tips;
  static struct oid_array ref_tips_before_fetch;
  static struct oid_array ref_tips_after_fetch;
@@@ -617,94 -617,6 +617,94 @@@ const struct submodule *submodule_from_
  	return submodule_from_path(null_sha1, ce->name);
  }
  
 +static struct oid_array *submodule_commits(struct string_list *submodules,
 +					   const char *path)
 +{
 +	struct string_list_item *item;
 +
 +	item = string_list_insert(submodules, path);
 +	if (item->util)
 +		return (struct oid_array *) item->util;
 +
 +	/* NEEDSWORK: should we have oid_array_init()? */
 +	item->util = xcalloc(1, sizeof(struct oid_array));
 +	return (struct oid_array *) item->util;
 +}
 +
 +static void collect_changed_submodules_cb(struct diff_queue_struct *q,
 +					  struct diff_options *options,
 +					  void *data)
 +{
 +	int i;
 +	struct string_list *changed = data;
 +
 +	for (i = 0; i < q->nr; i++) {
 +		struct diff_filepair *p = q->queue[i];
 +		struct oid_array *commits;
 +		if (!S_ISGITLINK(p->two->mode))
 +			continue;
 +
 +		if (S_ISGITLINK(p->one->mode)) {
 +			/*
 +			 * NEEDSWORK: We should honor the name configured in
 +			 * the .gitmodules file of the commit we are examining
 +			 * here to be able to correctly follow submodules
 +			 * being moved around.
 +			 */
 +			commits = submodule_commits(changed, p->two->path);
 +			oid_array_append(commits, &p->two->oid);
 +		} else {
 +			/* Submodule is new or was moved here */
 +			/*
 +			 * NEEDSWORK: When the .git directories of submodules
 +			 * live inside the superprojects .git directory some
 +			 * day we should fetch new submodules directly into
 +			 * that location too when config or options request
 +			 * that so they can be checked out from there.
 +			 */
 +			continue;
 +		}
 +	}
 +}
 +
 +/*
 + * Collect the paths of submodules in 'changed' which have changed based on
 + * the revisions as specified in 'argv'.  Each entry in 'changed' will also
 + * have a corresponding 'struct oid_array' (in the 'util' field) which lists
 + * what the submodule pointers were updated to during the change.
 + */
 +static void collect_changed_submodules(struct string_list *changed,
 +				       struct argv_array *argv)
 +{
 +	struct rev_info rev;
 +	const struct commit *commit;
 +
 +	init_revisions(&rev, NULL);
 +	setup_revisions(argv->argc, argv->argv, &rev, NULL);
 +	if (prepare_revision_walk(&rev))
 +		die("revision walk setup failed");
 +
 +	while ((commit = get_revision(&rev))) {
 +		struct rev_info diff_rev;
 +
 +		init_revisions(&diff_rev, NULL);
 +		diff_rev.diffopt.output_format |= DIFF_FORMAT_CALLBACK;
 +		diff_rev.diffopt.format_callback = collect_changed_submodules_cb;
 +		diff_rev.diffopt.format_callback_data = changed;
 +		diff_tree_combined_merge(commit, 1, &diff_rev);
 +	}
 +
 +	reset_revision_walk();
 +}
 +
 +static void free_submodules_oids(struct string_list *submodules)
 +{
 +	struct string_list_item *item;
 +	for_each_string_list_item(item, submodules)
 +		oid_array_clear((struct oid_array *) item->util);
 +	string_list_clear(submodules, 1);
 +}
 +
  static int has_remote(const char *refname, const struct object_id *oid,
  		      int flags, void *cb_data)
  {
@@@ -732,44 -644,10 +732,44 @@@ static int submodule_has_commits(const 
  {
  	int has_commit = 1;
  
 +	/*
 +	 * Perform a cheap, but incorrect check for the existance of 'commits'.
 +	 * This is done by adding the submodule's object store to the in-core
 +	 * object store, and then querying for each commit's existance.  If we
 +	 * do not have the commit object anywhere, there is no chance we have
 +	 * it in the object store of the correct submodule and have it
 +	 * reachable from a ref, so we can fail early without spawning rev-list
 +	 * which is expensive.
 +	 */
  	if (add_submodule_odb(path))
  		return 0;
  
  	oid_array_for_each_unique(commits, check_has_commit, &has_commit);
 +
 +	if (has_commit) {
 +		/*
 +		 * Even if the submodule is checked out and the commit is
 +		 * present, make sure it exists in the submodule's object store
 +		 * and that it is reachable from a ref.
 +		 */
 +		struct child_process cp = CHILD_PROCESS_INIT;
 +		struct strbuf out = STRBUF_INIT;
 +
 +		argv_array_pushl(&cp.args, "rev-list", "-n", "1", NULL);
 +		oid_array_for_each_unique(commits, append_oid_to_argv, &cp.args);
 +		argv_array_pushl(&cp.args, "--not", "--all", NULL);
 +
 +		prepare_submodule_repo_env(&cp.env_array);
 +		cp.git_cmd = 1;
 +		cp.no_stdin = 1;
 +		cp.dir = path;
 +
 +		if (capture_command(&cp, &out, GIT_MAX_HEXSZ + 1) || out.len)
 +			has_commit = 0;
 +
 +		strbuf_release(&out);
 +	}
 +
  	return has_commit;
  }
  
@@@ -817,31 -695,91 +817,31 @@@ static int submodule_needs_pushing(cons
  	return 0;
  }
  
 -static struct oid_array *submodule_commits(struct string_list *submodules,
 -					    const char *path)
 -{
 -	struct string_list_item *item;
 -
 -	item = string_list_insert(submodules, path);
 -	if (item->util)
 -		return (struct oid_array *) item->util;
 -
 -	/* NEEDSWORK: should we have oid_array_init()? */
 -	item->util = xcalloc(1, sizeof(struct oid_array));
 -	return (struct oid_array *) item->util;
 -}
 -
 -static void collect_submodules_from_diff(struct diff_queue_struct *q,
 -					 struct diff_options *options,
 -					 void *data)
 -{
 -	int i;
 -	struct string_list *submodules = data;
 -
 -	for (i = 0; i < q->nr; i++) {
 -		struct diff_filepair *p = q->queue[i];
 -		struct oid_array *commits;
 -		if (!S_ISGITLINK(p->two->mode))
 -			continue;
 -		commits = submodule_commits(submodules, p->two->path);
 -		oid_array_append(commits, &p->two->oid);
 -	}
 -}
 -
 -static void find_unpushed_submodule_commits(struct commit *commit,
 -		struct string_list *needs_pushing)
 -{
 -	struct rev_info rev;
 -
 -	init_revisions(&rev, NULL);
 -	rev.diffopt.output_format |= DIFF_FORMAT_CALLBACK;
 -	rev.diffopt.format_callback = collect_submodules_from_diff;
 -	rev.diffopt.format_callback_data = needs_pushing;
 -	diff_tree_combined_merge(commit, 1, &rev);
 -}
 -
 -static void free_submodules_sha1s(struct string_list *submodules)
 -{
 -	struct string_list_item *item;
 -	for_each_string_list_item(item, submodules)
 -		oid_array_clear((struct oid_array *) item->util);
 -	string_list_clear(submodules, 1);
 -}
 -
  int find_unpushed_submodules(struct oid_array *commits,
  		const char *remotes_name, struct string_list *needs_pushing)
  {
 -	struct rev_info rev;
 -	struct commit *commit;
  	struct string_list submodules = STRING_LIST_INIT_DUP;
  	struct string_list_item *submodule;
  	struct argv_array argv = ARGV_ARRAY_INIT;
  
 -	init_revisions(&rev, NULL);
 -
  	/* argv.argv[0] will be ignored by setup_revisions */
  	argv_array_push(&argv, "find_unpushed_submodules");
  	oid_array_for_each_unique(commits, append_oid_to_argv, &argv);
  	argv_array_push(&argv, "--not");
  	argv_array_pushf(&argv, "--remotes=%s", remotes_name);
  
 -	setup_revisions(argv.argc, argv.argv, &rev, NULL);
 -	if (prepare_revision_walk(&rev))
 -		die("revision walk setup failed");
 -
 -	while ((commit = get_revision(&rev)) != NULL)
 -		find_unpushed_submodule_commits(commit, &submodules);
 -
 -	reset_revision_walk();
 -	argv_array_clear(&argv);
 +	collect_changed_submodules(&submodules, &argv);
  
  	for_each_string_list_item(submodule, &submodules) {
 -		struct oid_array *commits = (struct oid_array *) submodule->util;
 +		struct oid_array *commits = submodule->util;
 +		const char *path = submodule->string;
  
 -		if (submodule_needs_pushing(submodule->string, commits))
 -			string_list_insert(needs_pushing, submodule->string);
 +		if (submodule_needs_pushing(path, commits))
 +			string_list_insert(needs_pushing, path);
  	}
 -	free_submodules_sha1s(&submodules);
 +
 +	free_submodules_oids(&submodules);
 +	argv_array_clear(&argv);
  
  	return needs_pushing->nr;
  }
@@@ -958,56 -896,125 +958,56 @@@ int push_unpushed_submodules(struct oid
  	return ret;
  }
  
 -static int is_submodule_commit_present(const char *path, unsigned char sha1[20])
 +static int append_oid_to_array(const char *ref, const struct object_id *oid,
 +			       int flags, void *data)
  {
 -	int is_present = 0;
 -	if (!add_submodule_odb(path) && lookup_commit_reference(sha1)) {
 -		/* Even if the submodule is checked out and the commit is
 -		 * present, make sure it is reachable from a ref. */
 -		struct child_process cp = CHILD_PROCESS_INIT;
 -		const char *argv[] = {"rev-list", "-n", "1", NULL, "--not", "--all", NULL};
 -		struct strbuf buf = STRBUF_INIT;
 -
 -		argv[3] = sha1_to_hex(sha1);
 -		cp.argv = argv;
 -		prepare_submodule_repo_env(&cp.env_array);
 -		cp.git_cmd = 1;
 -		cp.no_stdin = 1;
 -		cp.dir = path;
 -		if (!capture_command(&cp, &buf, 1024) && !buf.len)
 -			is_present = 1;
 -
 -		strbuf_release(&buf);
 -	}
 -	return is_present;
 -}
 -
 -static void submodule_collect_changed_cb(struct diff_queue_struct *q,
 -					 struct diff_options *options,
 -					 void *data)
 -{
 -	int i;
 -	for (i = 0; i < q->nr; i++) {
 -		struct diff_filepair *p = q->queue[i];
 -		if (!S_ISGITLINK(p->two->mode))
 -			continue;
 -
 -		if (S_ISGITLINK(p->one->mode)) {
 -			/* NEEDSWORK: We should honor the name configured in
 -			 * the .gitmodules file of the commit we are examining
 -			 * here to be able to correctly follow submodules
 -			 * being moved around. */
 -			struct string_list_item *path;
 -			path = unsorted_string_list_lookup(&changed_submodule_paths, p->two->path);
 -			if (!path && !is_submodule_commit_present(p->two->path, p->two->oid.hash))
 -				string_list_append(&changed_submodule_paths, xstrdup(p->two->path));
 -		} else {
 -			/* Submodule is new or was moved here */
 -			/* NEEDSWORK: When the .git directories of submodules
 -			 * live inside the superprojects .git directory some
 -			 * day we should fetch new submodules directly into
 -			 * that location too when config or options request
 -			 * that so they can be checked out from there. */
 -			continue;
 -		}
 -	}
 -}
 -
 -static int add_sha1_to_array(const char *ref, const struct object_id *oid,
 -			     int flags, void *data)
 -{
 -	oid_array_append(data, oid);
 +	struct oid_array *array = data;
 +	oid_array_append(array, oid);
  	return 0;
  }
  
  void check_for_new_submodule_commits(struct object_id *oid)
  {
  	if (!initialized_fetch_ref_tips) {
 -		for_each_ref(add_sha1_to_array, &ref_tips_before_fetch);
 +		for_each_ref(append_oid_to_array, &ref_tips_before_fetch);
  		initialized_fetch_ref_tips = 1;
  	}
  
  	oid_array_append(&ref_tips_after_fetch, oid);
  }
  
 -static int add_oid_to_argv(const struct object_id *oid, void *data)
 -{
 -	argv_array_push(data, oid_to_hex(oid));
 -	return 0;
 -}
 -
  static void calculate_changed_submodule_paths(void)
  {
 -	struct rev_info rev;
 -	struct commit *commit;
  	struct argv_array argv = ARGV_ARRAY_INIT;
 +	struct string_list changed_submodules = STRING_LIST_INIT_DUP;
 +	const struct string_list_item *item;
  
  	/* No need to check if there are no submodules configured */
  	if (!submodule_from_path(NULL, NULL))
  		return;
  
 -	init_revisions(&rev, NULL);
  	argv_array_push(&argv, "--"); /* argv[0] program name */
  	oid_array_for_each_unique(&ref_tips_after_fetch,
 -				   add_oid_to_argv, &argv);
 +				   append_oid_to_argv, &argv);
  	argv_array_push(&argv, "--not");
  	oid_array_for_each_unique(&ref_tips_before_fetch,
 -				   add_oid_to_argv, &argv);
 -	setup_revisions(argv.argc, argv.argv, &rev, NULL);
 -	if (prepare_revision_walk(&rev))
 -		die("revision walk setup failed");
 +				   append_oid_to_argv, &argv);
  
  	/*
  	 * Collect all submodules (whether checked out or not) for which new
  	 * commits have been recorded upstream in "changed_submodule_paths".
  	 */
 -	while ((commit = get_revision(&rev))) {
 -		struct commit_list *parent = commit->parents;
 -		while (parent) {
 -			struct diff_options diff_opts;
 -			diff_setup(&diff_opts);
 -			DIFF_OPT_SET(&diff_opts, RECURSIVE);
 -			diff_opts.output_format |= DIFF_FORMAT_CALLBACK;
 -			diff_opts.format_callback = submodule_collect_changed_cb;
 -			diff_setup_done(&diff_opts);
 -			diff_tree_sha1(parent->item->object.oid.hash, commit->object.oid.hash, "", &diff_opts);
 -			diffcore_std(&diff_opts);
 -			diff_flush(&diff_opts);
 -			parent = parent->next;
 -		}
 +	collect_changed_submodules(&changed_submodules, &argv);
 +
 +	for_each_string_list_item(item, &changed_submodules) {
 +		struct oid_array *commits = item->util;
 +		const char *path = item->string;
 +
 +		if (!submodule_has_commits(path, commits))
 +			string_list_append(&changed_submodule_paths, path);
  	}
  
 +	free_submodules_oids(&changed_submodules);
  	argv_array_clear(&argv);
  	oid_array_clear(&ref_tips_before_fetch);
  	oid_array_clear(&ref_tips_after_fetch);
@@@ -1356,7 -1363,7 +1356,7 @@@ static int submodule_has_dirty_index(co
  {
  	struct child_process cp = CHILD_PROCESS_INIT;
  
- 	prepare_submodule_repo_env_no_git_dir(&cp.env_array);
+ 	prepare_submodule_repo_env(&cp.env_array);
  
  	cp.git_cmd = 1;
  	argv_array_pushl(&cp.args, "diff-index", "--quiet",
@@@ -1373,7 -1380,7 +1373,7 @@@
  static void submodule_reset_index(const char *path)
  {
  	struct child_process cp = CHILD_PROCESS_INIT;
- 	prepare_submodule_repo_env_no_git_dir(&cp.env_array);
+ 	prepare_submodule_repo_env(&cp.env_array);
  
  	cp.git_cmd = 1;
  	cp.no_stdin = 1;
@@@ -1402,23 -1409,6 +1402,23 @@@ int submodule_move_head(const char *pat
  	int ret = 0;
  	struct child_process cp = CHILD_PROCESS_INIT;
  	const struct submodule *sub;
 +	int *error_code_ptr, error_code;
 +
 +	if (!is_submodule_initialized(path))
 +		return 0;
 +
 +	if (flags & SUBMODULE_MOVE_HEAD_FORCE)
 +		/*
 +		 * Pass non NULL pointer to is_submodule_populated_gently
 +		 * to prevent die()-ing. We'll use connect_work_tree_and_git_dir
 +		 * to fixup the submodule in the force case later.
 +		 */
 +		error_code_ptr = &error_code;
 +	else
 +		error_code_ptr = NULL;
 +
 +	if (old && !is_submodule_populated_gently(path, error_code_ptr))
 +		return 0;
  
  	sub = submodule_from_path(null_sha1, path);
  
@@@ -1437,24 -1427,18 +1437,24 @@@
  				absorb_git_dir_into_superproject("", path,
  					ABSORB_GITDIR_RECURSE_SUBMODULES);
  		} else {
 -			struct strbuf sb = STRBUF_INIT;
 -			strbuf_addf(&sb, "%s/modules/%s",
 +			char *gitdir = xstrfmt("%s/modules/%s",
  				    get_git_common_dir(), sub->name);
 -			connect_work_tree_and_git_dir(path, sb.buf);
 -			strbuf_release(&sb);
 +			connect_work_tree_and_git_dir(path, gitdir);
 +			free(gitdir);
  
  			/* make sure the index is clean as well */
  			submodule_reset_index(path);
  		}
 +
 +		if (old && (flags & SUBMODULE_MOVE_HEAD_FORCE)) {
 +			char *gitdir = xstrfmt("%s/modules/%s",
 +				    get_git_common_dir(), sub->name);
 +			connect_work_tree_and_git_dir(path, gitdir);
 +			free(gitdir);
 +		}
  	}
  
- 	prepare_submodule_repo_env_no_git_dir(&cp.env_array);
+ 	prepare_submodule_repo_env(&cp.env_array);
  
  	cp.git_cmd = 1;
  	cp.no_stdin = 1;
@@@ -1462,7 -1446,7 +1462,7 @@@
  
  	argv_array_pushf(&cp.args, "--super-prefix=%s%s/",
  			get_super_prefix_or_empty(), path);
- 	argv_array_pushl(&cp.args, "read-tree", NULL);
+ 	argv_array_pushl(&cp.args, "read-tree", "--recurse-submodules", NULL);
  
  	if (flags & SUBMODULE_MOVE_HEAD_DRY_RUN)
  		argv_array_push(&cp.args, "-n");
@@@ -1484,15 -1468,16 +1484,16 @@@
  
  	if (!(flags & SUBMODULE_MOVE_HEAD_DRY_RUN)) {
  		if (new) {
- 			struct child_process cp1 = CHILD_PROCESS_INIT;
+ 			child_process_init(&cp);
  			/* also set the HEAD accordingly */
- 			cp1.git_cmd = 1;
- 			cp1.no_stdin = 1;
- 			cp1.dir = path;
+ 			cp.git_cmd = 1;
+ 			cp.no_stdin = 1;
+ 			cp.dir = path;
  
- 			argv_array_pushl(&cp1.args, "update-ref", "HEAD", new, NULL);
+ 			prepare_submodule_repo_env(&cp.env_array);
+ 			argv_array_pushl(&cp.args, "update-ref", "HEAD", new, NULL);
  
- 			if (run_command(&cp1)) {
+ 			if (run_command(&cp)) {
  				ret = -1;
  				goto out;
  			}
diff --combined t/lib-submodule-update.sh
index f0b1b18206,2c17826e95..58bd4aeb2c
--- a/t/lib-submodule-update.sh
+++ b/t/lib-submodule-update.sh
@@@ -73,7 -73,6 +73,7 @@@ create_lib_submodule_repo () 
  
  		git checkout -b "add_sub1" &&
  		git submodule add ../submodule_update_sub1 sub1 &&
 +		git submodule add ../submodule_update_sub1 uninitialized_sub &&
  		git config -f .gitmodules submodule.sub1.ignore all &&
  		git config submodule.sub1.ignore all &&
  		git add .gitmodules &&
@@@ -788,11 -787,6 +788,6 @@@ test_submodule_switch_recursing () 
  	then
  		RESULTDS=failure
  	fi
- 	RESULTR=success
- 	if test "$KNOWN_FAILURE_SUBMODULE_RECURSIVE_NESTED" = 1
- 	then
- 		RESULTR=failure
- 	fi
  	RESULTOI=success
  	if test "$KNOWN_FAILURE_SUBMODULE_OVERWRITE_IGNORED_UNTRACKED" = 1
  	then
@@@ -1004,7 -998,7 +999,7 @@@
  	'
  
  	# recursing deeper than one level doesn't work yet.
- 	test_expect_$RESULTR "$command: modified submodule updates submodule recursively" '
+ 	test_expect_success "$command: modified submodule updates submodule recursively" '
  		prolog &&
  		reset_work_tree_to_interested add_nested_sub &&
  		(
@@@ -1213,31 -1207,14 +1208,31 @@@ test_submodule_forced_switch_recursing 
  		)
  	'
  	# Updating a submodule from an invalid sha1 updates
 -	test_expect_success "$command: modified submodule does not update submodule work tree from invalid commit" '
 +	test_expect_success "$command: modified submodule does update submodule work tree from invalid commit" '
  		prolog &&
  		reset_work_tree_to_interested invalid_sub1 &&
  		(
  			cd submodule_update &&
  			git branch -t valid_sub1 origin/valid_sub1 &&
 -			test_must_fail $command valid_sub1 &&
 -			test_superproject_content origin/invalid_sub1
 +			$command valid_sub1 &&
 +			test_superproject_content origin/valid_sub1 &&
 +			test_submodule_content sub1 origin/valid_sub1
 +		)
 +	'
 +
 +	# Old versions of Git were buggy writing the .git link file
 +	# (e.g. before f8eaa0ba98b and then moving the superproject repo
 +	# whose submodules contained absolute paths)
 +	test_expect_success "$command: updating submodules fixes .git links" '
 +		prolog &&
 +		reset_work_tree_to_interested add_sub1 &&
 +		(
 +			cd submodule_update &&
 +			git branch -t modify_sub1 origin/modify_sub1 &&
 +			echo "gitdir: bogus/path" >sub1/.git &&
 +			$command modify_sub1 &&
 +			test_superproject_content origin/modify_sub1 &&
 +			test_submodule_content sub1 origin/modify_sub1
  		)
  	'
  }