Re: [PATCH 2/3] submodule: ensure core.worktree is set after update

["SZEDER Gábor" <szeder.dev@gmail.com>]

> +static int connect_gitdir_workingtree(int argc, const char **argv, const char *prefix)
> +{
> +	struct strbuf sb = STRBUF_INIT;
> +	const char *name, *path;
> +	char *sm_gitdir;
> +
> +	if (argc != 3)
> +		BUG("submodule--helper connect-gitdir-workingtree <name> <path>");

So this aborts when it's invoked with the wrong number of cmdline
arguments.

> +
> +	name = argv[1];
> +	path = argv[2];
> +
> +	strbuf_addf(&sb, "%s/modules/%s", get_git_dir(), name);
> +	sm_gitdir = absolute_pathdup(sb.buf);
> +
> +	connect_work_tree_and_git_dir(path, sm_gitdir, 0);
> +
> +	strbuf_release(&sb);
> +	free(sm_gitdir);
> +
> +	return 0;
> +}
> +
>  #define SUPPORT_SUPER_PREFIX (1<<0)
>  
>  struct cmd_struct {

> diff --git a/git-submodule.sh b/git-submodule.sh
> index 78073cd87d1..6596a77c5ef 100755
> --- a/git-submodule.sh
> +++ b/git-submodule.sh
> @@ -615,6 +615,11 @@ cmd_update()
>  			die "$(eval_gettext "Unable to find current \${remote_name}/\${branch} revision in submodule path '\$sm_path'")"
>  		fi
>  
> +		if ! $(git config -f "$(git rev-parse --git-common-dir)/modules/$name/config" core.worktree) 2>/dev/null
> +		then
> +			git submodule--helper connect-gitdir-workingtree $name $sm_path

The path to the submodule, $sm_path, may contain spaces, so it must
be quoted.

I'm sure you would have noticed this already, had you checked this
'submodule--helper's exit code :)  In t7406 the test 'submodule update
properly revives a moved submodule' does update a submodule with a
space in its name, and thus executes 'submodule--helper' with one more
argument than expected, causing it to abort, but since there is no
error checking, 'git submodule update' continues anyway, and in the
end the test tends to pass[1].

I think it would be prudent to check the exit code of all
'submodule--helper' executions.


[1] I wrote "tends to", because e.g. on Travis CI the aborting
    'submodule--helper' often dumps its core, and the extra 'core'
    file shows up in the output of 'git status' and 'test_cmp' notices
    it.


> +		fi
> +
>  		if test "$subsha1" != "$sha1" || test -n "$force"
>  		then
>  			subforce=$force
> -- 
> 2.18.0.rc1.244.gcf134e6275-goog
> 
>