Re: [PATCH] apply: reload .gitattributes after patching it

[Jeff King <peff@peff.net>]

On Fri, Aug 09, 2019 at 02:43:18PM +0200, SZEDER Gábor wrote:

> > I think you could do this with:
> > 
> >   size_t len;
> >   if (strip_suffix(patch->new_name, GITATTRIBUTES_FILE, &len) &&
> >       len > 0 && is_dir_sep(patch->new_name[len-1]))
> >           flush_attributes = 1;
> > 
> > Not sure if that is better or worse. It avoids the extra memory
> > boilerplate, but the logic is a slightly more subtle.
> 
> Subtle indeed :) because you have to allow len == 0 to catch the
> toplevel .gitattributes file.

I'll pretend I was leaving that as a puzzle for the readers. :)

> But there is an other subtlety here: when I read the commit message
> saying "patch that touches a path ending in ".gitattributes"." and saw
> the new call to strip_path_suffix(), I immediately thought what would
> happen with a file called 'foo.gitattributes'.  Only when I looked
> into strip_path_suffix() became it clear that it only removes full
> path components, so such a filename won't cause any trouble (though
> perhaps the worst thing that could happen is that we unnecessarily
> flush the attributes cache).

Right. I think the term we want here is really "basename". So in fact:

  if (!strcmp(basename(patch->new_name), GITATTRIBUTES_FILE))

would do what we want, except for the annoying caveat that basename() is
allowed to modify its parameter (to remove trailing directory
separators, but we know we wouldn't have them here).

-Peff