Subject: Re: [PATCH v4 1/6] worktree.c: add find_worktree()

From: Ramsay Jones <[email-masked]>
Date: Fri, 3 Jun 2016 16:00:51 +0100

On 03/06/16 13:19, Nguyễn Thái Ngọc Duy wrote:
> So far we haven't needed to identify an existing worktree from command
> line. Future commands such as lock or move will need it. The current
> implementation identifies worktrees by path (*). In future, the function
> could learn to identify by $(basename $path) or tags...
> 
> (*) We could probably go cheaper with comparing inode number (and
> probably more reliable than paths when unicode enters the game). But not
> all systems have good inode that so let's stick to something simple for
> now.
> 
> Helped-by: Eric Sunshine <[email-masked]>
> Signed-off-by: Nguyễn Thái Ngọc Duy <[email-masked]>
> ---
>  worktree.c | 15 +++++++++++++++
>  worktree.h |  8 ++++++++
>  2 files changed, 23 insertions(+)
> 
> diff --git a/worktree.c b/worktree.c
> index e2a94e0..554f566 100644
> --- a/worktree.c
> +++ b/worktree.c
> @@ -214,6 +214,21 @@ const char *get_worktree_git_dir(const struct worktree *wt)
>  		return git_common_path("worktrees/%s", wt->id);
>  }
>  
> +struct worktree *find_worktree(struct worktree **list,
> +			       const char *prefix,
> +			       const char *arg)
> +{
> +	char *path;
> +
> +	arg = prefix_filename(prefix, strlen(prefix), arg);
> +	path = xstrdup(real_path(arg));
> +	for (; *list; list++)
> +		if (!fspathcmp(path, real_path((*list)->path)))

The results of the call to real_path() should probably be cached
in the worktree structure, since real_path() is relatively expensive
(it calls chdir(), lstat(), readlink() etc.), so you don't want to
re-compute the same result time-after-time ...

> +			break;
> +	free(path);
> +	return *list;
> +}
> +

ATB,
Ramsay Jones