qOn Tue, Mar 26, 2013 at 11:39:27AM -0700, Junio C Hamano wrote:
> So here is an attempt to fix the unintended regression, on top of
> 9db9eecfe5c2 (attr: avoid calling find_basename() twice per path,
> 2013-01-16).  It consists of four patches.

Not that I disagree with this. Just wanted to see how far the "dtype"
idea went. How about this? git_check_attr() now takes dtype as an
argument and the caller must not add the trailing slash. This could be
split into two patches, one for git_check_attr prototype change, and
the other the real meat.

-- 8< --
diff --git a/archive.c b/archive.c
index 93e00bb..ab811b3 100644
--- a/archive.c
+++ b/archive.c
@@ -112,7 +112,7 @@ static int write_archive_entry(const unsigned char *sha1, 
const char *base,
        write_archive_entry_fn_t write_entry = c->write_entry;
        struct git_attr_check check[2];
        const char *path_without_prefix;
-       int err;
+       int err, dtype;
 
        args->convert = 0;
        strbuf_reset(&path);
@@ -120,18 +120,18 @@ static int write_archive_entry(const unsigned char *sha1, 
const char *base,
        strbuf_add(&path, args->base, args->baselen);
        strbuf_add(&path, base, baselen);
        strbuf_addstr(&path, filename);
-       if (S_ISDIR(mode) || S_ISGITLINK(mode))
-               strbuf_addch(&path, '/');
+       dtype = S_ISDIR(mode) || S_ISGITLINK(mode) ? DT_DIR : DT_REG;
        path_without_prefix = path.buf + args->baselen;
 
        setup_archive_check(check);
-       if (!git_check_attr(path_without_prefix, ARRAY_SIZE(check), check)) {
+       if (!git_check_attr(path_without_prefix, dtype, ARRAY_SIZE(check), 
check)) {
                if (ATTR_TRUE(check[0].value))
                        return 0;
                args->convert = ATTR_TRUE(check[1].value);
        }
 
        if (S_ISDIR(mode) || S_ISGITLINK(mode)) {
+               strbuf_addch(&path, '/');
                if (args->verbose)
                        fprintf(stderr, "%.*s\n", (int)path.len, path.buf);
                err = write_entry(args, sha1, path.buf, path.len, mode);
diff --git a/attr.c b/attr.c
index e2f9377..ab6422c 100644
--- a/attr.c
+++ b/attr.c
@@ -255,6 +255,8 @@ static struct match_attr *parse_attr_line(const char *line, 
const char *src,
                                      &res->u.pat.patternlen,
                                      &res->u.pat.flags,
                                      &res->u.pat.nowildcardlen);
+               if (res->u.pat.flags & EXC_FLAG_MUSTBEDIR)
+                       p[res->u.pat.patternlen] = '\0';
                if (res->u.pat.flags & EXC_FLAG_NEGATIVE) {
                        warning(_("Negative patterns are ignored in git 
attributes\n"
                                  "Use '\\!' for literal leading 
exclamation."));
@@ -657,15 +659,14 @@ static void prepare_attr_stack(const char *path, int 
dirlen)
 }
 
 static int path_matches(const char *pathname, int pathlen,
-                       const char *basename,
+                       const char *basename, int dtype,
                        const struct pattern *pat,
                        const char *base, int baselen)
 {
        const char *pattern = pat->pattern;
        int prefix = pat->nowildcardlen;
 
-       if ((pat->flags & EXC_FLAG_MUSTBEDIR) &&
-           ((!pathlen) || (pathname[pathlen-1] != '/')))
+       if ((pat->flags & EXC_FLAG_MUSTBEDIR) && dtype != DT_DIR)
                return 0;
 
        if (pat->flags & EXC_FLAG_NODIR) {
@@ -704,7 +705,7 @@ static int fill_one(const char *what, struct match_attr *a, 
int rem)
 }
 
 static int fill(const char *path, int pathlen, const char *basename,
-               struct attr_stack *stk, int rem)
+               int dtype, struct attr_stack *stk, int rem)
 {
        int i;
        const char *base = stk->origin ? stk->origin : "";
@@ -713,7 +714,7 @@ static int fill(const char *path, int pathlen, const char 
*basename,
                struct match_attr *a = stk->attrs[i];
                if (a->is_macro)
                        continue;
-               if (path_matches(path, pathlen, basename,
+               if (path_matches(path, pathlen, basename, dtype,
                                 &a->u.pat, base, stk->originlen))
                        rem = fill_one("fill", a, rem);
        }
@@ -748,20 +749,17 @@ static int macroexpand_one(int attr_nr, int rem)
  * Collect all attributes for path into the array pointed to by
  * check_all_attr.
  */
-static void collect_all_attrs(const char *path)
+static void collect_all_attrs(const char *path, int dtype)
 {
        struct attr_stack *stk;
        int i, pathlen, rem, dirlen;
-       const char *basename, *cp, *last_slash = NULL;
+       const char *basename, *cp;
 
-       for (cp = path; *cp; cp++) {
-               if (*cp == '/' && cp[1])
-                       last_slash = cp;
-       }
-       pathlen = cp - path;
-       if (last_slash) {
-               basename = last_slash + 1;
-               dirlen = last_slash - path;
+       cp = strrchr(path, '/');
+       pathlen = strlen(path);
+       if (cp) {
+               basename = cp + 1;
+               dirlen = cp - path;
        } else {
                basename = path;
                dirlen = 0;
@@ -773,14 +771,14 @@ static void collect_all_attrs(const char *path)
 
        rem = attr_nr;
        for (stk = attr_stack; 0 < rem && stk; stk = stk->prev)
-               rem = fill(path, pathlen, basename, stk, rem);
+               rem = fill(path, pathlen, basename, dtype, stk, rem);
 }
 
-int git_check_attr(const char *path, int num, struct git_attr_check *check)
+int git_check_attr(const char *path, int dtype, int num, struct git_attr_check 
*check)
 {
        int i;
 
-       collect_all_attrs(path);
+       collect_all_attrs(path, dtype);
 
        for (i = 0; i < num; i++) {
                const char *value = 
check_all_attr[check[i].attr->attr_nr].value;
@@ -796,7 +794,7 @@ int git_all_attrs(const char *path, int *num, struct 
git_attr_check **check)
 {
        int i, count, j;
 
-       collect_all_attrs(path);
+       collect_all_attrs(path, DT_REG);
 
        /* Count the number of attributes that are set. */
        count = 0;
diff --git a/attr.h b/attr.h
index 8b08d33..ce39c9c 100644
--- a/attr.h
+++ b/attr.h
@@ -36,7 +36,7 @@ struct git_attr_check {
  */
 char *git_attr_name(struct git_attr *);
 
-int git_check_attr(const char *path, int, struct git_attr_check *);
+int git_check_attr(const char *path, int, int, struct git_attr_check *);
 
 /*
  * Retrieve all attributes that apply to the specified path.  *num
diff --git a/builtin/check-attr.c b/builtin/check-attr.c
index 075d01d..261e57d 100644
--- a/builtin/check-attr.c
+++ b/builtin/check-attr.c
@@ -49,7 +49,7 @@ static void check_attr(const char *prefix, int cnt,
        char *full_path =
                prefix_path(prefix, prefix ? strlen(prefix) : 0, file);
        if (check != NULL) {
-               if (git_check_attr(full_path, cnt, check))
+               if (git_check_attr(full_path, DT_REG, cnt, check))
                        die("git_check_attr died");
                output_attr(cnt, check, file);
        } else {
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index f069462..7a77288 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -894,7 +894,7 @@ static int no_try_delta(const char *path)
        struct git_attr_check check[1];
 
        setup_delta_attr_check(check);
-       if (git_check_attr(path, ARRAY_SIZE(check), check))
+       if (git_check_attr(path, DT_REG, ARRAY_SIZE(check), check))
                return 0;
        if (ATTR_FALSE(check->value))
                return 1;
diff --git a/convert.c b/convert.c
index 3520252..3f09cbb 100644
--- a/convert.c
+++ b/convert.c
@@ -755,7 +755,7 @@ static void convert_attrs(struct conv_attrs *ca, const char 
*path)
                git_config(read_convert_config, NULL);
        }
 
-       if (!git_check_attr(path, NUM_CONV_ATTRS, ccheck)) {
+       if (!git_check_attr(path, DT_REG, NUM_CONV_ATTRS, ccheck)) {
                ca->crlf_action = git_path_check_crlf(path, ccheck + 4);
                if (ca->crlf_action == CRLF_GUESS)
                        ca->crlf_action = git_path_check_crlf(path, ccheck + 0);
diff --git a/ll-merge.c b/ll-merge.c
index fb61ea6..1944392 100644
--- a/ll-merge.c
+++ b/ll-merge.c
@@ -342,7 +342,7 @@ static int git_path_check_merge(const char *path, struct 
git_attr_check check[2]
                check[0].attr = git_attr("merge");
                check[1].attr = git_attr("conflict-marker-size");
        }
-       return git_check_attr(path, 2, check);
+       return git_check_attr(path, DT_REG, 2, check);
 }
 
 static void normalize_file(mmfile_t *mm, const char *path)
@@ -399,7 +399,7 @@ int ll_merge_marker_size(const char *path)
 
        if (!check.attr)
                check.attr = git_attr("conflict-marker-size");
-       if (!git_check_attr(path, 1, &check) && check.value) {
+       if (!git_check_attr(path, DT_REG, 1, &check) && check.value) {
                marker_size = atoi(check.value);
                if (marker_size <= 0)
                        marker_size = DEFAULT_CONFLICT_MARKER_SIZE;
diff --git a/t/t5002-archive-attr-pattern.sh b/t/t5002-archive-attr-pattern.sh
index 0c847fb..98ccc3c 100755
--- a/t/t5002-archive-attr-pattern.sh
+++ b/t/t5002-archive-attr-pattern.sh
@@ -27,6 +27,10 @@ test_expect_success 'setup' '
        echo ignored-only-if-dir/ export-ignore >>.git/info/attributes &&
        git add ignored-only-if-dir &&
 
+       mkdir -p ignored-without-slash &&
+       echo ignored without slash >ignored-without-slash/foo &&
+       git add ignored-without-slash/foo &&
+       echo ignored-without-slash export-ignore >>.git/info/attributes &&
 
        mkdir -p one-level-lower/two-levels-lower/ignored-only-if-dir &&
        echo ignored by ignored dir 
>one-level-lower/two-levels-lower/ignored-only-if-dir/ignored-by-ignored-dir &&
@@ -49,6 +53,8 @@ test_expect_exists    
archive/not-ignored-dir/ignored-only-if-dir
 test_expect_exists     archive/not-ignored-dir/
 test_expect_missing    archive/ignored-only-if-dir/
 test_expect_missing    archive/ignored-ony-if-dir/ignored-by-ignored-dir
+test_expect_missing    archive/ignored-without-slash/ &&
+test_expect_missing    archive/ignored-without-slash/foo &&
 test_expect_exists     archive/one-level-lower/
 test_expect_missing    
archive/one-level-lower/two-levels-lower/ignored-only-if-dir/
 test_expect_missing    
archive/one-level-lower/two-levels-lower/ignored-ony-if-dir/ignored-by-ignored-dir
diff --git a/userdiff.c b/userdiff.c
index ea43a03..fd4f576 100644
--- a/userdiff.c
+++ b/userdiff.c
@@ -260,7 +260,7 @@ struct userdiff_driver *userdiff_find_by_path(const char 
*path)
 
        if (!path)
                return NULL;
-       if (git_check_attr(path, 1, &check))
+       if (git_check_attr(path, DT_REG, 1, &check))
                return NULL;
 
        if (ATTR_TRUE(check.value))
diff --git a/ws.c b/ws.c
index b498d75..34c2145 100644
--- a/ws.c
+++ b/ws.c
@@ -88,7 +88,7 @@ unsigned whitespace_rule(const char *pathname)
        struct git_attr_check attr_whitespace_rule;
 
        setup_whitespace_attr_check(&attr_whitespace_rule);
-       if (!git_check_attr(pathname, 1, &attr_whitespace_rule)) {
+       if (!git_check_attr(pathname, DT_REG, 1, &attr_whitespace_rule)) {
                const char *value;
 
                value = attr_whitespace_rule.value;
-- 8< --
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to