{"thread":{"id":"24459","subject":"[PATCH] t7002: test git grep --no-index from a bare repository","startedAt":"2010-07-21T12:23:38Z","lastAt":"2010-07-22T22:36:29Z","messageCount":3,"participants":["Nguyễn Thái Ngọc Duy","Jonathan Nieder","Nguyen Thai Ngoc Duy"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"145965","messageId":"1279715018-24225-1-git-send-email-pclouds@gmail.com","threadId":"24459","inReplyTo":null,"subject":"[PATCH] t7002: test git grep --no-index from a bare repository","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2010-07-21T12:23:38Z","receivedAt":"2010-07-21T12:23:38Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"From: Jonathan Nieder <jrnieder@gmail.com>\n\nIf git grep --no-index is to be a general-purpose replacement for\nstandard grep, then it should work even for examining the content of a\n.git directory.  The current implementation has some problems:\n\n * .git/info/exclude is honored though it shouldn’t be\n * when run from within a .git directory, grep --no-index searches\n   the entire .git directory instead of just directories below\n   the current working directory.\n * when run from within a .git directory, prefix will always\n   be NULL, so run_pager() will not be able to move back to\n   the original cwd.\n\nThe last few tests (which demonstrate that grep --no-index looks for\n.git/info/exclude even when no repository is involved) are by Duy.\nAny bugs in the other tests are my fault.\n\nAlso clarify in document what files will be searched in.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\nSigned-off-by: Jonathan Nieder <jrnieder@gmail.com>\n---\n There is an interesting thing about this command. Back in tp/setup\n series, there is a patch that changes the current behavior,\n \"calculate prefix even if no worktree is found\". grep is interesting\n because it depends on the current behavior, i.e. prefix being NULL\n in bare repo, while it still needs true prefix to do chdir()\n stuff in run_pager().\n\n So I have a feeling that my patch is the right way to go. But\n I really need to watch all other commands and make sure they don't\n depend on this behavior like grep.\n\n Documentation/git-grep.txt |    5 +-\n builtin/grep.c             |    2 +\n\nThese are my additional changes.\n\n t/t7810-grep.sh            |  116 ++++++++++++++++++++++++++++++++++++++++++++\n\nThis one remains the same from tp/setup.\n\n 3 files changed, 121 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt\nindex 5474dd7..bb420d9 100644\n--- a/Documentation/git-grep.txt\n+++ b/Documentation/git-grep.txt\n@@ -28,8 +28,9 @@ SYNOPSIS\n DESCRIPTION\n -----------\n Look for specified patterns in the tracked files in the work tree, blobs\n-registered in the index file, or blobs in given tree objects.\n-\n+registered in the index file, or blobs in given tree objects. By default\n+it will only search tracked files within the current directory (or full\n+tree if in bare repository).\n \n OPTIONS\n -------\ndiff --git a/builtin/grep.c b/builtin/grep.c\nindex 597f76b..e8abdc7 100644\n--- a/builtin/grep.c\n+++ b/builtin/grep.c\n@@ -1109,6 +1109,8 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n \n \tif (use_threads)\n \t\thit |= wait_all();\n+\n+\t/* FIXME prefix is NULL in bare repo, no matter where cwd is */\n \tif (hit && show_in_pager)\n \t\trun_pager(&opt, prefix);\n \tfree_grep_patterns(&opt);\ndiff --git a/t/t7810-grep.sh b/t/t7810-grep.sh\nindex 8a63227..7329433 100755\n--- a/t/t7810-grep.sh\n+++ b/t/t7810-grep.sh\n@@ -494,6 +494,98 @@ test_expect_success 'inside git repository but with --no-index' '\n \t)\n '\n \n+test_expect_success 'set up bare repository' '\n+\n+\trm -fr bare &&\n+\tmkdir -p bare/basis/sub &&\n+\techo hello >bare/basis/file1 &&\n+\techo world >bare/basis/sub/file2 &&\n+\techo \".*o*\" >bare/basis/.gitignore &&\n+\t(\n+\t\tcd bare/basis &&\n+\t\tgit init &&\n+\t\tgit ls-files --other | git update-index --add --stdin &&\n+\t\ttree=$(git write-tree) &&\n+\t\tcommit=$(printf \"basis\\n\" | git commit-tree \"$tree\") &&\n+\t\tgit update-ref -m init refs/heads/master \"$commit\" &&\n+\t\tgit update-index --refresh &&\n+\t\tgit diff-index --exit-code HEAD\n+\t) &&\n+\tgit clone --no-checkout --bare bare/basis bare/d.git &&\n+\tmkdir -p bare/d.git/sub &&\n+\techo olleh >bare/d.git/fich1 &&\n+\techo dlrow >bare/d.git/sub/file2 &&\n+\techo \"file2\" >bare/d.git/.gitignore &&\n+\techo \"o*\" >>bare/d.git/.gitignore\n+\t{\n+\t\techo \"HEAD:.gitignore:.*o*\" &&\n+\t\techo HEAD:file1:hello &&\n+\t\techo HEAD:sub/file2:world\n+\t} >bare/expect.full &&\n+\t: >bare/expect.empty &&\n+\techo file2:dlrow >bare/expect.sub\n+'\n+\n+test_expect_success 'in bare repo, grep without --no-index or --cached fails' '\n+\t(\n+\t\tcd bare/d.git &&\n+\t\ttest_must_fail git grep o >../actual.plain 2>../actual.msg &&\n+\t\tgrep \"work tree\" ../actual.msg &&\n+\t\ttest_cmp ../expect.empty ../actual.plain &&\n+\t\tcd sub &&\n+\t\ttest_must_fail git grep o >../../actual.sub 2>../../actual.msg &&\n+\t\ttest_cmp ../../expect.empty ../../actual.sub &&\n+\t\tgrep \"work tree\" ../../actual.msg\n+\t)\n+'\n+\n+test_expect_success 'in bare repo, --cached and HEAD ignore working dir' '\n+\n+\t(\n+\t\tcd bare/d.git &&\n+\t\ttest_must_fail git grep --cached o >../actual.cached 2>../actual.msg &&\n+\t\ttest_cmp ../expect.empty ../actual.cached &&\n+\t\t! grep fatal ../actual.msg &&\n+\t\tgit grep -e o HEAD >../actual.full &&\n+\t\ttest_cmp ../expect.full ../actual.full &&\n+\t\tcd sub &&\n+\t\ttest_must_fail git grep o >../../actual.sub 2>../../actual.msg &&\n+\t\ttest_cmp ../../expect.empty ../../actual.sub &&\n+\t\tgit grep -e o HEAD >../../actual.full-sub &&\n+\t\ttest_cmp ../../expect.full ../../actual.full-sub\n+\t)\n+'\n+\n+test_expect_success '--no-index from a bare repository' '\n+\trm -f bare/d.git/info/exclude &&\n+\t(\n+\t\tcd bare/d.git &&\n+\t\tgit grep --no-index o >../actual.noindex &&\n+\t\tgrep \"^fich1:olleh\\$\" ../actual.noindex &&\n+\t\tgrep \"^.gitignore:o[*]\\$\" ../actual.noindex &&\n+\t\t! grep file2 ../actual.noindex\n+\t)\n+'\n+\n+test_expect_failure '--no-index from a subdirectory of a bare repository' '\n+\t(\n+\t\tcd bare/d.git/sub &&\n+\t\tgit grep --no-index o >../../actual.sub &&\n+\t\ttest_cmp ../../expect.sub ../../actual.sub\n+\t)\n+'\n+\n+test_expect_failure '--no-index neglects info/exclude in bare repo' '\n+\techo \"fich1\" >bare/d.git/info/exclude &&\n+\t(\n+\t\tcd bare/d.git &&\n+\t\tgit grep --no-index o >../actual.noindex &&\n+\t\tgrep \"^fich1:olleh\\$\" ../actual.noindex &&\n+\t\tgrep \"^.gitignore:o[*]\\$\" ../actual.noindex &&\n+\t\t! grep file2 ../actual.noindex\n+\t)\n+'\n+\n test_expect_success 'setup double-dash tests' '\n cat >double-dash <<EOF &&\n --\n@@ -527,4 +619,28 @@ test_expect_success 'grep -e -- -- path' '\n \ttest_cmp expected actual\n '\n \n+test_expect_success 'Setup fake .git' '\n+\tcd t &&\n+\tGIT_CEILING_DIRECTORIES=\"`pwd`\" &&\n+\texport GIT_CEILING_DIRECTORIES &&\n+\tcd a &&\n+\tmkdir -p .git/info &&\n+\tcd ../..\n+\n+'\n+\n+test_expect_failure 'Ignore fake .git/info/exclude' '\n+\t(\n+\t\tcd t/a &&\n+\t\techo v > .git/info/exclude &&\n+\t\tgit grep --no-index vvv . &&\n+\t\trm .git/info/exclude\n+\t)\n+'\n+\n+test_expect_success 'Unsetup fake .git' '\n+\trm -rf t/a &&\n+\tunset GIT_CEILING_DIRECTORIES\n+'\n+\n test_done\n-- \n1.7.1.rc1.69.g24c2f7\n"},{"id":"145981","messageId":"20100722070219.GA6577@burratino","threadId":"24459","inReplyTo":"1279715018-24225-1-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH] t7002: test git grep --no-index from a bare repository","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-07-22T07:02:19Z","receivedAt":"2010-07-22T07:02:19Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi Duy,\n\nNguyễn Thái Ngọc Duy wrote:\n\n> Subject: [PATCH] t7002: test git grep --no-index from a bare repository\n\nIt’s t7810 now (to keep t7811 company).\n\n>  There is an interesting thing about this command. Back in tp/setup\n>  series, there is a patch that changes the current behavior,\n>  \"calculate prefix even if no worktree is found\". grep is interesting\n>  because it depends on the current behavior, i.e. prefix being NULL\n>  in bare repo, while it still needs true prefix to do chdir()\n>  stuff in run_pager().\n\nYes, sorry to let this hang for so long.  I liked your setup series\nfor many reasons and am happy to see pieces of it coming back to life.\n\n> +++ b/Documentation/git-grep.txt\n> @@ -28,8 +28,9 @@ SYNOPSIS\n>  DESCRIPTION\n>  -----------\n>  Look for specified patterns in the tracked files in the work tree, blobs\n> -registered in the index file, or blobs in given tree objects.\n> -\n> +registered in the index file, or blobs in given tree objects. By default\n> +it will only search tracked files within the current directory (or full\n> +tree if in bare repository).\n\nProbably deserves more detail.\n\n\tSearches for lines matching the specified patterns in the\n\twork tree, the index, or a specified tree.\n\n\tBy default, 'git grep' only examines tracked files in the\n\tsubtree of the work tree rooted at the current working\n\tdirectory.  Output consists of matching lines preceded with\n\tthe corresponding filename and a colon.\n\n\tWith `--cached`, 'git grep' does the same search in the\n\tversion scheduled for the next commit in the index.\n\n\tWith `--no-index`, 'git grep' pays no mind to the index\n\tfile and reports *all* matching files under the working\n\tdirectory.\n\n\tGiven a commit name, 'git grep' does the same search in that\n\thistorical version.  More generally, given a tree name, 'git\n\tgrep' searches the subtree of that tree object corresponding\n\tto the path to the current directory from the root of the work\n\ttree (or the entire tree if there is no work tree, as in a\n\tbare repository).\n\n>  \n>  OPTIONS\n>  -------\n> diff --git a/builtin/grep.c b/builtin/grep.c\n> index 597f76b..e8abdc7 100644\n> --- a/builtin/grep.c\n> +++ b/builtin/grep.c\n> @@ -1109,6 +1109,8 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n>  \n>  \tif (use_threads)\n>  \t\thit |= wait_all();\n> +\n> +\t/* FIXME prefix is NULL in bare repo, no matter where cwd is */\n>  \tif (hit && show_in_pager)\n>  \t\trun_pager(&opt, prefix);\n\nThis comment seems kind of unhelpful.  Maybe something like\n\n\t/*\n\t * NOTE NOTE NOTE: Even in a bare repository, the user\n\t * probably expected the command specified with -O to run in\n\t * the current directory, but when --no-index is supplied, we\n\t * are passing it paths relative to the .git directory.\n\t * Until that changes, this needs not to chdir() in that case.\n\t */\n\nDo I understand correctly?\n\n-- 8< --\nSubject: grep documentation: flesh out description\n\nAs Duy noticed, it is not always obvious what directory ‘git grep’\nis going to search in.  Add some other details to the description,\ntoo.\n\nInspired by http://gitster.livejournal.com/27674.html\n\nCc: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\nSigned-off-by: Jonathan Nieder <jrnieder@gmail.com>\n---\n Documentation/git-grep.txt |   21 +++++++++++++++++++--\n 1 files changed, 19 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt\nindex 5474dd7..d6cfbc6 100644\n--- a/Documentation/git-grep.txt\n+++ b/Documentation/git-grep.txt\n@@ -27,9 +27,26 @@ SYNOPSIS\n \n DESCRIPTION\n -----------\n-Look for specified patterns in the tracked files in the work tree, blobs\n-registered in the index file, or blobs in given tree objects.\n-\n+Searches for lines matching the specified patterns in the work\n+tree, the index, or the specified tree objects.\n+\n+By default, 'git grep' only examines tracked files in the subtree\n+of the work tree rooted at the current working directory.  Output\n+consists of matching lines preceded with the corresponding\n+filename and a colon.\n+\n+With `--cached`, 'git grep' does the same search in the version\n+scheduled for the next commit in the index.\n+\n+With `--no-index`, 'git grep' pays no mind to the index file and\n+reports *all* matching files under the working directory.\n+\n+When passed a commit name, 'git grep' does the same search but in\n+that historical version.  More generally, given a tree name,\n+'git grep' searches the subtree of that tree object corresponding\n+to the path to the current directory from the root of the work tree\n+(or the entire tree if there is no work tree, as in a bare\n+repository).\n \n OPTIONS\n -------\n-- \n1.7.2.rc3\n"},{"id":"146028","messageId":"AANLkTik4LvZwHdZ6cFW2EcBXWZ8fOrp5qqIfkepndL3k@mail.gmail.com","threadId":"24459","inReplyTo":"20100722070219.GA6577@burratino","subject":"Re: [PATCH] t7002: test git grep --no-index from a bare repository","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2010-07-22T22:36:29Z","receivedAt":"2010-07-22T22:36:29Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"2010/7/22 Jonathan Nieder <jrnieder@gmail.com>:\n> Hi Duy,\n>\n> Nguyễn Thái Ngọc Duy wrote:\n>\n>> Subject: [PATCH] t7002: test git grep --no-index from a bare repository\n>\n> It’s t7810 now (to keep t7811 company).\n>\n>>  There is an interesting thing about this command. Back in tp/setup\n>>  series, there is a patch that changes the current behavior,\n>>  \"calculate prefix even if no worktree is found\". grep is interesting\n>>  because it depends on the current behavior, i.e. prefix being NULL\n>>  in bare repo, while it still needs true prefix to do chdir()\n>>  stuff in run_pager().\n>\n> Yes, sorry to let this hang for so long.  I liked your setup series\n> for many reasons and am happy to see pieces of it coming back to life.\n>\n>> +++ b/Documentation/git-grep.txt\n>> @@ -28,8 +28,9 @@ SYNOPSIS\n>>  DESCRIPTION\n>>  -----------\n>>  Look for specified patterns in the tracked files in the work tree, blobs\n>> -registered in the index file, or blobs in given tree objects.\n>> -\n>> +registered in the index file, or blobs in given tree objects. By default\n>> +it will only search tracked files within the current directory (or full\n>> +tree if in bare repository).\n>\n> Probably deserves more detail.\n>\n>        Searches for lines matching the specified patterns in the\n>        work tree, the index, or a specified tree.\n>\n>        By default, 'git grep' only examines tracked files in the\n>        subtree of the work tree rooted at the current working\n>        directory.  Output consists of matching lines preceded with\n>        the corresponding filename and a colon.\n>\n>        With `--cached`, 'git grep' does the same search in the\n>        version scheduled for the next commit in the index.\n>\n>        With `--no-index`, 'git grep' pays no mind to the index\n>        file and reports *all* matching files under the working\n>        directory.\n>\n>        Given a commit name, 'git grep' does the same search in that\n>        historical version.  More generally, given a tree name, 'git\n>        grep' searches the subtree of that tree object corresponding\n>        to the path to the current directory from the root of the work\n>        tree (or the entire tree if there is no work tree, as in a\n>        bare repository).\n>\n\nYeah.. grep looks more complex than I thought :) Documentation patch\nshould go separately then. Please go ahead and make a patch of this\ntext. Oh you did.\n\n>>\n>>  OPTIONS\n>>  -------\n>> diff --git a/builtin/grep.c b/builtin/grep.c\n>> index 597f76b..e8abdc7 100644\n>> --- a/builtin/grep.c\n>> +++ b/builtin/grep.c\n>> @@ -1109,6 +1109,8 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n>>\n>>       if (use_threads)\n>>               hit |= wait_all();\n>> +\n>> +     /* FIXME prefix is NULL in bare repo, no matter where cwd is */\n>>       if (hit && show_in_pager)\n>>               run_pager(&opt, prefix);\n>\n> This comment seems kind of unhelpful.  Maybe something like\n>\n>        /*\n>         * NOTE NOTE NOTE: Even in a bare repository, the user\n>         * probably expected the command specified with -O to run in\n>         * the current directory, but when --no-index is supplied, we\n>         * are passing it paths relative to the .git directory.\n>         * Until that changes, this needs not to chdir() in that case.\n>         */\n>\n> Do I understand correctly?\n\nHmm.. on second thought, the fault is probably not grep's. These\nthings must be consistent together:\n\n - new cwd after setup is at worktree top-level directory (or\nundefined if no worktree is found?)\n - prefix must be aligned with new cwd. That is, chdir(prefix) must\ngive the original cwd (**1**)\n - gitdir is relative to new cwd\n - worktree is relative to new cwd\n\nSo probably best to adjust cwd in setup_git_directory_gently() in this\ncase to align with the NULL prefix, not the other way around.\nSomething like this (still incorrect):\n\ndiff --git a/setup.c b/setup.c\nindex 87c21f0..b67b3aa 100644\n--- a/setup.c\n+++ b/setup.c\n@@ -412,6 +412,15 @@ const char *setup_git_directory_gently(int *nongit_ok)\n \t\t\tinside_git_dir = 1;\n \t\t\tif (!work_tree_env)\n \t\t\t\tinside_work_tree = 0;\n+\n+\t\t\t/*\n+\t\t\t * The old behavior is return NULL here.\n+\t\t\t * Follow it and cwd back to because\n+\t\t\t * NULL prefix means cwd does not move\n+\t\t\t */\n+\t\t\tif (chdir(cwd))\n+\t\t\t\tdie_errno(\"Cannot come back to cwd\");\n+\n \t\t\tif (offset != len) {\n \t\t\t\troot_len = offset_1st_component(cwd);\n \t\t\t\tcwd[offset > root_len ? offset : root_len] = '\\0';\n\n\n(**1**) Not entirely correct, if original cwd is outside worktree,\nthen prefix is NULL anyway because prefix is not designed to be\n\"../../../\"\n-- \nDuy\n"}]}