Re: [PATCH 07/17] connect_work_tree_and_git_dir: safely create leading directories

[Brandon Williams <bmwill@google.com>]

On 03/09, Stefan Beller wrote:
> In a later patch we'll use connect_work_tree_and_git_dir when the
> directory for the gitlink file doesn't exist yet. This patch makes
> connect_work_tree_and_git_dir safe to use for both cases of
> either the git dir or the working dir missing.
> 
> To do so, we need to call safe_create_leading_directories[_const]
> on both directories. However this has to happen before we construct
> the absolute paths as real_pathdup assumes the directories to
> be there already.
> 
> So for both the config file in the git dir as well as the .git link
> file we need to
> a) construct the name
> b) call SCLD
> c) get the absolute path
> d) once a-c is done for both we can consume the absolute path
>    to compute the relative path to each other and store those
>    relative paths.
> 
> The implementation provided here puts a) and b) for both cases first,
> and then performs c and d after.
> 
> One of the two users of 'connect_work_tree_and_git_dir' already checked
> for the directory being there, so we can loose that check as
> connect_work_tree_and_git_dir handles this functionality now.
> 
> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---
>  dir.c       | 32 +++++++++++++++++++++-----------
>  submodule.c | 11 ++---------
>  2 files changed, 23 insertions(+), 20 deletions(-)
> 
> diff --git a/dir.c b/dir.c
> index 4541f9e146..6f52af7abb 100644
> --- a/dir.c
> +++ b/dir.c
> @@ -2728,23 +2728,33 @@ void untracked_cache_add_to_index(struct index_state *istate,
>  /* Update gitfile and core.worktree setting to connect work tree and git dir */
>  void connect_work_tree_and_git_dir(const char *work_tree_, const char *git_dir_)
>  {
> -	struct strbuf file_name = STRBUF_INIT;
> +	struct strbuf gitfile_sb = STRBUF_INIT;
> +	struct strbuf cfg_sb = STRBUF_INIT;
>  	struct strbuf rel_path = STRBUF_INIT;
> -	char *git_dir = real_pathdup(git_dir_);
> -	char *work_tree = real_pathdup(work_tree_);
> +	char *git_dir, *work_tree;
>  
> -	/* Update gitfile */
> -	strbuf_addf(&file_name, "%s/.git", work_tree);
> -	write_file(file_name.buf, "gitdir: %s",
> -		   relative_path(git_dir, work_tree, &rel_path));
> +	/* Prepare .git file */
> +	strbuf_addf(&gitfile_sb, "%s/.git", work_tree_);
> +	if (safe_create_leading_directories_const(gitfile_sb.buf))
> +		die(_("could not create directories for %s"), gitfile_sb.buf);
> +
> +	/* Prepare config file */
> +	strbuf_addf(&cfg_sb, "%s/config", git_dir_);
> +	if (safe_create_leading_directories_const(cfg_sb.buf))
> +		die(_("could not create directories for %s"), cfg_sb.buf);
>  
> +	git_dir = real_pathdup(git_dir_);
> +	work_tree = real_pathdup(work_tree_);

Just a note that this is a spot that'll be affected by the change to
real_pathdup() which adds a 'die_on_error' parameter to correct bad
behaviour I introduced.

> +
> +	/* Write .git file */
> +	write_file(gitfile_sb.buf, "gitdir: %s",
> +		   relative_path(git_dir, work_tree, &rel_path));
>  	/* Update core.worktree setting */
> -	strbuf_reset(&file_name);
> -	strbuf_addf(&file_name, "%s/config", git_dir);
> -	git_config_set_in_file(file_name.buf, "core.worktree",
> +	git_config_set_in_file(cfg_sb.buf, "core.worktree",
>  			       relative_path(work_tree, git_dir, &rel_path));
>  
> -	strbuf_release(&file_name);
> +	strbuf_release(&gitfile_sb);
> +	strbuf_release(&cfg_sb);
>  	strbuf_release(&rel_path);
>  	free(work_tree);
>  	free(git_dir);
> diff --git a/submodule.c b/submodule.c
> index 0e55372f37..04d185738f 100644
> --- a/submodule.c
> +++ b/submodule.c
> @@ -1442,8 +1442,6 @@ void absorb_git_dir_into_superproject(const char *prefix,
>  
>  	/* Not populated? */
>  	if (!sub_git_dir) {
> -		char *real_new_git_dir;
> -		const char *new_git_dir;
>  		const struct submodule *sub;
>  
>  		if (err_code == READ_GITFILE_ERR_STAT_FAILED) {
> @@ -1466,13 +1464,8 @@ void absorb_git_dir_into_superproject(const char *prefix,
>  		sub = submodule_from_path(null_sha1, path);
>  		if (!sub)
>  			die(_("could not lookup name for submodule '%s'"), path);
> -		new_git_dir = git_path("modules/%s", sub->name);
> -		if (safe_create_leading_directories_const(new_git_dir) < 0)
> -			die(_("could not create directory '%s'"), new_git_dir);
> -		real_new_git_dir = real_pathdup(new_git_dir);
> -		connect_work_tree_and_git_dir(path, real_new_git_dir);
> -
> -		free(real_new_git_dir);
> +		connect_work_tree_and_git_dir(path,
> +			git_path("modules/%s", sub->name));
>  	} else {
>  		/* Is it already absorbed into the superprojects git dir? */
>  		char *real_sub_git_dir = real_pathdup(sub_git_dir);
> -- 
> 2.12.0.rc1.45.g207f5fbb2b
> 

-- 
Brandon Williams