Re: [PATCHv2 4/5] submodule--helper, module_clone: always operate on absolute paths

[Junio C Hamano <gitster@pobox.com>]

Stefan Beller <sbeller@google.com> writes:

> +	if (!is_absolute_path(sm_gitdir)) {
> +		char *cwd = xgetcwd();
> +		strbuf_addf(&sb, "%s/%s", cwd, sm_gitdir);
> +		sm_gitdir = strbuf_detach(&sb, 0);
> +		free(cwd);

It would be surprising that this would be the first codepath that
needs to get an absolute pathname in the codebase that is more than
10 years old, wouldn't it?  Don't we have a reasonable API helper
function to do this kind of thing already?

    ... goes and looks ...

Doesn't absolute_path() work here?

> @@ -221,7 +240,6 @@ static int module_clone(int argc, const char **argv, const char *prefix)
>  	submodule_dot_git = fopen(sb.buf, "w");
>  	if (!submodule_dot_git)
>  		die_errno(_("cannot open file '%s'"), sb.buf);
> -
>  	fprintf(submodule_dot_git, "gitdir: %s\n",
>  		relative_path(sm_gitdir, path, &rel_path));
>  	if (fclose(submodule_dot_git))

Looks like an unrelated change to me.

> @@ -229,24 +247,16 @@ static int module_clone(int argc, const char **argv, const char *prefix)
>  	strbuf_reset(&sb);
>  	strbuf_reset(&rel_path);
>  
> -	cwd = xgetcwd();
>  	/* Redirect the worktree of the submodule in the superproject's config */
> -	if (!is_absolute_path(sm_gitdir)) {
> -		strbuf_addf(&sb, "%s/%s", cwd, sm_gitdir);
> -		free(sm_gitdir);
> -		sm_gitdir = strbuf_detach(&sb, NULL);
> -	}
> -
> -	strbuf_addf(&sb, "%s/%s", cwd, path);
>  	p = git_pathdup_submodule(path, "config");
>  	if (!p)
>  		die(_("could not get submodule directory for '%s'"), path);
>  	git_config_set_in_file(p, "core.worktree",
> -			       relative_path(sb.buf, sm_gitdir, &rel_path));
> +			       relative_path(path, sm_gitdir, &rel_path));
>  	strbuf_release(&sb);
>  	strbuf_release(&rel_path);
>  	free(sm_gitdir);
> -	free(cwd);
> +

This addition of blank, too.

>  	free(p);
>  	return 0;
>  }
> diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
> index fc11809..ea3fabb 100755
> --- a/t/t7400-submodule-basic.sh
> +++ b/t/t7400-submodule-basic.sh
> @@ -818,7 +818,7 @@ test_expect_success 'submodule add --name allows to replace a submodule with ano
>  	)
>  '
>  
> -test_expect_failure 'recursive relative submodules stay relative' '
> +test_expect_success 'recursive relative submodules stay relative' '
>  	test_when_finished "rm -rf super clone2 subsub sub3" &&
>  	mkdir subsub &&
>  	(