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
