tree-walk: support :(attr) matching

This lets us use :(attr) with "git grep <tree-ish>" or "git log".

:(attr) requires another round of checking before we can declare that
a path is matched. This is done after path matching since we have lots
of optimization to take a shortcut when things don't match.

Note that if :(attr) is present, we can't return
all_entries_interesting / all_entries_not_interesting anymore because
we can't be certain about that. Not until match_pathspec_attrs() can
tell us "yes all these paths satisfy :(attr)".

Second note. Even though we walk a specific tree, we use attributes
from _worktree_ (or falling back to the index), not from .gitattributes
files on that tree. This by itself is not necessarily wrong, but the
user just have to be aware of this.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/Documentation/glossary-content.txt b/Documentation/glossary-content.txt
index 0d2aa48c63856aba78d559445186a9b9e6bddd7d..023ca95e7c39d88fcac832d73a7454e9fb71e5a1 100644 (file)
--- a/Documentation/glossary-content.txt
+++ b/Documentation/glossary-content.txt
@@ -404,6 +404,8 @@ these forms:
 - "`!ATTR`" requires that the attribute `ATTR` be
   unspecified.
 +
 - "`!ATTR`" requires that the attribute `ATTR` be
   unspecified.
 +

+Note that when matching against a tree object, attributes are still
+obtained from working tree, not from the given tree object.

 
 exclude;;
        After a path matches any non-exclude pathspec, it will be run
 
 exclude;;
        After a path matches any non-exclude pathspec, it will be run

diff --git a/t/t6135-pathspec-with-attrs.sh b/t/t6135-pathspec-with-attrs.sh
index e436a7396241e6aad71a5f62500ffe0b9d7c0b7e..457cc167c774a0e4551cfc98c4f9aec6a17e1416 100755 (executable)
--- a/t/t6135-pathspec-with-attrs.sh
+++ b/t/t6135-pathspec-with-attrs.sh
@@ -31,7 +31,7 @@ test_expect_success 'setup a tree' '
        mkdir sub &&
        while read path
        do
        mkdir sub &&
        while read path
        do

-               : >$path &&
+               echo content >$path &&

                git add $path || return 1
        done <expect &&
        git commit -m "initial commit" &&
                git add $path || return 1
        done <expect &&
        git commit -m "initial commit" &&


@@ -48,6 +48,10 @@ test_expect_success 'pathspec with labels and non existent .gitattributes' '
        test_must_be_empty actual
 '
 
        test_must_be_empty actual
 '
 

+test_expect_success 'pathspec with labels and non existent .gitattributes (2)' '
+       test_must_fail git grep content HEAD -- ":(attr:label)"
+'
+

 test_expect_success 'setup .gitattributes' '
        cat <<-\EOF >.gitattributes &&
        fileA labelA
 test_expect_success 'setup .gitattributes' '
        cat <<-\EOF >.gitattributes &&
        fileA labelA


@@ -74,6 +78,15 @@ test_expect_success 'check specific set attr' '
        test_cmp expect actual
 '
 
        test_cmp expect actual
 '
 

+test_expect_success 'check specific set attr (2)' '
+       cat <<-\EOF >expect &&
+       HEAD:fileSetLabel
+       HEAD:sub/fileSetLabel
+       EOF
+       git grep -l content HEAD ":(attr:label)" >actual &&
+       test_cmp expect actual
+'
+

 test_expect_success 'check specific unset attr' '
        cat <<-\EOF >expect &&
        fileUnsetLabel
 test_expect_success 'check specific unset attr' '
        cat <<-\EOF >expect &&
        fileUnsetLabel


@@ -83,6 +96,15 @@ test_expect_success 'check specific unset attr' '
        test_cmp expect actual
 '
 
        test_cmp expect actual
 '
 

+test_expect_success 'check specific unset attr (2)' '
+       cat <<-\EOF >expect &&
+       HEAD:fileUnsetLabel
+       HEAD:sub/fileUnsetLabel
+       EOF
+       git grep -l content HEAD ":(attr:-label)" >actual &&
+       test_cmp expect actual
+'
+

 test_expect_success 'check specific value attr' '
        cat <<-\EOF >expect &&
        fileValue
 test_expect_success 'check specific value attr' '
        cat <<-\EOF >expect &&
        fileValue


