Re: [PATCH 04/14] connect_work_tree_and_git_dir: safely create leading directories

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 15, 2017, 18:22 UTC
Message-ID: <xmqqh93v48mv.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <20170215003423.20245-5-sbeller@google.com>

Stefan Beller <sbeller@google.com> writes:

Show 5 quoted lines

> In a later patch we'll use connect_work_tree_and_git_dir when the
> directory for the gitlink file doesn't exist yet. Safely create
> the directory first.
>
> Signed-off-by: Stefan Beller <sbeller@google.com>

Among the existing two callers, the "absorb" logic in submodule.c has safe-create-leading-directory (SCLD) immediately before the call to this function, which can now be lost with this change. The other one in cmd_mv() has the target directory as the result of moving the original directory, and I do not think there is any corresponding call that can be lost from there after this change, but it is not an error to call SCLD on a path that already exists, so all is OK.

Is it sensible to let the code continue with just an fprintf() (not even warning() or error()) when SCLD fails?

Show 14 quoted lines

> ---
>  dir.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/dir.c b/dir.c
> index 4541f9e146..69ca3d1411 100644
> --- a/dir.c
> +++ b/dir.c
> @@ -2735,6 +2735,8 @@ void connect_work_tree_and_git_dir(const char *work_tree_, const char *git_dir_)
>  
>  	/* Update gitfile */
>  	strbuf_addf(&file_name, "%s/.git", work_tree);
> +	if (safe_create_leading_directories_const(file_name.buf))
> +		fprintf(stderr, "could not create directories for %s\n", file_name.buf);

>  	write_file(file_name.buf, "gitdir: %s",
>  		   relative_path(git_dir, work_tree, &rel_path));