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

[Duy Nguyen <pclouds@gmail.com>]

On Fri, Feb 22, 2019 at 12:07 AM Phillip Wood <phillip.wood@talktalk.net> wrote:
>
> Hi Michal/Duy
>
> On 21/02/2019 13:50, Michal Suchánek wrote:
> > On Thu, 21 Feb 2019 17:50:38 +0700
> > Duy Nguyen <pclouds@gmail.com> wrote:
> >
> >> 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.
> >
> > The problem is we don't forbid worktree names ending with ".lock".
> > Which means that if we start to forbid them now existing worktrees
> > might become inaccessible.
>
> I think it is also racy as the renaming breaks the use of mkdir erroring
> out if the directory already exists.

You mean the part where we see "fred" exists and decide to try the
name "fred1" instead (i.e. patch 1/2)?

I don't think it's the problem if that's the case. We mkdir
"fred.lock" _then_ check if "fred" exists. If it does, remove
fred.lock and move on to fred1.lock. Then we rename fred1.lock to
fred1 and error out if rename fails.
-- 
Duy