Re: [PATCH 3/5] worktree.c: add is_worktree_locked()

[Duy Nguyen <pclouds@gmail.com>]

On Fri, May 13, 2016 at 11:52 PM, Eric Sunshine <sunshine@sunshineco.com> wrote:
> Actually, I recall that when I suggested the idea of 'struct worktree'
> and get_worktrees() to Mike that it would be natural for the structure
> to have a 'locked' (or 'locked_reason') field, in which case the
> reason could be stored there instead of in this static strbuf. In
> fact, my idea at the time was that get_worktrees() would populate that
> field automatically, rather than having to do so via a separate
> on-demand function call as in this patch.

I'm keeping this as a separate function for now. I don't like
get_worktrees() doing extra work unless it has to. We soon will see
the complete picture of "git worktree" then we can merge it back to
get_worktrees() if it turns out checking "locked" is the common
operation. is_worktree_locked() then may become a thin wrapper to
access this "locked" field.

>> +extern const char *is_worktree_locked(const struct worktree *wt);
>
> I was wondering if builtin/worktree.c:prune_worktree() should be
> updated to invoke this new function instead of consulting
> "worktrees/<id>/locked" manually, but I see that the entire "prune
> worktrees" functionality in builting/worktree.c first needs to be
> updated to the get_worktrees() API for that to happen.

I thought about updating prune too. But it is in a bit special
situation where it may need to consider invalid (or partly invalid)
worktrees as well. So far worktree api is about valid worktrees only
if I'm not mistaken and we probably should keep it that way, otherwise
all callers may need to check "is this worktree valid" all over the
place.
-- 
Duy