Re: [PATCH v2 2/2] attr: more matching optimizations from .gitignore

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Nguyễn Thái Ngọc Duy <pclouds@xxxxxxxxx> writes:

> .gitattributes and .gitignore share the same pattern syntax but has
> separate matching implementation. Over the years, ignore's
> implementation accumulates more optimizations while attr's stays the
> same.
>
> This patch adds those optimizations to attr. Basically it tries to
> avoid fnmatch as much as possible in favor of strncmp.
>
> A few notes from this work is put in the documentation:
>
> * "!pattern" syntax is not supported in .gitattributes as it's not

s/not supported/forbidden/;

A reader can take "not supported" as "silently ignored", which is
not the case.  An explicit "forbidden" does not have such a
misinterpretation.

It would save time from both of us if you can check what is queued
on 'pu'.  I do not think I touched the code for off-by-one bugs
there, though.

>   clear what it means (e.g. "!path attr" is about unsetting attr, or
>   undefining it..)
>
> * patterns applying to directories
>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@xxxxxxxxx>
> ---
>  How about this? Diff from the previous version:
>
>    diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt
>    index cc2ff1d..9a0ed19 100644
>    --- a/Documentation/gitattributes.txt
>    +++ b/Documentation/gitattributes.txt
>    @@ -23,7 +23,7 @@ Each line in `gitattributes` file is of form:
>     That is, a pattern followed by an attributes list,
>     separated by whitespaces.  When the pattern matches the
>     path in question, the attributes listed on the line are given to
>    -the path. Only files can be attached attributes to.
>    +the path.
>     
>     Each attribute can be in one of these states for a given path:
>     
>    @@ -58,6 +58,13 @@ attribute.  The rules how the pattern matches paths are the
>     same as in `.gitignore` files; see linkgit:gitignore[5].
>     Unlike `.gitignore`, negative patterns are not supported.
>     
>    +Note that if a .gitignore rule matches a directory, the directory
>    +is ignored, which may be seen as assigning "ignore" attribute the
>    +directory and all files and directories inside. However, if a
>    +.gitattributes rule matches a directory, it manipulates
>    +attributes on that directory only, not files and directories
>    +inside.

Why do you even need to mention .gitignore in gitattributes manual
where it is irrelevant from the reader's point of view?

Besides, the interpretation the "may be seen as" suggests is
actively wrong.  It is assigning "ignore-this-and-below" attribute
to the directory, and there is no inconsistency between the two.

Again, I'd suggest dropping this addition.

>    diff --git a/attr.c b/attr.c
>    index 7e85f82..4faf1ff 100644
>    --- a/attr.c
>    +++ b/attr.c
>    @@ -250,7 +250,6 @@ static struct match_attr *parse_attr_line(const char *line, const char *src,
>     	else {
>     		char *p = (char *)&(res->state[num_attr]);
>     		memcpy(p, name, namelen);
>    -		p[namelen] = 0;
>     		res->u.pat.pattern = p;
>     		parse_exclude_pattern(&res->u.pat.pattern,
>     				      &res->u.pat.patternlen,
>    @@ -690,16 +689,18 @@ static int path_matches(const char *pathname, int pathlen,
>     	 * contain the trailing slash
>     	 */
>     
>    -	if (pathlen < baselen ||
>    +	if (pathlen < baselen + 1 ||
>     	    (baselen && pathname[baselen] != '/') ||
>    -	    strncmp(pathname, base, baselen))
>    +	    strncmp_icase(pathname, base, baselen))
>     		return 0;
>     
>     	namelen = baselen ? pathlen - baselen - 1 : pathlen;
>     	name = pathname + pathlen - namelen;
>     
>    -	/* if the non-wildcard part is longer than the remaining
>    -	   pathname, surely it cannot match */
>    +	/*
>    +	 * if the non-wildcard part is longer than the remaining
>    +	 * pathname, surely it cannot match
>    +	 */
>     	if (!namelen || prefix > namelen)
>     		return 0;

--
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


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]