Re: [PATCH 6/7] submodule--helper: replace connect-gitdir-workingtree by ensure-core-worktree

[Brandon Williams <bmwill@google.com>]

On 08/03, Stefan Beller wrote:
> e98317508c0 (submodule: ensure core.worktree is set after update,
> 2018-06-18) was overly aggressive in calling connect_work_tree_and_git_dir
> as that ensures both the 'core.worktree' configuration is set as well as
> setting up correct gitlink file pointing at the git directory.
> 
> We do not need to check for the gitlink in this part of the cmd_update
> in git-submodule.sh, as the initial call to update-clone will have ensured
> that. So we can reduce the work to only (check and potentially) set the
> 'core.worktree' setting.
> 
> While at it move the check from shell to C as that proves to be useful in
> a follow up patch, as we do not need the 'name' in shell now.
> 
> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---
>  builtin/submodule--helper.c | 64 +++++++++++++++++++++++--------------
>  git-submodule.sh            |  7 ++--
>  2 files changed, 42 insertions(+), 29 deletions(-)
> 
> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
> index 8b1088ab58a..e7635d5d9ab 100644
> --- a/builtin/submodule--helper.c
> +++ b/builtin/submodule--helper.c
> @@ -1964,6 +1964,45 @@ static int push_check(int argc, const char **argv, const char *prefix)
>  	return 0;
>  }
>  
> +static int ensure_core_worktree(int argc, const char **argv, const char *prefix)
> +{
> +	const struct submodule *sub;
> +	const char *path;
> +	char *cw;
> +	struct repository subrepo;
> +
> +	if (argc != 2)
> +		BUG("submodule--helper connect-gitdir-workingtree <name> <path>");
> +
> +	path = argv[1];
> +
> +	sub = submodule_from_path(the_repository, &null_oid, path);
> +	if (!sub)
> +		BUG("We could get the submodule handle before?");
> +
> +	if (repo_submodule_init(&subrepo, the_repository, path))
> +		die(_("could not get a repository handle for submodule '%s'"), path);
> +
> +	if (!repo_config_get_string(&subrepo, "core.worktree", &cw)) {
> +		char *cfg_file, *abs_path;
> +		const char *rel_path;
> +		struct strbuf sb = STRBUF_INIT;
> +
> +		cfg_file = xstrfmt("%s/config", subrepo.gitdir);

As I mentioned here:
https://public-inbox.org/git/20180807230637.247200-1-bmwill@google.com/T/#t

This lines should probably be more like:

  cfg_file = repo_git_path(&subrepo, "config");

> +
> +		abs_path = absolute_pathdup(path);
> +		rel_path = relative_path(abs_path, subrepo.gitdir, &sb);
> +
> +		git_config_set_in_file(cfg_file, "core.worktree", rel_path);
> +
> +		free(cfg_file);
> +		free(abs_path);
> +		strbuf_release(&sb);
> +	}
> +
> +	return 0;
> +}
> +
>  static int absorb_git_dirs(int argc, const char **argv, const char *prefix)
>  {
>  	int i;
> @@ -2029,29 +2068,6 @@ static int check_name(int argc, const char **argv, const char *prefix)
>  	return 0;
>  }
>  
> -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>");
> -
> -	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 {
> @@ -2065,7 +2081,7 @@ static struct cmd_struct commands[] = {
>  	{"name", module_name, 0},
>  	{"clone", module_clone, 0},
>  	{"update-clone", update_clone, 0},
> -	{"connect-gitdir-workingtree", connect_gitdir_workingtree, 0},
> +	{"ensure-core-worktree", ensure_core_worktree, 0},
>  	{"relative-path", resolve_relative_path, 0},
>  	{"resolve-relative-url", resolve_relative_url, 0},
>  	{"resolve-relative-url-test", resolve_relative_url_test, 0},
> diff --git a/git-submodule.sh b/git-submodule.sh
> index 8caaf274e25..19d010eac06 100755
> --- a/git-submodule.sh
> +++ b/git-submodule.sh
> @@ -535,6 +535,8 @@ cmd_update()
>  	do
>  		die_if_unmatched "$quickabort" "$sha1"
>  
> +		git submodule--helper ensure-core-worktree "$sm_path"
> +
>  		name=$(git submodule--helper name "$sm_path") || exit
>  		if ! test -z "$update"
>  		then
> @@ -577,11 +579,6 @@ 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"
> -		fi
> -
>  		if test "$subsha1" != "$sha1" || test -n "$force"
>  		then
>  			subforce=$force
> -- 
> 2.18.0.132.g195c49a2227
> 

-- 
Brandon Williams