Re: [PATCH 2/2] setup: don't fail if commondir reference is deleted.

[Duy Nguyen <pclouds@gmail.com>]

On Tue, Feb 19, 2019 at 12:05 AM Michal Suchanek <msuchanek@suse.de> wrote:
>
> When adding wotktrees git can die in get_common_dir_noenv while
> examining existing worktrees because the commondir file does not exist.
> Rather than testing if the file exists before reading it handle ENOENT.

I don't think we could go around fixing every access to incomplete
worktrees like this. If this is because of racy 'worktree add', then
perhaps a better solution is make it absolutely clear it's not ready
for anybody to access.

For example, we can suffix the worktree directory name with ".lock"
and make sure get_worktrees() ignores entries ending with ".lock".
That should protect other commands while 'worktree add' is still
running. Only when the worktree is complete that 'worktree add' should
rename the directory to lose ".lock" and run external commands like
git-checkout to populate the worktree.

> Signed-off-by: Michal Suchanek <msuchanek@suse.de>
> ---
> v2:
> - do not test file existence first, just read it and handle ENOENT.
> - handle zero size file correctly
> ---
>  setup.c | 16 +++++++++++-----
>  1 file changed, 11 insertions(+), 5 deletions(-)
>
> diff --git a/setup.c b/setup.c
> index ca9e8a949ed8..dd865f280d34 100644
> --- a/setup.c
> +++ b/setup.c
> @@ -270,12 +270,20 @@ int get_common_dir_noenv(struct strbuf *sb, const char *gitdir)
>  {
>         struct strbuf data = STRBUF_INIT;
>         struct strbuf path = STRBUF_INIT;
> -       int ret = 0;
> +       int ret;
>
>         strbuf_addf(&path, "%s/commondir", gitdir);
> -       if (file_exists(path.buf)) {
> -               if (strbuf_read_file(&data, path.buf, 0) <= 0)
> +       ret = strbuf_read_file(&data, path.buf, 0);
> +       if (ret <= 0) {
> +               /*
> +                * if file is missing or zero size (just being written)
> +                * assume default, bail otherwise
> +                */
> +               if (ret && errno != ENOENT)
>                         die_errno(_("failed to read %s"), path.buf);
> +               strbuf_addstr(sb, gitdir);
> +               ret = 0;
> +       } else {
>                 while (data.len && (data.buf[data.len - 1] == '\n' ||
>                                     data.buf[data.len - 1] == '\r'))
>                         data.len--;
> @@ -286,8 +294,6 @@ int get_common_dir_noenv(struct strbuf *sb, const char *gitdir)
>                 strbuf_addbuf(&path, &data);
>                 strbuf_add_real_path(sb, path.buf);
>                 ret = 1;
> -       } else {
> -               strbuf_addstr(sb, gitdir);
>         }
>
>         strbuf_release(&data);
> --
> 2.20.1
>


-- 
Duy