@@ -94,6 +116,16 @@ test_expect_success 'check specific value attr' '
        test_must_be_empty actual
 '
 
        test_must_be_empty actual
 '
 

+test_expect_success 'check specific value attr (2)' '
+       cat <<-\EOF >expect &&
+       HEAD:fileValue
+       HEAD:sub/fileValue
+       EOF
+       git grep -l content HEAD ":(attr:label=foo)" >actual &&
+       test_cmp expect actual &&
+       test_must_fail git grep -l content HEAD ":(attr:label=bar)"
+'
+

 test_expect_success 'check unspecified attr' '
        cat <<-\EOF >expect &&
        .gitattributes
 test_expect_success 'check unspecified attr' '
        cat <<-\EOF >expect &&
        .gitattributes


@@ -118,6 +150,30 @@ test_expect_success 'check unspecified attr' '
        test_cmp expect actual
 '
 
        test_cmp expect actual
 '
 

+test_expect_success 'check unspecified attr (2)' '
+       cat <<-\EOF >expect &&
+       HEAD:.gitattributes
+       HEAD:fileA
+       HEAD:fileAB
+       HEAD:fileAC
+       HEAD:fileB
+       HEAD:fileBC
+       HEAD:fileC
+       HEAD:fileNoLabel
+       HEAD:fileWrongLabel
+       HEAD:sub/fileA
+       HEAD:sub/fileAB
+       HEAD:sub/fileAC
+       HEAD:sub/fileB
+       HEAD:sub/fileBC
+       HEAD:sub/fileC
+       HEAD:sub/fileNoLabel
+       HEAD:sub/fileWrongLabel
+       EOF
+       git grep -l ^ HEAD ":(attr:!label)" >actual &&
+       test_cmp expect actual
+'
+

 test_expect_success 'check multiple unspecified attr' '
        cat <<-\EOF >expect &&
        .gitattributes
 test_expect_success 'check multiple unspecified attr' '
        cat <<-\EOF >expect &&
        .gitattributes

