Most of the changes between v2 and v3 of this series were to address the few
reviewer comments.
* use 'path' as the directory to cd into for the child process and use 'name'
  for the super_prefix
* flush output from childprocess on error and print a more useful error msg
* change is_submodule_checked_out to is_submodule_populated
* fix GIT_DIR for a submodule child process from being set to '.git'
* Addition of a test for searching history of a submodule which had been moved.

The series as a whole probably needs a few more rounds of
review so any additional input would be appreciated!  

Thanks!
Brandon

Brandon Williams (6):
  submodules: add helper functions to determine presence of submodules
  submodules: load gitmodules file from commit sha1
  grep: add submodules as a grep source type
  grep: optionally recurse into submodules
  grep: enable recurse-submodules to work on <tree> objects
  grep: search history of moved submodules

 Documentation/git-grep.txt         |  14 ++
 builtin/grep.c                     | 391 ++++++++++++++++++++++++++++++++++---
 cache.h                            |   2 +
 config.c                           |   8 +-
 git.c                              |   2 +-
 grep.c                             |  16 +-
 grep.h                             |   1 +
 submodule-config.c                 |   6 +-
 submodule-config.h                 |   3 +
 submodule.c                        |  50 +++++
 submodule.h                        |   3 +
 t/t7814-grep-recurse-submodules.sh | 182 +++++++++++++++++
 12 files changed, 646 insertions(+), 32 deletions(-)
 create mode 100755 t/t7814-grep-recurse-submodules.sh

-- interdiff based on 'bw/grep-recurse-submodules'

diff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt
index 386a868..71f32f3 100644
--- a/Documentation/git-grep.txt
+++ b/Documentation/git-grep.txt
@@ -26,7 +26,7 @@ SYNOPSIS
           [--threads <num>]
           [-f <file>] [-e] <pattern>
           [--and|--or|--not|(|)|-e <pattern>...]
-          [--recurse-submodules] [--parent-basename]
+          [--recurse-submodules] [--parent-basename <basename>]
           [ [--[no-]exclude-standard] [--cached | --no-index | --untracked] | 
<tree>...]
           [--] [<pathspec>...]
 
@@ -95,7 +95,7 @@ OPTIONS
        <tree> option the prefix of all submodule output will be the name of
        the parent project's <tree> object.
 
---parent-basename::
+--parent-basename <basename>::
        For internal use only.  In order to produce uniform output with the
        --recurse-submodules option, this option can be used to provide the
        basename of a parent's <tree> object to a submodule so the submodule
diff --git a/builtin/grep.c b/builtin/grep.c
index bdf1b90..1879432 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -538,14 +538,22 @@ static int grep_submodule_launch(struct grep_opt *opt,
        struct child_process cp = CHILD_PROCESS_INIT;
        int status, i;
        const char *end_of_base;
+       const char *name;
        struct work_item *w = opt->output_priv;
 
+       end_of_base = strchr(gs->name, ':');
+       if (end_of_base)
+               name = end_of_base + 1;
+       else
+               name = gs->name;
+
        prepare_submodule_repo_env(&cp.env_array);
+       argv_array_push(&cp.env_array, GIT_DIR_ENVIRONMENT);
 
        /* Add super prefix */
        argv_array_pushf(&cp.args, "--super-prefix=%s%s/",
-                        super_prefix ? super_prefix: "",
-                        gs->path);
+                        super_prefix ? super_prefix : "",
+                        name);
        argv_array_push(&cp.args, "grep");
 
        /*
@@ -556,7 +564,6 @@ static int grep_submodule_launch(struct grep_opt *opt,
         * parent project's object name to the submodule so the submodule can
         * prefix its output with the parent's name and not its own SHA1.
         */
