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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html