diff --git a/tree-walk.c b/tree-walk.c
index 517bcdecf9ef2d452958e7b1e0dced5ae99569c0..08210a4109c70364e630290a24478333aeee449a 100644 (file)
--- a/tree-walk.c
+++ b/tree-walk.c
@@ -949,7 +949,8 @@ static enum interesting do_match(struct index_state *istate,
                       PATHSPEC_LITERAL |
                       PATHSPEC_GLOB |
                       PATHSPEC_ICASE |
                       PATHSPEC_LITERAL |
                       PATHSPEC_GLOB |
                       PATHSPEC_ICASE |

-                      PATHSPEC_EXCLUDE);
+                      PATHSPEC_EXCLUDE |
+                      PATHSPEC_ATTR);

 
        if (!ps->nr) {
                if (!ps->recursive ||
 
        if (!ps->nr) {
                if (!ps->recursive ||


@@ -981,14 +982,20 @@ static enum interesting do_match(struct index_state *istate,
 
                        if (!ps->recursive ||
                            !(ps->magic & PATHSPEC_MAXDEPTH) ||
 
                        if (!ps->recursive ||
                            !(ps->magic & PATHSPEC_MAXDEPTH) ||

-                           ps->max_depth == -1)
-                               return all_entries_interesting;
-
-                       return within_depth(base_str + matchlen + 1,
-                                           baselen - matchlen - 1,
-                                           !!S_ISDIR(entry->mode),
-                                           ps->max_depth) ?
-                               entry_interesting : entry_not_interesting;
+                           ps->max_depth == -1) {
+                               if (!item->attr_match_nr)
+                                       return all_entries_interesting;
+                               else
+                                       goto interesting;
+                       }
+
+                       if (within_depth(base_str + matchlen + 1,
+                                        baselen - matchlen - 1,
+                                        !!S_ISDIR(entry->mode),
+                                        ps->max_depth))
+                               goto interesting;
+                       else
+                               return entry_not_interesting;

                }
 
                /* Either there must be no base, or the base must match. */
                }
 
                /* Either there must be no base, or the base must match. */


@@ -996,12 +1003,12 @@ static enum interesting do_match(struct index_state *istate,
                        if (match_entry(item, entry, pathlen,
                                        match + baselen, matchlen - baselen,
                                        &never_interesting))
                        if (match_entry(item, entry, pathlen,
                                        match + baselen, matchlen - baselen,
                                        &never_interesting))

-                               return entry_interesting;
+                               goto interesting;

 
                        if (item->nowildcard_len < item->len) {
                                if (!git_fnmatch(item, match + baselen, entry->path,
                                                 item->nowildcard_len - baselen))
 
                        if (item->nowildcard_len < item->len) {
                                if (!git_fnmatch(item, match + baselen, entry->path,
                                                 item->nowildcard_len - baselen))

-                                       return entry_interesting;
+                                       goto interesting;

 
                                /*
                                 * Match all directories. We'll try to
 
                                /*
                                 * Match all directories. We'll try to


@@ -1022,7 +1029,7 @@ static enum interesting do_match(struct index_state *istate,
                                    !ps_strncmp(item, match + baselen,
                                                entry->path,
                                                item->nowildcard_len - baselen))
                                    !ps_strncmp(item, match + baselen,
                                                entry->path,
                                                item->nowildcard_len - baselen))

-                                       return entry_interesting;
+                                       goto interesting;

                        }
 
                        continue;
                        }
 
                        continue;


@@ -1057,7 +1064,7 @@ static enum interesting do_match(struct index_state *istate,
                if (!git_fnmatch(item, match, base->buf + base_offset,
                                 item->nowildcard_len)) {
                        strbuf_setlen(base, base_offset + baselen);
                if (!git_fnmatch(item, match, base->buf + base_offset,
                                 item->nowildcard_len)) {
                        strbuf_setlen(base, base_offset + baselen);

-                       return entry_interesting;
+                       goto interesting;

                }
 
                /*
                }
 
                /*


@@ -1071,7 +1078,7 @@ static enum interesting do_match(struct index_state *istate,
                    !ps_strncmp(item, match, base->buf + base_offset,
                                item->nowildcard_len)) {
                        strbuf_setlen(base, base_offset + baselen);
                    !ps_strncmp(item, match, base->buf + base_offset,
                                item->nowildcard_len)) {
                        strbuf_setlen(base, base_offset + baselen);

-                       return entry_interesting;
+                       goto interesting;

                }
 
                strbuf_setlen(base, base_offset + baselen);
                }
 
                strbuf_setlen(base, base_offset + baselen);


@@ -1085,6 +1092,38 @@ static enum interesting do_match(struct index_state *istate,
                 */
                if (ps->recursive && S_ISDIR(entry->mode))
                        return entry_interesting;
                 */
                if (ps->recursive && S_ISDIR(entry->mode))
                        return entry_interesting;

+               continue;
+interesting:
+               if (item->attr_match_nr) {
+                       int ret;
+
+                       /*
+                        * Must not return all_entries_not_interesting
+                        * prematurely. We do not know if all entries do not
+                        * match some attributes with current attr API.
+                        */
+                       never_interesting = entry_not_interesting;
+
+                       /*
+                        * Consider all directories interesting (because some
+                        * of those files inside may match some attributes
+                        * even though the parent dir does not)
+                        *
+                        * FIXME: attributes _can_ match directories and we
+                        * can probably return all_entries_interesting or
+                        * all_entries_not_interesting here if matched.
+                        */
+                       if (S_ISDIR(entry->mode))
+                               return entry_interesting;
+
+                       strbuf_add(base, entry->path, pathlen);
+                       ret = match_pathspec_attrs(istate, base->buf + base_offset,
+                                                  base->len - base_offset, item);
+                       strbuf_setlen(base, base_offset + baselen);
+                       if (!ret)
+                               continue;
+               }
+               return entry_interesting;

        }
        return never_interesting; /* No matches */
 }
        }
        return never_interesting; /* No matches */
 }