Re: [PATCH v3 09/10] fetch: fetch unpopulated, changed submodules

> In the process of writing the new tests [1], I noticed some failures of
> the form:
> 
>   # rm the submodule's working tree directory.
>   $ git rm submodule
>   [...]
> 
>   # Do a fetch that requires running a child process from the submodule.
>   $ git fetch --recurse-submodules same-name-1 
>   [...]
> 
>   # Fatal error tells us that we cannot chdir to the deleted working
>     tree.
>   fatal: cannot chdir to '../../../submodule': No such file or directory
> 
> This happens because submodules set/unset a value for core.worktree when
> they are checked out/"un-checked out" (see submodule_move_head() and
> connect_work_tree_and_git_dir()), but "git rm" doesn't know that
> core.worktree should be updated.
> 
> I've worked around this by passing "--work-tree=." to the child process
> [2], but this feels like a hack, especially because this bug should
> affect all child processes in a "git rm"-ed submodule (this probably
> includes the "git branch" processes in gc/branch-recurse-submodules, but
> I haven't confirmed it yet). 

> Some more comprehensive solutions that
> could be future work are:
> 
> - Teach "git [add|rm]" to unset core.worktree (the reverse operation,
>   "git restore", should already do the correct thing). This won't detect
>   submodules removed with "rm -r" though.

> @@ -865,6 +876,8 @@ static void collect_changed_submodules_cb(struct diff_queue_struct *q,
>  		if (!item->util)
>  			item->util = xcalloc(1, sizeof(struct changed_submodule_data));
>  		cs_data = item->util;
> +		cs_data->super_oid = commit_oid;
> +		cs_data->path = xstrdup(p->two->path);

> @@ -1253,14 +1266,33 @@ void check_for_new_submodule_commits(struct object_id *oid)
>  	oid_array_append(&ref_tips_after_fetch, oid);
>  }
>  
> +/*
> + * Returns 1 if there is at least one submodule gitdir in
> + * $GIT_DIR/modules and 0 otherwise. This follows
> + * submodule_name_to_gitdir(), which looks for submodules in
> + * $GIT_DIR/modules, not $GIT_COMMON_DIR.
> + *
> + * A submodule can be moved to $GIT_DIR/modules manually by running "git
> + * submodule absorbgitdirs", or it may be initialized there by "git
> + * submodule update".
> + */
> +static int repo_has_absorbed_submodules(struct repository *r)
> +{
> +	struct strbuf buf = STRBUF_INIT;
> +
> +	strbuf_repo_git_path(&buf, r, "modules/");
> +	return file_exists(buf.buf) && !is_empty_dir(buf.buf);
> +}

> @@ -1333,7 +1365,8 @@ int submodule_touches_in_range(struct repository *r,
>  }
>  
>  struct submodule_parallel_fetch {
> -	int count;
> +	int index_count;
> +	int changed_count;
>  	struct strvec args;
>  	struct repository *r;
>  	const char *prefix;

> @@ -1343,6 +1376,7 @@ struct submodule_parallel_fetch {
>  	int result;
>  
>  	struct string_list changed_submodule_names;
> +	struct string_list seen_submodule_names;
>  
>  	/* Pending fetches by OIDs */
>  	struct fetch_task **oid_fetch_tasks;

> +		/*
> +		 * NEEDSWORK: A submodule unpopulated by "git rm" will
> +		 * have core.worktree set, but the actual core.worktree
> +		 * directory won't exist, causing the child process to
> +		 * fail. Forcibly set --work-tree until we get smarter
> +		 * handling for core.worktree in unpopulated submodules.
> +		 */
> +		strvec_push(&task->git_args, "--work-tree=.");
> +		return task;
> +	}
> +	return NULL;
> +}

> +# In downstream, init "submodule2", but do not check it out while
> +# fetching. This lets us assert that unpopulated submodules can be
> +# fetched.
> +test_expect_success 'setup downstream branch with other submodule' '
> +	mkdir submodule2 &&
> +	(
> +		cd submodule2 &&
> +		git init &&
> +		echo sub2content >sub2file &&
> +		git add sub2file &&
> +		git commit -a -m new &&
> +		git branch -M sub2
> +	) &&
> +	git checkout -b super-sub2-only &&
> +	git submodule add "$pwd/submodule2" submodule2 &&
> +	git commit -m "add sub2" &&
> +	git checkout super &&
> +	(
> +		cd downstream &&
> +		git fetch --recurse-submodules origin &&
> +		git checkout super-sub2-only &&
> +		# Explicitly run "git submodule update" because sub2 is new
> +		# and has not been cloned.
> +		git submodule update --init &&
> +		git checkout --recurse-submodules super
> +	)
> +'

> +	# Use test_cmp manually because verify_fetch_result does not
> +	# consider submodule2. All the repos should be fetched, but only
> +	# submodule2 should be read from a commit
> +	cat <<-EOF > expect.err.combined &&
> +	From $pwd/.
> +	   OLD_HEAD..$super_head  super           -> origin/super
> +	   OLD_HEAD..$super_sub2_only_head  super-sub2-only -> origin/super-sub2-only
> +	Fetching submodule submodule
> +	From $pwd/submodule
> +	   OLD_HEAD..$sub_head  sub        -> origin/sub
> +	Fetching submodule submodule/subdir/deepsubmodule
> +	From $pwd/deepsubmodule
> +	   OLD_HEAD..$deep_head  deep       -> origin/deep
> +	Fetching submodule submodule2 at commit $super_sub2_only_head
> +	From $pwd/submodule2
> +	   OLD_HEAD..$sub2_head  sub2       -> origin/sub2
> +	EOF
> +	sed -E "s/[0-9a-f]+\.\./OLD_HEAD\.\./" actual.err >actual.err.cmp &&
> +	test_cmp expect.err.combined actual.err.cmp
> +'

> +test_expect_success 'fetch --recurse-submodules updates name-conflicted, populated submodule' '
> +	test_when_finished "git -C same-name-downstream checkout master" &&
> +	(
> +		cd same-name-1 &&
> +		test_commit -C submodule --no-tag b1 &&
> +		git add submodule &&
> +		git commit -m "super-b1"
> +	) &&
> +	(
> +		cd same-name-2 &&
> +		test_commit -C submodule --no-tag b2 &&
> +		git add submodule &&
> +		git commit -m "super-b2"
> +	) &&
> +	(
> +		cd same-name-downstream &&
> +		# even though the .gitmodules is correct, we cannot
> +		# fetch from same-name-2
> +		git checkout same-name-2/master &&
> +		git fetch --recurse-submodules same-name-1 &&
> +		test_must_fail git fetch --recurse-submodules same-name-2

> +	(
> +		cd same-name-downstream/.git/modules/submodule &&
> +		# The submodule has core.worktree pointing to the "git
> +		# rm"-ed directory, overwrite the invalid value.
> +		git --work-tree=. cat-file -e $head1 &&
> +		test_must_fail git --work-tree=. cat-file -e $head2
> +	)