On 09/27/2016 06:37 AM, Jeff King wrote:
> Commit e8adf23 (xdl_change_compact(): introduce the concept
> of a change group, 2016-08-22) added a "struct group" type
> to xdiff/xdiffi.c. But the POSIX system header "grp.h"
> already defines "struct group" (it is part of the getgrnam
> interface). This happens to work because the new type is
> local to xdiffi.c, and the xdiff code includes a relatively
> small set of system headers. But it will break compilation
> if xdiff ever switches to using git-compat-util.h.  It can
> also probably cause confusion with tools that look at the
> whole code base, like coccinelle or ctags.
> 
> Let's resolve by giving the xdiff variant a scoped name,
> which is closer to other xdiff types anyway (e.g.,
> xdlfile_t, though note that xdiff is fond if typedefs when
> Git usually is not).

Makes sense to me. I didn't try to adhere to xdiff conventions too
tightly because I don't think that project is alive anymore, so I don't
expect we'll be upstreaming anything [1]. But this change definitely
makes sense.

Thanks,
Michael

[1] Though I've since learned that libgit2 also bases their diff code on
xdiff, so if we avoid changing things gratuitously there is more chance
that our two projects can benefit from each other's improvements
whenever they are also licensed compatibly.

Reply via email to