Re: [RFC PATCH 2/3] grep: prepare grep for submodules

[Nguyen Thai Ngoc Duy <pclouds@gmail.com>]

On 10/1/10, Chris Packham <judge.packham@gmail.com> wrote:
>  > You can make setup_explicit_git_dir() realize that situation (current
>  > working directory outside $GIT_WORK_TREE), then calculate and save the
>  > submodule prefix in startup_info struct. Then "git grep" or any
>  > commands can just read startup_info to find out the submodule prefix.
>
>
> Here's my first naive attempt at implementing what you describe. Needs
>  tests, more comments, sign-off etc.

Thanks.

>  One situation that could be handled better is when the cwd is a
>  subdirectory of the specified worktree. At the moment this ends up
>  giving the full path to the worktree, the output would look much nicer
>  if it gave the relative path (e.g. ../../).

Hmm.. if cwd is inside a worktree, prefix (the 3rd parameter in
cmd_grep) should be correctly set and "git grep" should also show
relative path. Or are you talking about another command?

>
>  ---8<---
>  From: Chris Packham <judge.packham@gmail.com>
>  Date: Thu, 30 Sep 2010 11:19:29 -0700
>  Subject: [RFC PATCH] save the work tree prefix in startup_info
>
>  This is the relative path between the cwd and the worktree or the
>  absolute path of the worktree if the worktree is not a subdirectory
>  of the worktree.
>  ---
>   cache.h |    1 +
>   dir.c   |   26 ++++++++++++++++++++++++++
>   dir.h   |    1 +
>   setup.c |    4 ++++
>   4 files changed, 32 insertions(+), 0 deletions(-)
>
>  diff --git a/cache.h b/cache.h
>  index e1d3ffd..f320e78 100644
>  --- a/cache.h
>  +++ b/cache.h
>  @@ -1111,6 +1111,7 @@ const char *split_cmdline_strerror(int cmdline_errno);
>   /* git.c */
>   struct startup_info {
>         int have_repository;
>  +       const char *prefix;

You should use another name here to avoid confusion with the current
prefix, relative to worktree toplevel directory. I'm thinking of
outer_prefix or cwd_prefix, but I'm usually bad at naming.

>   };
>   extern struct startup_info *startup_info;
>
>  diff --git a/dir.c b/dir.c
>  index 58ec1a1..2148730 100644
>  --- a/dir.c
>  +++ b/dir.c
>  @@ -1036,6 +1036,32 @@ char *get_relative_cwd(char *buffer, int size,
>  const char *dir)
>         }
>   }
>
>  +char *get_relative_wt(char *buffer, int size, const char *dir)
>  +{
>  +       char *cwd = buffer;
>  +
>  +       if (!dir)
>  +               return NULL;
>  +       if (!getcwd(buffer, size))
>  +               die_errno("can't find the current directory");
>  +       if (!is_absolute_path(dir))
>  +               dir = make_absolute_path(dir);
>  +       if (strstr(dir, cwd)) {

Why not strncmp?

>  +               dir += strlen(cwd);
>  +               switch(*dir){
>  +               case '\0':
>  +                       return NULL;
>  +               case '/':
>  +                       dir++;
>  +                       break;

Yeah.

>  +               default:
>  +                       break;

Should we properly handle relative path that includes ".." here too?

>  +               }
>  +       }
>  +       strncpy(buffer, dir, size);

So if "cwd" is inside "dir", an absolute "dir" is returned? That does
not look like a prefix to me.

>  +       return buffer;
>  +}
>  +
>   int is_inside_dir(const char *dir)
>   {
>         char buffer[PATH_MAX];
>  diff --git a/dir.h b/dir.h
>  index b3e2104..d3c161f 100644
>  --- a/dir.h
>  +++ b/dir.h
>  @@ -81,6 +81,7 @@ extern void add_exclude(const char *string, const char
>  *base,
>   extern int file_exists(const char *);
>
>   extern char *get_relative_cwd(char *buffer, int size, const char *dir);
>  +extern char *get_relative_wt(char *buffer, int size, const char *dir);
>   extern int is_inside_dir(const char *dir);
>
>   static inline int is_dot_or_dotdot(const char *name)
>  diff --git a/setup.c b/setup.c
>  index a3b76de..bd9d9fd 100644
>  --- a/setup.c
>  +++ b/setup.c
>  @@ -317,6 +317,7 @@ static const char *setup_explicit_git_dir(const char
>  *gitdirenv,
>                                 const char *work_tree_env, int *nongit_ok)
>   {
>         static char buffer[1024 + 1];
>  +       static char wtbuf[1024 + 1];
>         const char *retval;
>
>         if (PATH_MAX - 40 < strlen(gitdirenv))
>  @@ -337,6 +338,9 @@ static const char *setup_explicit_git_dir(const char
>  *gitdirenv,
>         }
>         if (check_repository_format_gently(nongit_ok))
>                 return NULL;
>  +
>  +       startup_info->prefix=get_relative_wt(wtbuf, sizeof(wtbuf) - 1,
>  +                       get_git_work_tree());
>         retval = get_relative_cwd(buffer, sizeof(buffer) - 1,
>                         get_git_work_tree());
>         if (!retval || !*retval)
>
> --
>  1.7.3.1
>
>


-- 
Duy