On Fri, Feb 22, 2019 at 12:07 AM Phillip Wood <phillip.wood@xxxxxxxxxxxx> 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@xxxxxxxxx> wrote: > > > >> On Tue, Feb 19, 2019 at 12:05 AM Michal Suchanek <msuchanek@xxxxxxx> 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
