On 01/10/10 07:37, Nguyen Thai Ngoc Duy wrote:
> On 10/1/10, Chris Packham <judge.packham@xxxxxxxxx> 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?
I was testing this with grep but also with my submodule changes. I
should probably move this to its own topic branch and get it working
then rebase my grep changes on top of it.
>>
>> ---8<---
>> From: Chris Packham <judge.packham@xxxxxxxxx>
>> 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.
I couldn't come up with a better name either, thatâs why I used
"prefix". "cwd_prefix" seems sensible enough to me.
>> };
>> 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?
I actually tried strcmp first, expecting to get a number that I can
increment dir by. What I actually got was -1, maybe I just screwed up
the order of dir and cwd. I'll look into it.
>
>> + 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?
By now dir and cwd are both absolute paths. So I dn't think there is
anything to handle
>> + }
>> + }
>> + strncpy(buffer, dir, size);
>
> So if "cwd" is inside "dir", an absolute "dir" is returned? That does
> not look like a prefix to me.
That is a problem. Maybe I should be returning NULL in that case and let
the existing code handle the cwd inside dir case. I think if I wrote
some tests first I could see the various permutations better.
>> + 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
>>
>>
>
>
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html