ls-files: add pathspec matching for submodules
authorBrandon Williams <bmwill@google.com>
Fri, 7 Oct 2016 18:18:51 +0000 (11:18 -0700)
committerJunio C Hamano <gitster@pobox.com>
Mon, 10 Oct 2016 19:14:58 +0000 (12:14 -0700)
Pathspecs can be a bit tricky when trying to apply them to submodules.
The main challenge is that the pathspecs will be with respect to the
superproject and not with respect to paths in the submodule. The
approach this patch takes is to pass in the identical pathspec from the
superproject to the submodule in addition to the submodule-prefix, which
is the path from the root of the superproject to the submodule, and then
we can compare an entry in the submodule prepended with the
submodule-prefix to the pathspec in order to determine if there is a
match.

This patch also permits the pathspec logic to perform a prefix match against
submodules since a pathspec could refer to a file inside of a submodule.
Due to limitations in the wildmatch logic, a prefix match is only done
literally. If any wildcard character is encountered we'll simply punt
and produce a false positive match. More accurate matching will be done
once inside the submodule. This is due to the superproject not knowing
what files could exist in the submodule.

Signed-off-by: Brandon Williams <bmwill@google.com>
Reviewed-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Documentation/git-ls-files.txt
builtin/ls-files.c
dir.c
dir.h
t/t3007-ls-files-recurse-submodules.sh
index ea01d45e5c49a80cf73aec062ef0f6f6471df9d9..446209e2062df8d0b1c99a25eaaf4dc6f15bce3c 100644 (file)
@@ -140,8 +140,7 @@ a space) at the start of each line:
 
 --recurse-submodules::
        Recursively calls ls-files on each submodule in the repository.
-       Currently there is only support for the --cached mode without a
-       pathspec.
+       Currently there is only support for the --cached mode.
 
 --abbrev[=<n>]::
        Instead of showing the full 40-byte hexadecimal object
