Re: [PATCH v4 3/4] submodule: record superproject gitdir during absorbgitdirs

From: Jonathan Tan <jonathantanmy@google.com>
Date: Oct 18, 2021, 23:18 UTC
Message-ID: <20211018231818.89219-1-jonathantanmy@google.com>
In-Reply-To: <20211014203416.2802639-4-emilyshaffer@google.com>

Show 5 quoted lines

> Already during 'git submodule add' we record a pointer to the
> superproject's gitdir. However, this doesn't help brand-new
> submodules created with 'git init' and later absorbed with 'git
> submodule absorbgitdir'. Let's start adding that pointer during 'git
> submodule absorbgitdir' too.

s/absorbgitdir/absorbgitdirs/ (note the "s" at the end)

Show 16 quoted lines

> @@ -2114,6 +2115,15 @@ static void relocate_single_git_dir_into_superproject(const char *path)
>  
>  	relocate_gitdir(path, real_old_git_dir, real_new_git_dir);
>  
> +	/* cache pointer to superproject's gitdir */
> +	/* NEEDSWORK: this may differ if experimental.worktreeConfig is enabled */
> +	strbuf_addf(&config_path, "%s/config", real_new_git_dir);
> +	git_config_set_in_file(config_path.buf, "submodule.superprojectGitdir",
> +			       relative_path(absolute_path(get_git_dir()),
> +					     real_new_git_dir, &sb));
> +
> +	strbuf_release(&config_path);
> +	strbuf_release(&sb);
>  	free(old_git_dir);
>  	free(real_old_git_dir);
>  	free(real_new_git_dir);

Here [1] you mention that you'll delete the NEEDSWORK, but it's still there.

Having said that, it might be better to make a test in which we call this command while in a worktree of a superproject. The test might reveal that (as pointed out to me internally) you might need to use the common dir functions instead of the git dir functions to point to the directory that you want (git-worktree.txt distinguishes the 2 if you search for GIT_COMMON_DIR).

Besides that, all 4 patches look good (including the description of the new config variable).

[1] https://lore.kernel.org/git/YWc2iJ7FQJYCnQ7w@google.com/