2010/7/22 Jonathan Nieder <jrnieder@xxxxxxxxx>:
> Hi Duy,
>
> Nguyễn Thái Ngọc Duy wrote:
>
>> Subject: [PATCH] t7002: test git grep --no-index from a bare repository
>
> It’s t7810 now (to keep t7811 company).
>
>> There is an interesting thing about this command. Back in tp/setup
>> series, there is a patch that changes the current behavior,
>> "calculate prefix even if no worktree is found". grep is interesting
>> because it depends on the current behavior, i.e. prefix being NULL
>> in bare repo, while it still needs true prefix to do chdir()
>> stuff in run_pager().
>
> Yes, sorry to let this hang for so long. I liked your setup series
> for many reasons and am happy to see pieces of it coming back to life.
>
>> +++ b/Documentation/git-grep.txt
>> @@ -28,8 +28,9 @@ SYNOPSIS
>> DESCRIPTION
>> -----------
>> Look for specified patterns in the tracked files in the work tree, blobs
>> -registered in the index file, or blobs in given tree objects.
>> -
>> +registered in the index file, or blobs in given tree objects. By default
>> +it will only search tracked files within the current directory (or full
>> +tree if in bare repository).
>
> Probably deserves more detail.
>
> Searches for lines matching the specified patterns in the
> work tree, the index, or a specified tree.
>
> By default, 'git grep' only examines tracked files in the
> subtree of the work tree rooted at the current working
> directory. Output consists of matching lines preceded with
> the corresponding filename and a colon.
>
> With `--cached`, 'git grep' does the same search in the
> version scheduled for the next commit in the index.
>
> With `--no-index`, 'git grep' pays no mind to the index
> file and reports *all* matching files under the working
> directory.
>
> Given a commit name, 'git grep' does the same search in that
> historical version. More generally, given a tree name, 'git
> grep' searches the subtree of that tree object corresponding
> to the path to the current directory from the root of the work
> tree (or the entire tree if there is no work tree, as in a
> bare repository).
>
Yeah.. grep looks more complex than I thought :) Documentation patch
should go separately then. Please go ahead and make a patch of this
text. Oh you did.
>>
>> OPTIONS
>> -------
>> diff --git a/builtin/grep.c b/builtin/grep.c
>> index 597f76b..e8abdc7 100644
>> --- a/builtin/grep.c
>> +++ b/builtin/grep.c
>> @@ -1109,6 +1109,8 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
>>
>> if (use_threads)
>> hit |= wait_all();
>> +
>> + /* FIXME prefix is NULL in bare repo, no matter where cwd is */
>> if (hit && show_in_pager)
>> run_pager(&opt, prefix);
>
> This comment seems kind of unhelpful. Maybe something like
>
> /*
> * NOTE NOTE NOTE: Even in a bare repository, the user
> * probably expected the command specified with -O to run in
> * the current directory, but when --no-index is supplied, we
> * are passing it paths relative to the .git directory.
> * Until that changes, this needs not to chdir() in that case.
> */
>
> Do I understand correctly?
Hmm.. on second thought, the fault is probably not grep's. These
things must be consistent together:
- new cwd after setup is at worktree top-level directory (or
undefined if no worktree is found?)
- prefix must be aligned with new cwd. That is, chdir(prefix) must
give the original cwd (**1**)
- gitdir is relative to new cwd
- worktree is relative to new cwd
So probably best to adjust cwd in setup_git_directory_gently() in this
case to align with the NULL prefix, not the other way around.
Something like this (still incorrect):
diff --git a/setup.c b/setup.c
index 87c21f0..b67b3aa 100644
--- a/setup.c
+++ b/setup.c
@@ -412,6 +412,15 @@ const char *setup_git_directory_gently(int *nongit_ok)
inside_git_dir = 1;
if (!work_tree_env)
inside_work_tree = 0;
+
+ /*
+ * The old behavior is return NULL here.
+ * Follow it and cwd back to because
+ * NULL prefix means cwd does not move
+ */
+ if (chdir(cwd))
+ die_errno("Cannot come back to cwd");
+
if (offset != len) {
root_len = offset_1st_component(cwd);
cwd[offset > root_len ? offset : root_len] = '\0';
(**1**) Not entirely correct, if original cwd is outside worktree,
then prefix is NULL anyway because prefix is not designed to be
"../../../"
--
Duy
--
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