Re: [PATCH 5/7] t0060: test obscured .gitattributes and .gitignore matching

[Jeff King <peff@peff.net>]

On Mon, Oct 05, 2020 at 01:03:53AM -0700, Jonathan Nieder wrote:

> > Note that the ntfs magic prefix names in the tests come from the
> > algorithm described in e7cb0b4455 (and are different for each file).
> 
> Doesn't block this patch, but I'm curious: how hard would it be to make
> a test with an NTFS prerequisite that makes sure we got the magic prefix
> right?

I suspect hard since Dscho punted on it in the original series. :) If I
understand correctly, it would require having an NTFS filesystem, and
generating 10,000+ files with a clashing prefix.

> > +	for (; *argv; argv++) {
> > +		if (!strcmp("--not", *argv))
> > +			expect = !expect;
> > +		else if (expect != (is_hfs(*argv) || is_ntfs(*argv)))
> > +			 res = error("'%s' is %s.%s", *argv,
> > +				     expect ? "not " : "", x);
> > +		else
> > +			fprintf(stderr, "ok: '%s' is %s.%s\n",
> > +				*argv, expect ? "" : "not ", x);
> 
> micronit: extra space on the "res" line.

Thanks, fixed.

> This "if" cascade is a little hard to read, even though it does the
> right thing.  Can we make it more explicit?  E.g.

This is directly moved from the existing code. I'd prefer to keep the
overall structure intact to make that clear.

> I think it's a little easier to read with either (a) the dot included
> in the 'x' parameter or (b) the entire ".git" missing from the 'x'
> parameter.

Yeah, I agree that's worth doing. I took (b), as "dotgitx" implies that
"x" is "modules", etc. I had originally planned to automatically turn
"gitmodules" into "is_ntfs_dotgitmodules", too, but it required
macros and string-pasting. So I decided it was a bit too ugly. :)

-Peff