index b6144a56d3f44d83d111997ccdd7cacb5bcafac1..0f25914348e657712bfe252363f4fac3c59a9d84 100644 (file)
@@ -195,6 +195,7 @@ static void show_gitlink(const struct cache_entry *ce)
 {
        struct child_process cp = CHILD_PROCESS_INIT;
        int status;
+       int i;
 
        argv_array_pushf(&cp.args, "--super-prefix=%s%s/",
                         super_prefix ? super_prefix : "",
@@ -205,6 +206,15 @@ static void show_gitlink(const struct cache_entry *ce)
        /* add supported options */
        argv_array_pushv(&cp.args, submodules_options.argv);
 
+       /*
+        * Pass in the original pathspec args.  The submodule will be
+        * responsible for prepending the 'submodule_prefix' prior to comparing
+        * against the pathspec for matches.
+        */
+       argv_array_push(&cp.args, "--");
+       for (i = 0; i < pathspec.nr; i++)
+               argv_array_push(&cp.args, pathspec.items[i].original);
+
        cp.git_cmd = 1;
        cp.dir = ce->name;
        status = run_command(&cp);
@@ -223,7 +233,8 @@ static void show_ce_entry(const char *tag, const struct cache_entry *ce)
        if (len >= ce_namelen(ce))
                die("git ls-files: internal error - cache entry not superset of prefix");
 
-       if (recurse_submodules && S_ISGITLINK(ce->ce_mode)) {
+       if (recurse_submodules && S_ISGITLINK(ce->ce_mode) &&
+           submodule_path_match(&pathspec, name.buf, ps_matched)) {
                show_gitlink(ce);
        } else if (match_pathspec(&pathspec, name.buf, name.len,
                                  len, ps_matched,
@@ -602,16 +613,20 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix)
                die("ls-files --recurse-submodules does not support "
                    "--error-unmatch");
 
-       if (recurse_submodules && argc)
-               die("ls-files --recurse-submodules does not support pathspec");
-
        parse_pathspec(&pathspec, 0,
                       PATHSPEC_PREFER_CWD |
                       PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP,
                       prefix, argv);
 
-       /* Find common prefix for all pathspec's */
-       max_prefix = common_prefix(&pathspec);
+       /*
+        * Find common prefix for all pathspec's
+        * This is used as a performance optimization which unfortunately cannot
+        * be done when recursing into submodules
+        */
+       if (recurse_submodules)
+               max_prefix = NULL;
+       else
+               max_prefix = common_prefix(&pathspec);
        max_prefix_len = max_prefix ? strlen(max_prefix) : 0;
 
        /* Treat unmatching pathspec elements as errors */
diff --git a/dir.c b/dir.c
index 0ea235f3d643d7a9072aca75d7317d1fd7552b4d..28e97367d9ddc3748ae15f0cc7dbfdccf900190b 100644 (file)
--- a/dir.c
+++ b/dir.c
@@ -207,8 +207,9 @@ int within_depth(const char *name, int namelen,
        return 1;
 }
 
-#define DO_MATCH_EXCLUDE   1
-#define DO_MATCH_DIRECTORY 2
+#define DO_MATCH_EXCLUDE   (1<<0)
+#define DO_MATCH_DIRECTORY (1<<1)
+#define DO_MATCH_SUBMODULE (1<<2)
 
 /*
  * Does 'match' match the given name?
@@ -283,6 +284,32 @@ static int match_pathspec_item(const struct pathspec_item *item, int prefix,
                         item->nowildcard_len - prefix))
                return MATCHED_FNMATCH;
 
+       /* Perform checks to see if "name" is a super set of the pathspec */
+       if (flags & DO_MATCH_SUBMODULE) {
+               /* name is a literal prefix of the pathspec */
+               if ((namelen < matchlen) &&
+                   (match[namelen] == '/') &&
+                   !ps_strncmp(item, match, name, namelen))
+                       return MATCHED_RECURSIVELY;
+
+               /* name" doesn't match up to the first wild character */
+               if (item->nowildcard_len < item->len &&
+                   ps_strncmp(item, match, name,
+                              item->nowildcard_len - prefix))
+                       return 0;
+
+               /*
+                * Here is where we would perform a wildmatch to check if
+                * "name" can be matched as a directory (or a prefix) against
+                * the pathspec.  Since wildmatch doesn't have this capability
+                * at the present we have to punt and say that it is a match,
+                * potentially returning a false positive
+                * The submodules themselves will be able to perform more
+                * accurate matching to determine if the pathspec matches.
+                */
+               return MATCHED_RECURSIVELY;
+       }
+
        return 0;
 }
 
@@ -386,6 +413,21 @@ int match_pathspec(const struct pathspec *ps,
        return negative ? 0 : positive;
 }
 
+/**
+ * Check if a submodule is a superset of the pathspec
+ */
+int submodule_path_match(const struct pathspec *ps,
+                        const char *submodule_name,
+                        char *seen)
+{
+       int matched = do_match_pathspec(ps, submodule_name,
+                                       strlen(submodule_name),
+                                       0, seen,
+                                       DO_MATCH_DIRECTORY |
+                                       DO_MATCH_SUBMODULE);
+       return matched;
+}
+
 int report_path_error(const char *ps_matched,
                      const struct pathspec *pathspec,
                      const char *prefix)
diff --git a/dir.h b/dir.h
index da1a858b3a12daba5bd348fcaa342534edaacdb6..97c83bb383a6b1fb5ce58d5e81dead570b73ac43 100644 (file)
--- a/dir.h
+++ b/dir.h
@@ -304,6 +304,10 @@ extern int git_fnmatch(const struct pathspec_item *item,
                       const char *pattern, const char *string,
                       int prefix);
 
+extern int submodule_path_match(const struct pathspec *ps,
+                               const char *submodule_name,
+                               char *seen);
+
 static inline int ce_path_match(const struct cache_entry *ce,
                                const struct pathspec *pathspec,
                                char *seen)
index 33a2ea71cf46de046b3c693a4ff8ff5026eabf5f..a5426171d3ec652ffc02fc8279bc5802f57204a3 100755 (executable)
@@ -81,9 +81,111 @@ test_expect_success 'ls-files recurses more than 1 level' '
        test_cmp expect actual
 '
 
-test_expect_success '--recurse-submodules does not support using path arguments' '
-       test_must_fail git ls-files --recurse-submodules b 2>actual &&
-       test_i18ngrep "does not support pathspec" actual
+test_expect_success '--recurse-submodules and pathspecs setup' '
+       echo e >submodule/subsub/e.txt &&
+       git -C submodule/subsub add e.txt &&
+       git -C submodule/subsub commit -m "adding e.txt" &&
+       echo f >submodule/f.TXT &&
+       echo g >submodule/g.txt &&
+       git -C submodule add f.TXT g.txt &&
+       git -C submodule commit -m "add f and g" &&
+       echo h >h.txt &&
+       mkdir sib &&
+       echo sib >sib/file &&
+       git add h.txt sib/file &&
+       git commit -m "add h and sib/file" &&
+       git init sub &&
+       echo sub >sub/file &&
+       git -C sub add file &&
+       git -C sub commit -m "add file" &&
+       git submodule add ./sub &&
+       git commit -m "added sub" &&
+
+       cat >expect <<-\EOF &&
+       .gitmodules
+       a
+       b/b
+       h.txt
+       sib/file
+       sub/file
+       submodule/.gitmodules
+       submodule/c
+       submodule/f.TXT
+       submodule/g.txt
+       submodule/subsub/d
+       submodule/subsub/e.txt
+       EOF
+
+       git ls-files --recurse-submodules >actual &&
+       test_cmp expect actual &&
+       cat actual &&
+       git ls-files --recurse-submodules "*" >actual &&
+       test_cmp expect actual
+'
+
+test_expect_success '--recurse-submodules and pathspecs' '
+       cat >expect <<-\EOF &&
+       h.txt
+       submodule/g.txt
+       submodule/subsub/e.txt
+       EOF
+
+       git ls-files --recurse-submodules "*.txt" >actual &&
+       test_cmp expect actual
+'
+
+test_expect_success '--recurse-submodules and pathspecs' '
+       cat >expect <<-\EOF &&
+       h.txt
+       submodule/f.TXT
+       submodule/g.txt
+       submodule/subsub/e.txt
+       EOF
+
+       git ls-files --recurse-submodules ":(icase)*.txt" >actual &&
+       test_cmp expect actual
+'
+
+test_expect_success '--recurse-submodules and pathspecs' '
+       cat >expect <<-\EOF &&
+       h.txt
+       submodule/f.TXT
+       submodule/g.txt
+       EOF
+
+       git ls-files --recurse-submodules ":(icase)*.txt" ":(exclude)submodule/subsub/*" >actual &&
+       test_cmp expect actual
+'
+
+test_expect_success '--recurse-submodules and pathspecs' '
+       cat >expect <<-\EOF &&
+       sub/file
+       EOF
+
+       git ls-files --recurse-submodules "sub" >actual &&
+       test_cmp expect actual &&
+       git ls-files --recurse-submodules "sub/" >actual &&
+       test_cmp expect actual &&
+       git ls-files --recurse-submodules "sub/file" >actual &&
+       test_cmp expect actual &&
+       git ls-files --recurse-submodules "su*/file" >actual &&
+       test_cmp expect actual &&
+       git ls-files --recurse-submodules "su?/file" >actual &&
+       test_cmp expect actual
+'
+
+test_expect_success '--recurse-submodules and pathspecs' '
+       cat >expect <<-\EOF &&
+       sib/file
+       sub/file
+       EOF
+
+       git ls-files --recurse-submodules "s??/file" >actual &&
+       test_cmp expect actual &&
+       git ls-files --recurse-submodules "s???file" >actual &&
+       test_cmp expect actual &&
+       git ls-files --recurse-submodules "s*file" >actual &&
+       test_cmp expect actual
 '
 
 test_expect_success '--recurse-submodules does not support --error-unmatch' '