{"thread":{"id":"25457","subject":"[RGC/PATCHv2] grep: submodule support","startedAt":"2010-10-15T23:26:39Z","lastAt":"2010-10-18T03:37:01Z","messageCount":15,"participants":["Chris Packham","Jens Lehmann","Jonathan Nieder","Nguyen Thai Ngoc Duy"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"153608","messageId":"1287185204-843-1-git-send-email-judge.packham@gmail.com","threadId":"25457","inReplyTo":null,"subject":"[RGC/PATCHv2] grep: submodule support","fromName":"Chris Packham","fromEmail":"judge.packham@gmail.com","sentAt":"2010-10-15T23:26:39Z","receivedAt":"2010-10-15T23:26:39Z","isPatch":false,"sender":{"key":"judge.packham@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155667?v=4"},"body":"Here is my updated grep support for submodules series. The old series\ncan be seen here [1].\n\n[1] http://article.gmane.org/gmane.comp.version-control.git/157599\n\nThis the actual implementation is far from complete but the first 3\npatches should be OK on their own. Since the last series I've rebased to\nmaster on git.git so hopefully more people can try it out if they want\nto. I'll be travelling/moving for the next week or so I may be a little \nunresponsive.\n\nThe changes since last time are:\n\n* use environment variables to pass some information to the sub-process\n* a lot more options supported by create_sub_grep_argv, including\n  rebuilding the pattern list.\n* figure out the correct prefix based on the cwd & worktree (thanks Duy).\n\n\nChris Packham (4):\n      grep: output the path from cwd to worktree\n      grep_cache: check pathspec first\n      add test for git grep --recursive\n      grep: add support for grepping in submodules\n\nNguyễn Thái Ngọc Duy (1):\n      worktree: provide better prefix to go back to original cwd\n"},{"id":"153609","messageId":"1287185204-843-2-git-send-email-judge.packham@gmail.com","threadId":"25457","inReplyTo":"1287185204-843-1-git-send-email-judge.packham@gmail.com","subject":"[RFC/PATCHv2 1/5] worktree: provide better prefix to go back to original cwd","fromName":"Chris Packham","fromEmail":"judge.packham@gmail.com","sentAt":"2010-10-15T23:26:40Z","receivedAt":"2010-10-15T23:26:40Z","isPatch":false,"sender":{"key":"judge.packham@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155667?v=4"},"body":"From: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n\nWhen both GIT_DIR and GIT_WORK_TREE are set, if cwd is outside worktree,\nprefix (the one passed to every builtin commands) will be set to NULL,\nwhich means \"user stays at worktree topdir\".\n\nAs a consequence, command line arguments are supposed to be relative\nto worktree topdir, not current working directory. Not very intuitive.\nMoreover, output from such situation is (again) relative to worktree\ntopdir. Users are expected to understand that.\n\nThis patch allows builtin commands access to original cwd even if it's\noutside worktree, via cwd_to_worktree and worktree_to_cwd fields. As\nthe name implies, if you stay at original cwd, \"cd $(cwd_to_worktree)\"\nwould take you to worktree topdir and vice versa.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n builtin/rev-parse.c        |   10 ++++\n cache.h                    |    2 +\n setup.c                    |  108 ++++++++++++++++++++++++++++++++++++++++++--\n t/t1510-worktree-prefix.sh |   52 +++++++++++++++++++++\n 4 files changed, 168 insertions(+), 4 deletions(-)\n create mode 100755 t/t1510-worktree-prefix.sh\n\ndiff --git a/builtin/rev-parse.c b/builtin/rev-parse.c\nindex a5a1c86..525610e 100644\n--- a/builtin/rev-parse.c\n+++ b/builtin/rev-parse.c\n@@ -623,6 +623,16 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)\n \t\t\t\t\tputs(prefix);\n \t\t\t\tcontinue;\n \t\t\t}\n+\t\t\tif (!strcmp(arg, \"--cwd-to-worktree\")) {\n+\t\t\t\tif (startup_info->cwd_to_worktree)\n+\t\t\t\t\tputs(startup_info->cwd_to_worktree);\n+\t\t\t\tcontinue;\n+\t\t\t}\n+\t\t\tif (!strcmp(arg, \"--worktree-to-cwd\")) {\n+\t\t\t\tif (startup_info->worktree_to_cwd)\n+\t\t\t\t\tputs(startup_info->worktree_to_cwd);\n+\t\t\t\tcontinue;\n+\t\t\t}\n \t\t\tif (!strcmp(arg, \"--show-cdup\")) {\n \t\t\t\tconst char *pfx = prefix;\n \t\t\t\tif (!is_inside_work_tree()) {\ndiff --git a/cache.h b/cache.h\nindex 33decd9..c001272 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -1117,6 +1117,8 @@ const char *split_cmdline_strerror(int cmdline_errno);\n /* git.c */\n struct startup_info {\n \tint have_repository;\n+\tchar *cwd_to_worktree; /* chdir(\"this\"); from cwd would return to worktree */\n+\tchar *worktree_to_cwd; /* chdir(\"this\"); from worktree would return to cwd */\n };\n extern struct startup_info *startup_info;\n \ndiff --git a/setup.c b/setup.c\nindex a3b76de..413703b 100644\n--- a/setup.c\n+++ b/setup.c\n@@ -313,10 +313,109 @@ const char *read_gitfile_gently(const char *path)\n \treturn path;\n }\n \n+/*\n+ * Given \"foo/bar\" and \"hey/hello/world\", return \"../../hey/hello/world/\"\n+ * Either path1 or path2 can be NULL\n+ */\n+static char *make_path_to_path(const char *path1, const char *path2)\n+{\n+\tint nr_back = 0;\n+\tint i, pathlen = path2 ? strlen(path2) : 0;\n+\tchar *buf, *p;\n+\n+\tif (path1 && *path1) {\n+\t\tnr_back = 1;\n+\t\twhile (*path1) {\n+\t\t\tif (*path1 == '/')\n+\t\t\t\tnr_back++;\n+\t\t\tpath1++;\n+\t\t}\n+\t}\n+\n+\tif (!nr_back && !pathlen)\n+\t\treturn NULL;\n+\n+\tp = buf = xmalloc(3*nr_back + pathlen + 2); /* \"../\"^nr_back + path2 + '/' + NULL */\n+\tfor (i = 0; i < nr_back; i++) {\n+\t\tmemcpy(p, \"../\", 3);\n+\t\tp += 3;\n+\t}\n+\tif (pathlen) {\n+\t\tmemcpy(p, path2, pathlen);\n+\t\tp += pathlen;\n+\t\t*p++ = '/';\n+\t}\n+\t*p = '\\0';\n+\treturn buf;\n+}\n+\n+/*\n+ * Return a prefix if cwd inside worktree, or NULL otherwise.\n+ * Also fill startup_info struct.\n+ */\n+static const char *setup_prefix(const char *cwd)\n+{\n+\tconst char *worktree = get_git_work_tree();\n+\tint len = 0, cwd_len = strlen(cwd), worktree_len = strlen(worktree);\n+\n+\twhile (worktree[len] && worktree[len] == cwd[len])\n+\t\tlen++;\n+\n+\tif (!worktree[len] && !cwd[len]) {\n+\t\tif (startup_info) {\n+\t\t\tstartup_info->cwd_to_worktree = NULL;\n+\t\t\tstartup_info->worktree_to_cwd = NULL;\n+\t\t}\n+\t\treturn NULL;\n+\t}\n+\t/* get /foo/, not /foo/baa if /foo/baa1 and /foo/baa2 are given */\n+\telse if (worktree[len] && cwd[len]) {\n+\t\twhile (len && worktree[len] != '/')\n+\t\t\tlen--;\n+\t\tlen++;\n+\t}\n+\telse {\n+\t\tif (worktree[len]) {\n+\t\t\tif (worktree[len] != '/') {\n+\t\t\t\twhile (len && worktree[len] != '/')\n+\t\t\t\t\tlen--;\n+\t\t\t}\n+\t\t}\n+\t\telse {\n+\t\t\tif (cwd[len] != '/') {\n+\t\t\t\twhile (len && cwd[len] != '/')\n+\t\t\t\t\tlen--;\n+\t\t\t}\n+\t\t}\n+\t\tlen++;\t\t/* must be a slash here, skip it */\n+\t}\n+\n+\tif (len < cwd_len && len < worktree_len) {\n+\t\tif (startup_info) {\n+\t\t\tstartup_info->cwd_to_worktree = make_path_to_path(cwd+len, worktree+len);\n+\t\t\tstartup_info->worktree_to_cwd = make_path_to_path(worktree+len, cwd+len);\n+\t\t}\n+\t\treturn NULL;\n+\t}\n+\n+\tif (startup_info) {\n+\t\tif (len < cwd_len) { /* cwd inside worktree */\n+\t\t\tstartup_info->cwd_to_worktree = make_path_to_path(cwd+len, NULL);\n+\t\t\tstartup_info->worktree_to_cwd = make_path_to_path(NULL, cwd+len);\n+\t\t}\n+\t\telse {\n+\t\t\tstartup_info->cwd_to_worktree = make_path_to_path(NULL, worktree+len);\n+\t\t\tstartup_info->worktree_to_cwd = make_path_to_path(worktree+len, NULL);\n+\t\t}\n+\t}\n+\n+\treturn len < cwd_len ? cwd+len : NULL;\n+}\n+\n static const char *setup_explicit_git_dir(const char *gitdirenv,\n \t\t\t\tconst char *work_tree_env, int *nongit_ok)\n {\n-\tstatic char buffer[1024 + 1];\n+\tstatic char buffer[PATH_MAX];\n \tconst char *retval;\n \n \tif (PATH_MAX - 40 < strlen(gitdirenv))\n@@ -337,9 +436,10 @@ static const char *setup_explicit_git_dir(const char *gitdirenv,\n \t}\n \tif (check_repository_format_gently(nongit_ok))\n \t\treturn NULL;\n-\tretval = get_relative_cwd(buffer, sizeof(buffer) - 1,\n-\t\t\tget_git_work_tree());\n-\tif (!retval || !*retval)\n+\tif (!getcwd(buffer, sizeof(buffer)))\n+\t\tdie_errno(\"can't find the current directory\");\n+\tretval = setup_prefix(buffer);\n+\tif (!retval)\n \t\treturn NULL;\n \tset_git_dir(make_absolute_path(gitdirenv));\n \tif (chdir(work_tree_env) < 0)\ndiff --git a/t/t1510-worktree-prefix.sh b/t/t1510-worktree-prefix.sh\nnew file mode 100755\nindex 0000000..3839493\n--- /dev/null\n+++ b/t/t1510-worktree-prefix.sh\n@@ -0,0 +1,52 @@\n+#!/bin/sh\n+\n+test_description='test rev-parse --cwd-to-worktree and --worktree-to-cwd'\n+\n+. ./test-lib.sh\n+\n+test_expect_success 'setup' '\n+\tmkdir foo bar &&\n+\tmv .git foo &&\n+\tmkdir foo/bar &&\n+\tGIT_DIR=`pwd`/foo/.git &&\n+\tGIT_WORK_TREE=`pwd`/foo &&\n+\texport GIT_DIR GIT_WORK_TREE\n+'\n+\n+test_expect_success 'at root' '\n+\t(\n+\tcd foo &&\n+\tgit rev-parse --cwd-to-worktree --worktree-to-cwd >result &&\n+\t: >expected &&\n+\ttest_cmp expected result\n+\t)\n+'\n+\n+test_expect_success 'cwd inside worktree' '\n+\t(\n+\tcd foo/bar &&\n+\tgit rev-parse --cwd-to-worktree --worktree-to-cwd >result &&\n+\techo ../ >expected &&\n+\techo bar/ >>expected &&\n+\ttest_cmp expected result\n+\t)\n+'\n+\n+test_expect_success 'cwd outside worktree' '\n+\tgit rev-parse --cwd-to-worktree --worktree-to-cwd >result &&\n+\techo foo/ >expected &&\n+\techo ../ >>expected &&\n+\ttest_cmp expected result\n+'\n+\n+test_expect_success 'cwd outside worktree (2)' '\n+\t(\n+\tcd bar &&\n+\tgit rev-parse --cwd-to-worktree --worktree-to-cwd >result &&\n+\techo ../foo/ >expected &&\n+\techo ../bar/ >>expected &&\n+\ttest_cmp expected result\n+\t)\n+'\n+\n+test_done\n-- \n1.7.3.1\n"},{"id":"153610","messageId":"1287185204-843-3-git-send-email-judge.packham@gmail.com","threadId":"25457","inReplyTo":"1287185204-843-1-git-send-email-judge.packham@gmail.com","subject":"[RFC/PATCHv2 2/5] grep: output the path from cwd to worktree","fromName":"Chris Packham","fromEmail":"judge.packham@gmail.com","sentAt":"2010-10-15T23:26:41Z","receivedAt":"2010-10-15T23:26:41Z","isPatch":false,"sender":{"key":"judge.packham@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155667?v=4"},"body":"If we have detected that our worktree is not the cwd.\n\nSigned-off-by: Chris Packham <judge.packham@gmail.com>\n---\n builtin/grep.c |    4 ++++\n 1 files changed, 4 insertions(+), 0 deletions(-)\n\ndiff --git a/builtin/grep.c b/builtin/grep.c\nindex da32f3d..a51eb2c 100644\n--- a/builtin/grep.c\n+++ b/builtin/grep.c\n@@ -531,6 +531,8 @@ static int grep_file(struct grep_opt *opt, const char *filename)\n \tstruct strbuf buf = STRBUF_INIT;\n \tchar *name;\n \n+\tif (startup_info->cwd_to_worktree)\n+\t\tstrbuf_addstr(&buf,startup_info->cwd_to_worktree);\n \tif (opt->relative && opt->prefix_length)\n \t\tquote_path_relative(filename, -1, &buf, opt->prefix);\n \telse\n@@ -646,6 +648,8 @@ static int grep_tree(struct grep_opt *opt, const char **paths,\n \twhile (tree_entry(tree, &entry)) {\n \t\tint te_len = tree_entry_len(entry.path, entry.sha1);\n \t\tpathbuf.len = len;\n+\t\tif (startup_info->cwd_to_worktree)\n+\t\t\tstrbuf_addstr(&pathbuf,startup_info->cwd_to_worktree);\n \t\tstrbuf_add(&pathbuf, entry.path, te_len);\n \n \t\tif (S_ISDIR(entry.mode))\n-- \n1.7.3.1\n"},{"id":"153613","messageId":"1287185204-843-4-git-send-email-judge.packham@gmail.com","threadId":"25457","inReplyTo":"1287185204-843-1-git-send-email-judge.packham@gmail.com","subject":"[RFC/PATCHv2 3/5] grep_cache: check pathspec first","fromName":"Chris Packham","fromEmail":"judge.packham@gmail.com","sentAt":"2010-10-15T23:26:42Z","receivedAt":"2010-10-15T23:26:42Z","isPatch":false,"sender":{"key":"judge.packham@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155667?v=4"},"body":"This makes grep_cache consistent with grep_tree.\n\nSigned-off-by: Chris Packham <judge.packham@gmail.com>\n---\nIn addition to consistency, moving the pathspec check up allows the\naddition of a simple if clause for submodules later.\n\n builtin/grep.c |    4 ++--\n 1 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/grep.c b/builtin/grep.c\nindex a51eb2c..251c4e7 100644\n--- a/builtin/grep.c\n+++ b/builtin/grep.c\n@@ -595,10 +595,10 @@ static int grep_cache(struct grep_opt *opt, const char **paths, int cached)\n \n \tfor (nr = 0; nr < active_nr; nr++) {\n \t\tstruct cache_entry *ce = active_cache[nr];\n-\t\tif (!S_ISREG(ce->ce_mode))\n-\t\t\tcontinue;\n \t\tif (!pathspec_matches(paths, ce->name, opt->max_depth))\n \t\t\tcontinue;\n+\t\tif (!S_ISREG(ce->ce_mode))\n+\t\t\tcontinue;\n \t\t/*\n \t\t * If CE_VALID is on, we assume worktree file and its cache entry\n \t\t * are identical, even if worktree file has been modified, so use\n-- \n1.7.3.1\n"},{"id":"153612","messageId":"1287185204-843-5-git-send-email-judge.packham@gmail.com","threadId":"25457","inReplyTo":"1287185204-843-1-git-send-email-judge.packham@gmail.com","subject":"[RFC/PATCHv2 4/5] add test for git grep --recursive","fromName":"Chris Packham","fromEmail":"judge.packham@gmail.com","sentAt":"2010-10-15T23:26:43Z","receivedAt":"2010-10-15T23:26:43Z","isPatch":false,"sender":{"key":"judge.packham@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155667?v=4"},"body":"Signed-off-by: Chris Packham <judge.packham@gmail.com>\n---\n t/t7820-grep-recursive.sh |  183 +++++++++++++++++++++++++++++++++++++++++++++\n 1 files changed, 183 insertions(+), 0 deletions(-)\n create mode 100644 t/t7820-grep-recursive.sh\n\ndiff --git a/t/t7820-grep-recursive.sh b/t/t7820-grep-recursive.sh\nnew file mode 100644\nindex 0000000..3c386b4\n--- /dev/null\n+++ b/t/t7820-grep-recursive.sh\n@@ -0,0 +1,183 @@\n+#!/bin/sh\n+#\n+# Copyright (c) 2010 Chris Packham\n+#\n+\n+test_description='git grep --recursive test\n+\n+This test checks the ability of git grep to search within submodules when told\n+to do so with the --recursive option'\n+\n+. ./test-lib.sh\n+\n+restore_test_defaults()\n+{\n+\tunset GIT_SUPER_REFNAME\n+}\n+\n+test_expect_success 'setup' '\n+\tcat >t <<-\\EOF &&\n+\tone two three\n+\tfour five six\n+\tseven eight nine\n+\tEOF\n+\tgit add t &&\n+\tgit commit -m \"initial commit\"\n+'\n+submodurl=$TRASH_DIRECTORY\n+\n+test_expect_success 'setup submodules' '\n+\tfor mod in submodule1 submodule2 submodule3 submodule4 submodule5; do\n+\t\tgit submodule add \"$submodurl\" $mod &&\n+\t\tgit submodule init $mod\n+\tdone &&\n+\tgit commit -m \"setup submodules for test\"\n+'\n+\n+test_expect_success 'update data in each submodule' '\n+\tfor n in 1 2 3 4 5; do\n+\t\t(cd submodule$n &&\n+\t\t\tsed -i \"s/^four.*/& #$n/\" t &&\n+\t\t\tgit commit -a -m\"update\") &&\n+\t\tgit add submodule$n\n+\tdone &&\n+\tgit commit -m \"update data in each submodule\"\n+'\n+\n+test_expect_success 'non-recursive grep in base' '\n+\tcat >expected <<-\\EOF &&\n+\tt:four five six\n+\tEOF\n+\tgit grep \"five\" >actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'submodule-ref option' '\n+\tcat >expected <<-\\EOF &&\n+\tbar:t:four five six\n+\tEOF\n+\tGIT_SUPER_REFNAME=bar &&\n+\texport GIT_SUPER_REFNAME &&\n+\tgit grep \"five\" master >actual &&\n+\ttest_cmp expected actual &&\n+\trestore_test_defaults\n+'\n+\n+test_expect_success 'non-recursive grep in submodule' '\n+\t(\n+\t\tcd submodule1 &&\n+\t\tcat >expected <<-\\EOF &&\n+\t\tt:four five six #1\n+\t\tEOF\n+\t\tgit grep \"five\" >actual &&\n+\t\ttest_cmp expected actual\n+\t)\n+'\n+\n+test_expect_success 'recursive grep' '\n+\tcat >expected <<-\\EOF &&\n+\tsubmodule1/t:four five six #1\n+\tsubmodule2/t:four five six #2\n+\tsubmodule3/t:four five six #3\n+\tsubmodule4/t:four five six #4\n+\tsubmodule5/t:four five six #5\n+\tt:four five six\n+\tEOF\n+\tgit grep --recursive \"five\" >actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'recursive grep (with -n)' '\n+\tcat >expected <<-\\EOF &&\n+\tsubmodule1/t:2:four five six #1\n+\tsubmodule2/t:2:four five six #2\n+\tsubmodule3/t:2:four five six #3\n+\tsubmodule4/t:2:four five six #4\n+\tsubmodule5/t:2:four five six #5\n+\tt:2:four five six\n+\tEOF\n+\tgit grep --recursive -n \"five\" >actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'recursive grep (with -l)' '\n+\tcat >expected <<-\\EOF &&\n+\tsubmodule1/t\n+\tsubmodule2/t\n+\tsubmodule3/t\n+\tsubmodule4/t\n+\tsubmodule5/t\n+\tt\n+\tEOF\n+\tgit grep --recursive -l \"five\" >actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'recursive grep (with --or)' '\n+\tcat >expected <<-\\EOF &&\n+\tsubmodule1/t:one two three\n+\tsubmodule1/t:four five six #1\n+\tsubmodule2/t:one two three\n+\tsubmodule2/t:four five six #2\n+\tsubmodule3/t:one two three\n+\tsubmodule3/t:four five six #3\n+\tsubmodule4/t:one two three\n+\tsubmodule4/t:four five six #4\n+\tsubmodule5/t:one two three\n+\tsubmodule5/t:four five six #5\n+\tt:one two three\n+\tt:four five six\n+\tEOF\n+\tgit grep --recursive \\( -e \"five\" --or -e \"two\" \\) >actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'recursive grep (with --and --not)' '\n+\tcat >expected <<-\\EOF &&\n+\tsubmodule2/t:four five six #2\n+\tsubmodule3/t:four five six #3\n+\tsubmodule4/t:four five six #4\n+\tsubmodule5/t:four five six #5\n+\tt:four five six\n+\tEOF\n+\tgit grep --recursive \\( -e \"five\" --and --not -e \"#1\" \\) >actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'recursive grep with refspec' '\n+\tcat >expected <<-\\EOF &&\n+\tmaster:submodule1/t:four five six #1\n+\tmaster:submodule2/t:four five six #2\n+\tmaster:submodule3/t:four five six #3\n+\tmaster:submodule4/t:four five six #4\n+\tmaster:submodule5/t:four five six #5\n+\tmaster:t:four five six\n+\tEOF\n+\tgit grep --recursive five master >actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'recursive grep with pathspec' '\n+\tcat >expected <<-\\EOF &&\n+\tsubmodule2/t:four five six #2\n+\tEOF\n+\tgit grep --recursive five -- submodule2 >actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'recursive grep with pathspec and refspec' '\n+\tcat >expected <<-\\EOF &&\n+\tmaster:submodule2/t:four five six #2\n+\tEOF\n+\tgit grep --recursive five master -- submodule2 >actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_failure 'recursive grep with --max-depth' '\n+\tcat >expected <<-\\EOF &&\n+\tt:four five six\n+\tEOF\n+\tgit grep --recursive --max-depth=1 five  >actual &&\n+\ttest_cmp expected actual\n+'\n+test_done\n-- \n1.7.3.1\n"},{"id":"153611","messageId":"1287185204-843-6-git-send-email-judge.packham@gmail.com","threadId":"25457","inReplyTo":"1287185204-843-1-git-send-email-judge.packham@gmail.com","subject":"[RFC/PATCHv2 5/5] grep: add support for grepping in submodules","fromName":"Chris Packham","fromEmail":"judge.packham@gmail.com","sentAt":"2010-10-15T23:26:44Z","receivedAt":"2010-10-15T23:26:44Z","isPatch":false,"sender":{"key":"judge.packham@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155667?v=4"},"body":"When the --recurse-submodules option is given git grep will search in\nsubmodules as they are encountered.\n\nSigned-off-by: Chris Packham <judge.packham@gmail.com>\n---\nThe refspec from the super project is passed as an environment variable\nalong with the GIT_DIR and GIT_WORK_TREE. We re-build a command line for\nthe sub-process grep which works for the basic cases but I need to add\nsome more complex tests to ensure everything gets passed through\ncorrectly.\n    \nAt the moment I don't alter the pathspec or max-depth option but I'll do\nso in a future re-roll.\n\n builtin/grep.c |  103 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-\n grep.h         |    1 +\n 2 files changed, 102 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/grep.c b/builtin/grep.c\nindex 251c4e7..7bcdf05 100644\n--- a/builtin/grep.c\n+++ b/builtin/grep.c\n@@ -587,6 +587,91 @@ static void run_pager(struct grep_opt *opt, const char *prefix)\n \tfree(argv);\n }\n \n+static int pattern_list_to_argv(struct grep_opt *opt, const char **argv, int len)\n+{\n+\tint i = 0;\n+\tstruct grep_pat *p = opt->pattern_list;\n+\twhile(p) {\n+\t\tif (i > len)\n+\t\t\tdie(\"grep: not enough space for subprocess args\");\n+\t\tif (p->token == GREP_PATTERN)\n+\t\t\targv[i++] = \"-e\";\n+\t\targv[i++] = p->pattern;\n+\t\tp = p->next;\n+\t}\n+\treturn i;\n+}\n+\n+static const char **create_sub_grep_argv(struct grep_opt *opt,\n+\t\tconst char *path, const char *sha1, const char *tree_name)\n+{\n+\t#define NUM_ARGS 10\n+\tstruct strbuf buf = STRBUF_INIT;\n+\tconst char **argv;\n+\tint i = 0;\n+\n+\targv = xcalloc(NUM_ARGS, sizeof(const char *));\n+\targv[i++] = \"grep\";\n+\n+\tif (opt->linenum)\n+\t\targv[i++] = \"-n\";\n+\tif (opt->invert)\n+\t\targv[i++] = \"-v\";\n+\tif (opt->ignore_case)\n+\t\targv[i++] = \"-i\";\n+\tif (opt->count)\n+\t\targv[i++] = \"-c\";\n+\tif (opt->name_only)\n+\t\targv[i++] = \"-l\";\n+\tif (opt->recurse_submodules)\n+\t\targv[i++] = \"--recursive\";\n+\n+\ti += pattern_list_to_argv(opt, &argv[i], NUM_ARGS-(i+1));\n+\tif (sha1) {\n+\t\targv[i++] = sha1;\n+\t}\n+\targv[i++] = NULL;\n+\n+\tstrbuf_release(&buf);\n+\treturn argv;\n+}\n+\n+static int grep_submodule(struct grep_opt *opt, const char *path,\n+\t\t\t  const char *sha1, const char *tree_name)\n+{\n+\tstruct strbuf buf = STRBUF_INIT;\n+\tstruct strbuf pre_buf = STRBUF_INIT;\n+\tstruct child_process cp;\n+\tconst char **argv = create_sub_grep_argv(opt, path, sha1, tree_name);\n+\tconst char *git_dir;\n+\tint hit = 0;\n+\tmemset(&cp, 0, sizeof(cp));\n+\n+\tstrbuf_addf(&buf, \"%s/.git\", path);\n+\tgit_dir = read_gitfile_gently(buf.buf);\n+\tif (!git_dir)\n+\t\tgit_dir = buf.buf;\n+\tif (!is_directory(git_dir))\n+\t\tgoto out_free;\n+\n+\tsetenv(\"GIT_SUPER_REFNAME\", tree_name, 1);\n+\tsetenv(GIT_DIR_ENVIRONMENT, git_dir, 1);\n+\tsetenv(GIT_WORK_TREE_ENVIRONMENT, path, 1);\n+\tcp.argv = argv;\n+\tcp.git_cmd = 1;\n+\tcp.no_stdin = 1;\n+\tif (run_command(&cp) == 0)\n+\t\thit = 1;\n+out_free:\n+\tunsetenv(\"GIT_SUPER_REFNAME\");\n+\tunsetenv(GIT_DIR_ENVIRONMENT);\n+\tunsetenv(GIT_WORK_TREE_ENVIRONMENT);\n+\tfree(argv);\n+\tstrbuf_release(&buf);\n+\tstrbuf_release(&pre_buf);\n+\treturn hit;\n+}\n+\n static int grep_cache(struct grep_opt *opt, const char **paths, int cached)\n {\n \tint hit = 0;\n@@ -597,6 +682,10 @@ static int grep_cache(struct grep_opt *opt, const char **paths, int cached)\n \t\tstruct cache_entry *ce = active_cache[nr];\n \t\tif (!pathspec_matches(paths, ce->name, opt->max_depth))\n \t\t\tcontinue;\n+\t\tif (S_ISGITLINK(ce->ce_mode) && opt->recurse_submodules) {\n+\t\t\thit |= grep_submodule(opt, ce->name, NULL, NULL);\n+\t\t\tcontinue;\n+\t\t}\n \t\tif (!S_ISREG(ce->ce_mode))\n \t\t\tcontinue;\n \t\t/*\n@@ -634,11 +723,16 @@ static int grep_tree(struct grep_opt *opt, const char **paths,\n \tchar *down;\n \tint tn_len = strlen(tree_name);\n \tstruct strbuf pathbuf;\n+\tconst char *refname = getenv(\"GIT_SUPER_REFNAME\");\n+\tint rn_len = refname ? strlen(refname) : 0;\n \n-\tstrbuf_init(&pathbuf, PATH_MAX + tn_len);\n+\tstrbuf_init(&pathbuf, PATH_MAX + MAX(tn_len, rn_len));\n \n \tif (tn_len) {\n-\t\tstrbuf_add(&pathbuf, tree_name, tn_len);\n+\t\tif (refname)\n+\t\t\tstrbuf_add(&pathbuf, refname, rn_len);\n+\t\telse\n+\t\t\tstrbuf_add(&pathbuf, tree_name, tn_len);\n \t\tstrbuf_addch(&pathbuf, ':');\n \t\ttn_len = pathbuf.len;\n \t}\n@@ -664,6 +758,9 @@ static int grep_tree(struct grep_opt *opt, const char **paths,\n \t\t\t;\n \t\telse if (S_ISREG(entry.mode))\n \t\t\thit |= grep_sha1(opt, entry.sha1, pathbuf.buf, tn_len);\n+\t\telse if (S_ISGITLINK(entry.mode) && opt->recurse_submodules)\n+\t\t\thit |= grep_submodule(opt, entry.path,\n+\t\t\t\tsha1_to_hex(entry.sha1), tree_name);\n \t\telse if (S_ISDIR(entry.mode)) {\n \t\t\tenum object_type type;\n \t\t\tstruct tree_desc sub;\n@@ -931,6 +1028,8 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n \t\t\t    \"allow calling of grep(1) (ignored by this build)\"),\n \t\t{ OPTION_CALLBACK, 0, \"help-all\", &options, NULL, \"show usage\",\n \t\t  PARSE_OPT_HIDDEN | PARSE_OPT_NOARG, help_callback },\n+\t\tOPT_BOOLEAN(0, \"recursive\", &opt.recurse_submodules,\n+\t\t\t\"recurse into submodules\"),\n \t\tOPT_END()\n \t};\n \ndiff --git a/grep.h b/grep.h\nindex 06621fe..d5e2e11 100644\n--- a/grep.h\n+++ b/grep.h\n@@ -101,6 +101,7 @@ struct grep_opt {\n \tunsigned post_context;\n \tunsigned last_shown;\n \tint show_hunk_mark;\n+\tint recurse_submodules;\n \tvoid *priv;\n \n \tvoid (*output)(struct grep_opt *opt, const void *data, size_t size);\n-- \n1.7.3.1\n"},{"id":"153629","messageId":"4CB9CAD3.7060109@web.de","threadId":"25457","inReplyTo":"1287185204-843-1-git-send-email-judge.packham@gmail.com","subject":"Re: [RGC/PATCHv2] grep: submodule support","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2010-10-16T15:54:59Z","receivedAt":"2010-10-16T15:54:59Z","isPatch":false,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"Am 16.10.2010 01:26, schrieb Chris Packham:\n> Here is my updated grep support for submodules series.\n\nThanks Chris and Duy, I added this series to my git-submod-enhancements\nrepo on github and will give it some testing next week.\n\n> Chris Packham (4):\n>       grep: output the path from cwd to worktree\n>       grep_cache: check pathspec first\n>       add test for git grep --recursive\n\nHmm, is it useful to add tests before adding the feature tested? I\nthought it makes more sense to add them in the same commit or after\nadding what is tested (e.g. to reduce the noise when bisecting\nlater). Or am I missing something here?\n\n(And by the way: the test script should be executable)\n\n>       grep: add support for grepping in submodules\n\nNit: The commit message (5/5) talks about the old option name\n\"--recurse-submodules\" instead of the \"--recursive\" the patch\nis adding.\n\n> Nguyễn Thái Ngọc Duy (1):\n>       worktree: provide better prefix to go back to original cwd\n"},{"id":"153641","messageId":"20101016184259.GB30457@burratino","threadId":"25457","inReplyTo":"1287185204-843-2-git-send-email-judge.packham@gmail.com","subject":"Re: [RFC/PATCHv2 1/5] worktree: provide better prefix to go back to original cwd","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-10-16T18:42:59Z","receivedAt":"2010-10-16T18:42:59Z","isPatch":false,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi,\n\nChris Packham wrote:\n\n> From: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n> \n> When both GIT_DIR and GIT_WORK_TREE are set, if cwd is outside worktree,\n> prefix (the one passed to every builtin commands) will be set to NULL,\n> which means \"user stays at worktree topdir\".\n> \n> As a consequence, command line arguments are supposed to be relative\n> to worktree topdir, not current working directory. Not very intuitive.\n\nThanks.  More detailed history for this patch:\n\n - v0: http://thread.gmane.org/gmane.comp.version-control.git/157599/focus=157601\n - v1: http://thread.gmane.org/gmane.comp.version-control.git/158287\n - v2: http://thread.gmane.org/gmane.comp.version-control.git/158369\n\nAny thoughts about the previous questions?\n\n> --- a/cache.h\n> +++ b/cache.h\n> @@ -1117,6 +1117,8 @@ const char *split_cmdline_strerror(int cmdline_errno);\n>  /* git.c */\n>  struct startup_info {\n>  \tint have_repository;\n> +\tchar *cwd_to_worktree; /* chdir(\"this\"); from cwd would return to worktree */\n> +\tchar *worktree_to_cwd; /* chdir(\"this\"); from worktree would return to cwd */\n\ne.g. I still find these comments hard to understand.\n\n> --- a/setup.c\n> +++ b/setup.c\n> @@ -313,10 +313,109 @@ const char *read_gitfile_gently(const char *path)\n>  \treturn path;\n>  }\n>  \n> +/*\n> + * Given \"foo/bar\" and \"hey/hello/world\", return \"../../hey/hello/world/\"\n> + * Either path1 or path2 can be NULL\n> + */\n> +static char *make_path_to_path(const char *path1, const char *path2)\n> +{\n> +\tint nr_back = 0;\n> +\tint i, pathlen = path2 ? strlen(path2) : 0;\n> +\tchar *buf, *p;\n> +\n> +\tif (path1 && *path1) {\n> +\t\tnr_back = 1;\n> +\t\twhile (*path1) {\n> +\t\t\tif (*path1 == '/')\n> +\t\t\t\tnr_back++;\n\nThis still assumes Unix-style path separators.  Is that okay?  (The\nanswer could be yes; it just seems useful to document the assumptions...)\n\n> +/*\n> + * Return a prefix if cwd inside worktree, or NULL otherwise.\n> + * Also fill startup_info struct.\n> + */\n> +static const char *setup_prefix(const char *cwd)\n> +{\n> +\tconst char *worktree = get_git_work_tree();\n> +\tint len = 0, cwd_len = strlen(cwd), worktree_len = strlen(worktree);\n> +\n> +\twhile (worktree[len] && worktree[len] == cwd[len])\n> +\t\tlen++;\n> +\n> +\tif (!worktree[len] && !cwd[len]) {\n> +\t\tif (startup_info) {\n> +\t\t\tstartup_info->cwd_to_worktree = NULL;\n> +\t\t\tstartup_info->worktree_to_cwd = NULL;\n> +\t\t}\n> +\t\treturn NULL;\n> +\t}\n> +\t/* get /foo/, not /foo/baa if /foo/baa1 and /foo/baa2 are given */\n> +\telse if (worktree[len] && cwd[len]) {\n\nStyle: use cuddled braces.\n\n\t} else if (worktree[len] && cwd[len]) {\n\t\t/*\n\t\t * The result should be /foo/, not /foo/baa, when\n\t\t * worktree is /foo/baa1 and cwd is /foo/baa2.\n\t\t */\n\n> +\t\twhile (len && worktree[len] != '/')\n> +\t\t\tlen--;\n> +\t\tlen++;\n> +\t}\n> +\telse {\n\nLikewise.\n\n> +\t\tif (worktree[len]) {\n> +\t\t\tif (worktree[len] != '/') {\n> +\t\t\t\twhile (len && worktree[len] != '/')\n> +\t\t\t\t\tlen--;\n> +\t\t\t}\n> +\t\t}\n> +\t\telse {\n\nLikewise.\n\n> +\t\t\tif (cwd[len] != '/') {\n> +\t\t\t\twhile (len && cwd[len] != '/')\n> +\t\t\t\t\tlen--;\n> +\t\t\t}\n\nThis is repetitive.  Why is the if necessary?\n\n[...]\n>  static const char *setup_explicit_git_dir(const char *gitdirenv,\n>  \t\t\t\tconst char *work_tree_env, int *nongit_ok)\n>  {\n> -\tstatic char buffer[1024 + 1];\n> +\tstatic char buffer[PATH_MAX];\n\nUnexplained.\n\n[...]\n> --- /dev/null\n> +++ b/t/t1510-worktree-prefix.sh\n> @@ -0,0 +1,52 @@\n[...]\n> +test_expect_success 'at root' '\n> +\t(\n> +\tcd foo &&\n> +\tgit rev-parse --cwd-to-worktree --worktree-to-cwd >result &&\n> +\t: >expected &&\n> +\ttest_cmp expected result\n> +\t)\n> +'\n\nIt is clearer where a subshell begins and ends if it is indented.  The\nusual style in git shell scripts is to omit the \":\" in\n\n\t>empty\n\ncommands.  So maybe:\n\n\t(\n\t\tcd foo &&\n\t\tgit rev-parse --cwd-to-worktree --worktree-to-cwd >result\n\t) &&\n\t>expected &&\n\ttest_cmp expected foo/result\n\nSorry, a bit grumpy today.  Still, hope that helps,\nJonathan\n"},{"id":"153649","messageId":"4CBA5BC7.9090707@gmail.com","threadId":"25457","inReplyTo":"4CB9CAD3.7060109@web.de","subject":"Re: [RGC/PATCHv2] grep: submodule support","fromName":"Chris Packham","fromEmail":"judge.packham@gmail.com","sentAt":"2010-10-17T02:13:27Z","receivedAt":"2010-10-17T02:13:27Z","isPatch":false,"sender":{"key":"judge.packham@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155667?v=4"},"body":"On 16/10/10 08:54, Jens Lehmann wrote:\n> Am 16.10.2010 01:26, schrieb Chris Packham:\n>> Here is my updated grep support for submodules series.\n> \n> Thanks Chris and Duy, I added this series to my git-submod-enhancements\n> repo on github and will give it some testing next week.\n> \n>> Chris Packham (4):\n>>       grep: output the path from cwd to worktree\n>>       grep_cache: check pathspec first\n>>       add test for git grep --recursive\n> \n> Hmm, is it useful to add tests before adding the feature tested? I\n> thought it makes more sense to add them in the same commit or after\n> adding what is tested (e.g. to reduce the noise when bisecting\n> later). Or am I missing something here?\n\nThat's just reflects my personal preference to write the tests first\n(influenced by a former colleague who was keen on extreme\nprogramming/test driven development). Eventually these can be swapped or\nsquashed as needed, for now it's probably easier to review them separately.\n\n> \n> (And by the way: the test script should be executable)\n\nOK will fix.\n\n> \n>>       grep: add support for grepping in submodules\n> \n> Nit: The commit message (5/5) talks about the old option name\n> \"--recurse-submodules\" instead of the \"--recursive\" the patch\n> is adding.\n\nYeah I just noticed that. I'll update the commit message.\n\n> \n>> Nguyễn Thái Ngọc Duy (1):\n>>       worktree: provide better prefix to go back to original cwd\n"},{"id":"153652","messageId":"4CBA63E2.8030502@gmail.com","threadId":"25457","inReplyTo":"20101016184259.GB30457@burratino","subject":"Re: [RFC/PATCHv2 1/5] worktree: provide better prefix to go back to original cwd","fromName":"Chris Packham","fromEmail":"judge.packham@gmail.com","sentAt":"2010-10-17T02:48:02Z","receivedAt":"2010-10-17T02:48:02Z","isPatch":false,"sender":{"key":"judge.packham@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155667?v=4"},"body":"On 16/10/10 11:42, Jonathan Nieder wrote:\n> Hi,\n> \n> Chris Packham wrote:\n> \n>> From: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n>>\n>> When both GIT_DIR and GIT_WORK_TREE are set, if cwd is outside worktree,\n>> prefix (the one passed to every builtin commands) will be set to NULL,\n>> which means \"user stays at worktree topdir\".\n>>\n>> As a consequence, command line arguments are supposed to be relative\n>> to worktree topdir, not current working directory. Not very intuitive.\n> \n> Thanks.  More detailed history for this patch:\n> \n>  - v0: http://thread.gmane.org/gmane.comp.version-control.git/157599/focus=157601\n>  - v1: http://thread.gmane.org/gmane.comp.version-control.git/158287\n>  - v2: http://thread.gmane.org/gmane.comp.version-control.git/158369\n> \n\nI think I must have missed v2. I was playing around with my gmail\nfilters around that time so I could have missed them. Actually now I've\nfound the message it's missing the last 'm' in gmail.com. I'll grab the\nlatest patch and give it a test when I get a chance.\n\n> Any thoughts about the previous questions?\n> \n\nI haven't caught up on the newest thread so no great revelations. Except\nthat for the grep submodules use-case we can assume that the worktree\nwill be a subdirectory of the cwd. I don't think we want to limit\nourselves to that one use-case.\n"},{"id":"153663","messageId":"20101017094524.GA6034@do","threadId":"25457","inReplyTo":"4CBA63E2.8030502@gmail.com","subject":"Re: [RFC/PATCHv2 1/5] worktree: provide better prefix to go back to original cwd","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2010-10-17T10:01:38Z","receivedAt":"2010-10-17T10:01:38Z","isPatch":false,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Sat, Oct 16, 2010 at 07:48:02PM -0700, Chris Packham wrote:\n> On 16/10/10 11:42, Jonathan Nieder wrote:\n> > Hi,\n> > \n> > Chris Packham wrote:\n> > \n> >> From: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n> >>\n> >> When both GIT_DIR and GIT_WORK_TREE are set, if cwd is outside worktree,\n> >> prefix (the one passed to every builtin commands) will be set to NULL,\n> >> which means \"user stays at worktree topdir\".\n> >>\n> >> As a consequence, command line arguments are supposed to be relative\n> >> to worktree topdir, not current working directory. Not very intuitive.\n> > \n> > Thanks.  More detailed history for this patch:\n> > \n> >  - v0: http://thread.gmane.org/gmane.comp.version-control.git/157599/focus=157601\n> >  - v1: http://thread.gmane.org/gmane.comp.version-control.git/158287\n> >  - v2: http://thread.gmane.org/gmane.comp.version-control.git/158369\n> > \n> \n> I think I must have missed v2. I was playing around with my gmail\n> filters around that time so I could have missed them. Actually now I've\n> found the message it's missing the last 'm' in gmail.com. I'll grab the\n> latest patch and give it a test when I get a chance.\n\nI missed the last \"m\" in your email address. That's why v2 never reached you.\nI thought I sent you an email but probably forgot it.\n\nAnyway v2 does not work if worktree and cwd are on different Windows drives.\nThis on top should fix it:\n\n---8<---\ndiff --git a/setup.c b/setup.c\nindex 2389a9e..35d2691 100644\n--- a/setup.c\n+++ b/setup.c\n@@ -371,12 +371,8 @@ static const char *setup_prefix(const char *cwd)\n \t}\n \t/* get /foo/, not /foo/baa if /foo/baa1 and /foo/baa2 are given */\n \telse if (worktree[len] && cwd[len]) {\n-\t\twhile (len && !is_dir_sep(worktree[len]))\n-\t\t\tlen--;\n-\t\tlen++;\n-\n \t\t/* Worktree and cwd are on different drives? */\n-\t\tif (len == 3 && has_dos_drive_prefix(cwd)) {\n+\t\tif (!len && has_dos_drive_prefix(cwd)) {\n \t\t\tif (startup_info) {\n \t\t\t\t/* make_path_to_path will add the trailing slash */\n \t\t\t\tstartup_info->cwd_to_worktree = make_path_to_path(NULL, worktree);\n@@ -384,6 +380,10 @@ static const char *setup_prefix(const char *cwd)\n \t\t\t}\n \t\t\treturn NULL;\n \t\t}\n+\n+\t\twhile (len && !is_dir_sep(worktree[len]))\n+\t\t\tlen--;\n+\t\tlen++;\n \t}\n \telse {\n \t\tif (worktree[len]) {\n---8<---\n\n> \n> > Any thoughts about the previous questions?\n> > \n> \n> I haven't caught up on the newest thread so no great revelations. Except\n> that for the grep submodules use-case we can assume that the worktree\n> will be a subdirectory of the cwd. I don't think we want to limit\n> ourselves to that one use-case.\n\nWhile at it, have you thought of support --recursive and\n--full-tree [1]? There are issues with --full-tree and prefixes [2],\nwhich is why it is dropped but I think it's a good idea.\n\n--full-tree disregards where you stand and greps in whole repo. In a\nrepo with submodules, that would mean grep the supermodule and all\nsubmodules regardless where you stand, even if you stand in a\nsubmodule.\n\n[1] http://mid.gmane.org/7vk4xggv27.fsf@alter.siamese.dyndns.org\n[2] http://mid.gmane.org/7vskaqptvj.fsf@alter.siamese.dyndns.org\n-- \nDuy\n"},{"id":"153665","messageId":"AANLkTikSukNCkcx4RYRuzzKU8T64mT0boMDqoNBrWW73@mail.gmail.com","threadId":"25457","inReplyTo":"1287185204-843-6-git-send-email-judge.packham@gmail.com","subject":"Re: [RFC/PATCHv2 5/5] grep: add support for grepping in submodules","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2010-10-17T10:28:06Z","receivedAt":"2010-10-17T10:28:06Z","isPatch":false,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Sat, Oct 16, 2010 at 6:26 AM, Chris Packham <judge.packham@gmail.com> wrote:\n> +static int grep_submodule(struct grep_opt *opt, const char *path,\n> +                         const char *sha1, const char *tree_name)\n> +{\n> +       struct strbuf buf = STRBUF_INIT;\n> +       struct strbuf pre_buf = STRBUF_INIT;\n> +       struct child_process cp;\n> +       const char **argv = create_sub_grep_argv(opt, path, sha1, tree_name);\n\nCan we just save sha1 in a env variable and drop this argv rewrite?\n\n> +       const char *git_dir;\n> +       int hit = 0;\n> +       memset(&cp, 0, sizeof(cp));\n> +\n> +       strbuf_addf(&buf, \"%s/.git\", path);\n> +       git_dir = read_gitfile_gently(buf.buf);\n> +       if (!git_dir)\n> +               git_dir = buf.buf;\n> +       if (!is_directory(git_dir))\n> +               goto out_free;\n> +\n> +       setenv(\"GIT_SUPER_REFNAME\", tree_name, 1);\n> +       setenv(GIT_DIR_ENVIRONMENT, git_dir, 1);\n> +       setenv(GIT_WORK_TREE_ENVIRONMENT, path, 1);\n\ncp.env can be used to set these variables\n\n> +       cp.argv = argv;\n> +       cp.git_cmd = 1;\n> +       cp.no_stdin = 1;\n\nI think you stll need \"cp.dir = path;\" here because the setup routines\nwon't do that for you. But I need to check/test that.\n\n> +       if (run_command(&cp) == 0)\n> +               hit = 1;\n> +out_free:\n> +       unsetenv(\"GIT_SUPER_REFNAME\");\n> +       unsetenv(GIT_DIR_ENVIRONMENT);\n> +       unsetenv(GIT_WORK_TREE_ENVIRONMENT);\n> +       free(argv);\n> +       strbuf_release(&buf);\n> +       strbuf_release(&pre_buf);\n> +       return hit;\n> +}\n> +\n>  static int grep_cache(struct grep_opt *opt, const char **paths, int cached)\n>  {\n>        int hit = 0;\n> @@ -597,6 +682,10 @@ static int grep_cache(struct grep_opt *opt, const char **paths, int cached)\n>                struct cache_entry *ce = active_cache[nr];\n>                if (!pathspec_matches(paths, ce->name, opt->max_depth))\n>                        continue;\n> +               if (S_ISGITLINK(ce->ce_mode) && opt->recurse_submodules) {\n> +                       hit |= grep_submodule(opt, ce->name, NULL, NULL);\n> +                       continue;\n> +               }\n>                if (!S_ISREG(ce->ce_mode))\n>                        continue;\n>                /*\n> @@ -634,11 +723,16 @@ static int grep_tree(struct grep_opt *opt, const char **paths,\n>        char *down;\n>        int tn_len = strlen(tree_name);\n>        struct strbuf pathbuf;\n> +       const char *refname = getenv(\"GIT_SUPER_REFNAME\");\n> +       int rn_len = refname ? strlen(refname) : 0;\n>\n> -       strbuf_init(&pathbuf, PATH_MAX + tn_len);\n> +       strbuf_init(&pathbuf, PATH_MAX + MAX(tn_len, rn_len));\n>\n>        if (tn_len) {\n> -               strbuf_add(&pathbuf, tree_name, tn_len);\n> +               if (refname)\n> +                       strbuf_add(&pathbuf, refname, rn_len);\n> +               else\n> +                       strbuf_add(&pathbuf, tree_name, tn_len);\n>                strbuf_addch(&pathbuf, ':');\n>                tn_len = pathbuf.len;\n>        }\n> @@ -664,6 +758,9 @@ static int grep_tree(struct grep_opt *opt, const char **paths,\n>                        ;\n>                else if (S_ISREG(entry.mode))\n>                        hit |= grep_sha1(opt, entry.sha1, pathbuf.buf, tn_len);\n> +               else if (S_ISGITLINK(entry.mode) && opt->recurse_submodules)\n> +                       hit |= grep_submodule(opt, entry.path,\n> +                               sha1_to_hex(entry.sha1), tree_name);\n>                else if (S_ISDIR(entry.mode)) {\n>                        enum object_type type;\n>                        struct tree_desc sub;\n> @@ -931,6 +1028,8 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n>                            \"allow calling of grep(1) (ignored by this build)\"),\n>                { OPTION_CALLBACK, 0, \"help-all\", &options, NULL, \"show usage\",\n>                  PARSE_OPT_HIDDEN | PARSE_OPT_NOARG, help_callback },\n> +               OPT_BOOLEAN(0, \"recursive\", &opt.recurse_submodules,\n> +                       \"recurse into submodules\"),\n>                OPT_END()\n>        };\n>\n> diff --git a/grep.h b/grep.h\n> index 06621fe..d5e2e11 100644\n> --- a/grep.h\n> +++ b/grep.h\n> @@ -101,6 +101,7 @@ struct grep_opt {\n>        unsigned post_context;\n>        unsigned last_shown;\n>        int show_hunk_mark;\n> +       int recurse_submodules;\n>        void *priv;\n>\n>        void (*output)(struct grep_opt *opt, const void *data, size_t size);\n> --\n> 1.7.3.1\n>\n>\n\n\n\n-- \nDuy\n"},{"id":"153686","messageId":"4CBBAA8F.2040805@gmail.com","threadId":"25457","inReplyTo":"AANLkTikSukNCkcx4RYRuzzKU8T64mT0boMDqoNBrWW73@mail.gmail.com","subject":"Re: [RFC/PATCHv2 5/5] grep: add support for grepping in submodules","fromName":"Chris Packham","fromEmail":"judge.packham@gmail.com","sentAt":"2010-10-18T02:01:51Z","receivedAt":"2010-10-18T02:01:51Z","isPatch":false,"sender":{"key":"judge.packham@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155667?v=4"},"body":"On 17/10/10 03:28, Nguyen Thai Ngoc Duy wrote:\n> On Sat, Oct 16, 2010 at 6:26 AM, Chris Packham <judge.packham@gmail.com> wrote:\n>> +static int grep_submodule(struct grep_opt *opt, const char *path,\n>> +                         const char *sha1, const char *tree_name)\n>> +{\n>> +       struct strbuf buf = STRBUF_INIT;\n>> +       struct strbuf pre_buf = STRBUF_INIT;\n>> +       struct child_process cp;\n>> +       const char **argv = create_sub_grep_argv(opt, path, sha1, tree_name);\n> \n> Can we just save sha1 in a env variable and drop this argv rewrite?\n> \nThe tree_name is a hangover from when I was passing it as a command line\narg, I'll remove it. For now we still need to rewrite argv to modify the\npathspec and max-depth, eventually I'd like to ditch that approach in\nfavour of using fork() and modifying opts.\n\n>> +       const char *git_dir;\n>> +       int hit = 0;\n>> +       memset(&cp, 0, sizeof(cp));\n>> +\n>> +       strbuf_addf(&buf, \"%s/.git\", path);\n>> +       git_dir = read_gitfile_gently(buf.buf);\n>> +       if (!git_dir)\n>> +               git_dir = buf.buf;\n>> +       if (!is_directory(git_dir))\n>> +               goto out_free;\n>> +\n>> +       setenv(\"GIT_SUPER_REFNAME\", tree_name, 1);\n>> +       setenv(GIT_DIR_ENVIRONMENT, git_dir, 1);\n>> +       setenv(GIT_WORK_TREE_ENVIRONMENT, path, 1);\n> \n> cp.env can be used to set these variables\n> \nI was struggling a little with how to do this. The run_command\ninvocation in submodules.c uses local_repo_env which is protected\nagainst modification. I also don't understand how the actual values are\npassed. If someone could point me in the right direction it'd be\nappreciated.\n\n>> +       cp.argv = argv;\n>> +       cp.git_cmd = 1;\n>> +       cp.no_stdin = 1;\n> \n> I think you stll need \"cp.dir = path;\" here because the setup routines\n> won't do that for you. But I need to check/test that.\n> \nOriginally I did set cp.dir but that is used by run_command (via\nstart_command) to chdir. Which foils the cwd to worktree detection (at\nleast in V1 of that patch).\n\n>> +       if (run_command(&cp) == 0)\n>> +               hit = 1;\n>> +out_free:\n>> +       unsetenv(\"GIT_SUPER_REFNAME\");\n>> +       unsetenv(GIT_DIR_ENVIRONMENT);\n>> +       unsetenv(GIT_WORK_TREE_ENVIRONMENT);\n>> +       free(argv);\n>> +       strbuf_release(&buf);\n>> +       strbuf_release(&pre_buf);\n>> +       return hit;\n>> +}\n>> +\n>>  static int grep_cache(struct grep_opt *opt, const char **paths, int cached)\n>>  {\n>>        int hit = 0;\n>> @@ -597,6 +682,10 @@ static int grep_cache(struct grep_opt *opt, const char **paths, int cached)\n>>                struct cache_entry *ce = active_cache[nr];\n>>                if (!pathspec_matches(paths, ce->name, opt->max_depth))\n>>                        continue;\n>> +               if (S_ISGITLINK(ce->ce_mode) && opt->recurse_submodules) {\n>> +                       hit |= grep_submodule(opt, ce->name, NULL, NULL);\n>> +                       continue;\n>> +               }\n>>                if (!S_ISREG(ce->ce_mode))\n>>                        continue;\n>>                /*\n>> @@ -634,11 +723,16 @@ static int grep_tree(struct grep_opt *opt, const char **paths,\n>>        char *down;\n>>        int tn_len = strlen(tree_name);\n>>        struct strbuf pathbuf;\n>> +       const char *refname = getenv(\"GIT_SUPER_REFNAME\");\n>> +       int rn_len = refname ? strlen(refname) : 0;\n>>\n>> -       strbuf_init(&pathbuf, PATH_MAX + tn_len);\n>> +       strbuf_init(&pathbuf, PATH_MAX + MAX(tn_len, rn_len));\n>>\n>>        if (tn_len) {\n>> -               strbuf_add(&pathbuf, tree_name, tn_len);\n>> +               if (refname)\n>> +                       strbuf_add(&pathbuf, refname, rn_len);\n>> +               else\n>> +                       strbuf_add(&pathbuf, tree_name, tn_len);\n>>                strbuf_addch(&pathbuf, ':');\n>>                tn_len = pathbuf.len;\n>>        }\n>> @@ -664,6 +758,9 @@ static int grep_tree(struct grep_opt *opt, const char **paths,\n>>                        ;\n>>                else if (S_ISREG(entry.mode))\n>>                        hit |= grep_sha1(opt, entry.sha1, pathbuf.buf, tn_len);\n>> +               else if (S_ISGITLINK(entry.mode) && opt->recurse_submodules)\n>> +                       hit |= grep_submodule(opt, entry.path,\n>> +                               sha1_to_hex(entry.sha1), tree_name);\n>>                else if (S_ISDIR(entry.mode)) {\n>>                        enum object_type type;\n>>                        struct tree_desc sub;\n>> @@ -931,6 +1028,8 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n>>                            \"allow calling of grep(1) (ignored by this build)\"),\n>>                { OPTION_CALLBACK, 0, \"help-all\", &options, NULL, \"show usage\",\n>>                  PARSE_OPT_HIDDEN | PARSE_OPT_NOARG, help_callback },\n>> +               OPT_BOOLEAN(0, \"recursive\", &opt.recurse_submodules,\n>> +                       \"recurse into submodules\"),\n>>                OPT_END()\n>>        };\n>>\n>> diff --git a/grep.h b/grep.h\n>> index 06621fe..d5e2e11 100644\n>> --- a/grep.h\n>> +++ b/grep.h\n>> @@ -101,6 +101,7 @@ struct grep_opt {\n>>        unsigned post_context;\n>>        unsigned last_shown;\n>>        int show_hunk_mark;\n>> +       int recurse_submodules;\n>>        void *priv;\n>>\n>>        void (*output)(struct grep_opt *opt, const void *data, size_t size);\n>> --\n>> 1.7.3.1\n>>\n>>\n> \n> \n> \n"},{"id":"153687","messageId":"4CBBAB71.4010503@gmail.com","threadId":"25457","inReplyTo":"20101017094524.GA6034@do","subject":"Re: [RFC/PATCHv2 1/5] worktree: provide better prefix to go back to original cwd","fromName":"Chris Packham","fromEmail":"judge.packham@gmail.com","sentAt":"2010-10-18T02:05:37Z","receivedAt":"2010-10-18T02:05:37Z","isPatch":false,"sender":{"key":"judge.packham@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155667?v=4"},"body":"On 17/10/10 03:01, Nguyen Thai Ngoc Duy wrote:\n> While at it, have you thought of support --recursive and\n> --full-tree [1]? There are issues with --full-tree and prefixes [2],\n> which is why it is dropped but I think it's a good idea.\n> \n> --full-tree disregards where you stand and greps in whole repo. In a\n> repo with submodules, that would mean grep the supermodule and all\n> submodules regardless where you stand, even if you stand in a\n> submodule.\n> \n> [1] http://mid.gmane.org/7vk4xggv27.fsf@alter.siamese.dyndns.org\n> [2] http://mid.gmane.org/7vskaqptvj.fsf@alter.siamese.dyndns.org\n\nI haven't looked at those threads yet but it sounds useful to me. I\ndon't want this series to grow too far in scope so maybe this could be a\nfollow-up if my series is accepted.\n"},{"id":"153689","messageId":"AANLkTikUpUZrwj9S91ZarUbfkcPgpnyx3h54dJb2W9X0@mail.gmail.com","threadId":"25457","inReplyTo":"4CBBAA8F.2040805@gmail.com","subject":"Re: [RFC/PATCHv2 5/5] grep: add support for grepping in submodules","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2010-10-18T03:37:01Z","receivedAt":"2010-10-18T03:37:01Z","isPatch":false,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Mon, Oct 18, 2010 at 9:01 AM, Chris Packham <judge.packham@gmail.com> wrote:\n> On 17/10/10 03:28, Nguyen Thai Ngoc Duy wrote:\n>> On Sat, Oct 16, 2010 at 6:26 AM, Chris Packham <judge.packham@gmail.com> wrote:\n>>> +static int grep_submodule(struct grep_opt *opt, const char *path,\n>>> +                         const char *sha1, const char *tree_name)\n>>> +{\n>>> +       struct strbuf buf = STRBUF_INIT;\n>>> +       struct strbuf pre_buf = STRBUF_INIT;\n>>> +       struct child_process cp;\n>>> +       const char **argv = create_sub_grep_argv(opt, path, sha1, tree_name);\n>>\n>> Can we just save sha1 in a env variable and drop this argv rewrite?\n>>\n> The tree_name is a hangover from when I was passing it as a command line\n> arg, I'll remove it. For now we still need to rewrite argv to modify the\n> pathspec and max-depth, eventually I'd like to ditch that approach in\n> favour of using fork() and modifying opts.\n\nYou would also need to undo git repo setup, i.e. resetting static\nvariables in environment.c and setup.c, maybe some more. Also fork()\nis not available on Windows.\nSpawning new processes is the only feasible way I can think of now (or\neven in the next year).\n\n>>> +       const char *git_dir;\n>>> +       int hit = 0;\n>>> +       memset(&cp, 0, sizeof(cp));\n>>> +\n>>> +       strbuf_addf(&buf, \"%s/.git\", path);\n>>> +       git_dir = read_gitfile_gently(buf.buf);\n>>> +       if (!git_dir)\n>>> +               git_dir = buf.buf;\n>>> +       if (!is_directory(git_dir))\n>>> +               goto out_free;\n>>> +\n>>> +       setenv(\"GIT_SUPER_REFNAME\", tree_name, 1);\n>>> +       setenv(GIT_DIR_ENVIRONMENT, git_dir, 1);\n>>> +       setenv(GIT_WORK_TREE_ENVIRONMENT, path, 1);\n>>\n>> cp.env can be used to set these variables\n>>\n> I was struggling a little with how to do this. The run_command\n> invocation in submodules.c uses local_repo_env which is protected\n> against modification. I also don't understand how the actual values are\n> passed. If someone could point me in the right direction it'd be\n> appreciated.\n\nFrom what I read in run-command.c,the current env var set is copied\nas-is for new process, then any new variables from cp.env will be\nadded. Old variables can also be unset by saying \"VARIABLE=\" in\ncp.env. The final env var set will be used for new process. Current\nenv vars are untouched. Something like this should work:\n\nconst char *env[4];\n\nenv[0] = malloc_sprintf(\"GIT_SUPER_REFNAME=%s\", tree_name);\nenv[1] = malloc_sprintf(\"GIT_DIR_ENVIRONMENT=%s\", git_dir);\nenv[2] = malloc_sprintf(\"GIT_WORK_TREE+ENVIRONMENT=%s\", path);\nenv[3] = NULL;\ncp.env = env;\n\n>>> +       cp.argv = argv;\n>>> +       cp.git_cmd = 1;\n>>> +       cp.no_stdin = 1;\n>>\n>> I think you stll need \"cp.dir = path;\" here because the setup routines\n>> won't do that for you. But I need to check/test that.\n>>\n> Originally I did set cp.dir but that is used by run_command (via\n> start_command) to chdir. Which foils the cwd to worktree detection (at\n> least in V1 of that patch).\n\nRight. I forgot that. But then when new grep is run, it should move\nback to worktree. I believe current grep code assumes that cwd is\nworktree root.\n-- \nDuy\n"}]}