-       end_of_base = strchr(gs->name, ':');
        if (end_of_base)
                argv_array_pushf(&cp.args, "--parent-basename=%.*s",
                                 (int) (end_of_base - gs->name),
@@ -588,8 +595,12 @@ static int grep_submodule_launch(struct grep_opt *opt,
         * anything else is an error.
         */
        status = capture_command(&cp, &w->out, 0);
-       if (status && (status != 1))
-               exit(status);
+       if (status && (status != 1)) {
+               /* flush the buffer */
+               write_or_die(1, w->out.buf, w->out.len);
+               die("process for submodule '%s' failed with exit code: %d",
+                   gs->name, status);
+       }
 
        /* invert the return code to make a hit equal to 1 */
        return !status;
@@ -605,11 +616,20 @@ static int grep_submodule(struct grep_opt *opt, const 
unsigned char *sha1,
                          const char *filename, const char *path)
 {
        if (!(is_submodule_initialized(path) &&
-             is_submodule_checked_out(path))) {
-               warning("skiping submodule '%s%s' since it is not initialized 
and checked out",
-                       super_prefix ? super_prefix: "",
-                       path);
-               return 0;
+             is_submodule_populated(path))) {
+               /*
+                * If searching history, check for the presense of the
+                * submodule's gitdir before skipping the submodule.
+                */
+               if (sha1) {
+                       path = git_path("modules/%s",
+                                       submodule_from_path(null_sha1, 
path)->name);
+
+                       if(!(is_directory(path) && is_git_directory(path)))
+                               return 0;
+               } else {
+                       return 0;
+               }
        }
 
 #ifndef NO_PTHREADS
@@ -670,8 +690,7 @@ static int grep_cache(struct grep_opt *opt, const struct 
pathspec *pathspec,
                                        continue;
                                hit |= grep_sha1(opt, ce->oid.hash, ce->name,
                                                 0, ce->name);
-                       }
-                       else {
+                       } else {
                                hit |= grep_file(opt, ce->name);
                        }
                } else if (recurse_submodules && S_ISGITLINK(ce->ce_mode) &&
@@ -705,8 +724,8 @@ static int grep_tree(struct grep_opt *opt, const struct 
pathspec *pathspec,
        struct strbuf name = STRBUF_INIT;
        int name_base_len = 0;
        if (super_prefix) {
-               name_base_len = strlen(super_prefix);
                strbuf_addstr(&name, super_prefix);
+               name_base_len = name.len;
        }
 
        while (tree_entry(tree, &entry)) {
@@ -715,8 +734,16 @@ static int grep_tree(struct grep_opt *opt, const struct 
pathspec *pathspec,
                if (match != all_entries_interesting) {
                        strbuf_setlen(&name, name_base_len);
                        strbuf_addstr(&name, base->buf + tn_len);
-                       match = tree_entry_interesting(&entry, &name,
-                                                      0, pathspec);
+
+                       if (recurse_submodules && S_ISGITLINK(entry.mode)) {
+                               strbuf_addstr(&name, entry.path);
+                               match = submodule_path_match(pathspec, name.buf,
+                                                            NULL);
+                       } else {
+                               match = tree_entry_interesting(&entry, &name,
+                                                              0, pathspec);
+                       }
+
                        if (match == all_entries_not_interesting)
                                break;
                        if (match == entry_not_interesting)
diff --git a/submodule.c b/submodule.c
index f2a5668..062e58b 100644
--- a/submodule.c
+++ b/submodule.c
@@ -221,31 +221,30 @@ int is_submodule_initialized(const char *path)
        module = submodule_from_path(null_sha1, path);
 
        if (module) {
-               struct strbuf buf = STRBUF_INIT;
-               char *submodule_url = NULL;
+               char *key = xstrfmt("submodule.%s.url", module->name);
+               char *value = NULL;
 
-               strbuf_addf(&buf, "submodule.%s.url",module->name);
-               ret = !git_config_get_string(buf.buf, &submodule_url);
+               ret = !git_config_get_string(key, &value);
 
-               free(submodule_url);
-               strbuf_release(&buf);
+               free(value);
+               free(key);
        }
 
        return ret;
 }
 
 /*
- * Determine if a submodule has been checked out at a given 'path'
+ * Determine if a submodule has been populated at a given 'path'
  */
-int is_submodule_checked_out(const char *path)
+int is_submodule_populated(const char *path)
 {
        int ret = 0;
-       struct strbuf buf = STRBUF_INIT;
+       char *gitdir = xstrfmt("%s/.git", path);
 
-       strbuf_addf(&buf, "%s/.git", path);
-       ret = file_exists(buf.buf);
+       if (resolve_gitdir(gitdir))
+               ret = 1;
 
-       strbuf_release(&buf);
+       free(gitdir);
        return ret;
 }
 
diff --git a/submodule.h b/submodule.h
index 9a24ac8..9203d89 100644
--- a/submodule.h
+++ b/submodule.h
@@ -39,7 +39,7 @@ int submodule_config(const char *var, const char *value, void 
*cb);
 void gitmodules_config(void);
 extern void gitmodules_config_sha1(const unsigned char *commit_sha1);
 extern int is_submodule_initialized(const char *path);
-extern int is_submodule_checked_out(const char *path);
+extern int is_submodule_populated(const char *path);
 int parse_submodule_update_strategy(const char *value,
                struct submodule_update_strategy *dst);
 const char *submodule_strategy_to_string(const struct 
submodule_update_strategy *s);
diff --git a/t/t7814-grep-recurse-submodules.sh 
b/t/t7814-grep-recurse-submodules.sh
index 3d1892d..ee173ad 100755
--- a/t/t7814-grep-recurse-submodules.sh
+++ b/t/t7814-grep-recurse-submodules.sh
@@ -127,6 +127,47 @@ test_expect_success 'grep tree and pathspecs' '
        test_cmp expect actual
 '
 
+test_expect_success 'grep history with moved submoules' '
+       git init parent &&
+       echo "foobar" >parent/file &&
+       git -C parent add file &&
+       git -C parent commit -m "add file" &&
+
+       git init sub &&
+       echo "foobar" >sub/file &&
+       git -C sub add file &&
+       git -C sub commit -m "add file" &&
+
+       git -C parent submodule add ../sub &&
+       git -C parent commit -m "add submodule" &&
+
+       cat >expect <<-\EOF &&
+       file:foobar
+       sub/file:foobar
+       EOF
+       git -C parent grep -e "foobar" --recurse-submodules > actual &&
+       test_cmp expect actual &&
+
+       git -C parent mv sub sub-moved &&
+       git -C parent commit -m "moved submodule" &&
+
+       cat >expect <<-\EOF &&
+       file:foobar
+       sub-moved/file:foobar
+       EOF
+       git -C parent grep -e "foobar" --recurse-submodules > actual &&
+       test_cmp expect actual &&
+
+       cat >expect <<-\EOF &&
+       HEAD^:file:foobar
+       HEAD^:sub/file:foobar
+       EOF
+       git -C parent grep -e "foobar" --recurse-submodules HEAD^ > actual &&
+       test_cmp expect actual &&
+
+       rm -rf parent sub
+'
+
 test_incompatible_with_recurse_submodules ()
 {
        test_expect_success "--recurse-submodules and $1 are incompatible" "
diff --git a/tree-walk.c b/tree-walk.c
index b3f9961..828f435 100644
--- a/tree-walk.c
+++ b/tree-walk.c
@@ -999,11 +999,10 @@ static enum interesting do_match(const struct name_entry 
*entry,
                                        return entry_interesting;
 
                                /*
-                                * Match all directories and gitlinks. We'll
-                                * try to match files later on.
+                                * Match all directories. We'll try to
+                                * match files later on.
                                 */
-                               if (ps->recursive && (S_ISDIR(entry->mode) ||
-                                                     S_ISGITLINK(entry->mode)))
+                               if (ps->recursive && S_ISDIR(entry->mode))
                                        return entry_interesting;
                        }
 
@@ -1044,13 +1043,13 @@ static enum interesting do_match(const struct 
name_entry *entry,
                strbuf_setlen(base, base_offset + baselen);
 
                /*
-                * Match all directories and gitlinks. We'll try to match files
-                * later on.  max_depth is ignored but we may consider support
-                * it in future, see
+                * Match all directories. We'll try to match files
+                * later on.
+                * max_depth is ignored but we may consider support it
+                * in future, see
                 * 
http://thread.gmane.org/gmane.comp.version-control.git/163757/focus=163840
                 */
-               if (ps->recursive && (S_ISDIR(entry->mode) ||
-                                     S_ISGITLINK(entry->mode)))
+               if (ps->recursive && S_ISDIR(entry->mode))
                        return entry_interesting;
        }
        return never_interesting; /* No matches */

-- 
2.8.0.rc3.226.g39d4020

Reply via email to