Re: [regression?] trailing slash required in .gitattributes

On Tue, Mar 19, 2013 at 02:10:42PM -0400, Jeff King wrote:

quoted

The issue bisects to 94bc671 (Add directory pattern matching to
attributes, 2012-12-08). That commit actually tests not only that
"subdir/" matches, but also that just "subdir" does not match.

[...]
So I think the regression is accidental. And we would want tests like
this on top (which currently fail):
[...]

I'm having trouble figuring out the right solution for this.

The problem is in path_matches, which used to receive just the unadorned
pathname, and now receives "path/" for directories. It now looks like
this:

static int path_matches(const char *pathname, int pathlen,
			const char *basename,
			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] != '/')))
		return 0;

This first stanza checks that a pattern like "foo/" must be matched by a
real directory. Which is fine; that's the point of adding the "/" to the
pattern.

	if (pat->flags & EXC_FLAG_NODIR) {
		return match_basename(basename,
				      pathlen - (basename - pathname),
				      pattern, prefix,
				      pat->patternlen, pat->flags);
	}
	return match_pathname(pathname, pathlen,
			      base, baselen,
			      pattern, prefix, pat->patternlen, pat->flags);
}

But then here we'll end up feeding "foo/" to be compared with "foo",
which we don't want. For a pattern "foo", we want to match _either_
"foo/" or "foo". So you'd think something like:

  if (pathlen && pathname[pathlen-1] == '/')
          pathlen--;

would work. But it seems that match_basename, despite taking the length
of all of the strings we pass it, will happily use NUL-terminated
functions like strcmp or fnmatch. Converting the former to check lengths
should be pretty straightforward. But there is no version of fnmatch
that does what we want. I wonder if we using wildmatch can get around
this limitation.

-Peff