Re: [RFC-PATCHv6 4/4] pathspec: allow querying for attributes

[Junio C Hamano <gitster@pobox.com>]

Stefan Beller <sbeller@google.com> writes:

> + * attr:+val to find value set to true
> + * attr:-val to find a value set to false
> + * attr:!val to find a value that is not set
> + *     (i.e. it is neither set as "val", "val=<empty>", nor unset as "-val")
> + * attr:val=value: to find value that have at least a and b set.

I would have expected that there won't be "attr:+val", but it is
spelled as "attr:val" instead.

> +static void parse_attr_item(struct attr_item *attr, const char *value)

Please do not call something that is not part of the attribute
infrastructure as "attr_item"; I wasted time looking for the
structure definition for "attr_item" in <attr.h>.

> +static int match_attrs(const char *name, int namelen,
> +		       const struct pathspec_item *item)
> +{
> +	char *path;
> +	int i;
> +
> +	if (!check) {
> +		check = git_attr_check_alloc();
> +		for (i = 0; i < item->attr_nr; i++)
> +			git_attr_check_append(check, item->attrs[i].attr);
> +	}
> +
> +	path = xmemdupz(name, namelen);
> +	git_all_attrs(path, check);

PLEASE DON'T.  git_all_attrs() asks for all the attribute under the
sun and has no hope to perform sensibly, especially at the very leaf
level of the pathspec logic where one call to this function is made
for each and every path in the tree.

Instead, have a pointer to "struct git_attr_check" in pathspec_item
and make a call to git_check_attr(path, item->check) here.

Which means that you would need to prepare git_attr_check around ...

> +		if (skip_prefix(copyfrom, "attr:", &body)) {
> +			ALLOC_GROW(item->attrs, item->attr_nr + 1, item->attr_alloc);
> +			parse_attr_item(&item->attrs[item->attr_nr++], body);

... HERE.