Re: [PATCH] t7002: test git grep --no-index from a bare repository
- From
Nguyen Thai Ngoc Duy <pclouds@gmail.com>
- Date
- Jul 22, 2010, 22:36 UTC
- Message-ID
- <AANLkTik4LvZwHdZ6cFW2EcBXWZ8fOrp5qqIfkepndL3k@mail.gmail.com>
- In-Reply-To
- <20100722070219.GA6577@burratino>
2010/7/22 Jonathan Nieder <jrnieder@gmail.com>:
Show 53 quoted lines
> 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.
Show 27 quoted lines
>> >> 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