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

2017-02-15 Thread Junio C Hamano
Stefan Beller  writes:

> On Wed, Feb 15, 2017 at 10:22 AM, Junio C Hamano  wrote:
>> Stefan Beller  writes:
>>
>>> 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 
>>
>> 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?
>
> I'll move the code from the absorbing here (i.e. lose the
> SCLD lines there) and make it a die(_(..)) instead of fprintf here.

OK.  I didn't actually meant to suggest the former (I meant that I
expect that would happen in the future steps of this series, or it
can be left as a follow-up patch after the series settles); the
latter may be worth doing.

Thanks.


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

2017-02-15 Thread Stefan Beller
On Wed, Feb 15, 2017 at 10:22 AM, Junio C Hamano  wrote:
> Stefan Beller  writes:
>
>> 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 
>
> 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?

I'll move the code from the absorbing here (i.e. lose the
SCLD lines there) and make it a die(_(..)) instead of fprintf here.

Thanks,
Stefan


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

2017-02-15 Thread Junio C Hamano
Stefan Beller  writes:

> 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 

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?

> ---
>  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));


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

2017-02-14 Thread Stefan Beller
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 
---
 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));
 
-- 
2.12.0.rc0.16.gd1691994b4.dirty