Re: [PATCH v2 1/1] xdiff: provide indirection to git functions

[Junio C Hamano <gitster@pobox.com>]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> Hi,
>
> On Tue, 22 Feb 2022, Phillip Wood wrote:
>
>> On 17/02/2022 22:54, Edward Thomson wrote:
>> > Provide an indirection layer into the git-specific functionality and
>> > utilities in `git-xdiff.h`, prefixing those types and functions with
>> > `xdl_` (and `XDL_` for macros).  This allows other projects that use
>> > git's xdiff implementation to keep up-to-date; they can now take all the
>> > files _except_ `git-xdiff.h`, which they have customized for their own
>> > environment.
>>
>> The changes since V1 look good,
>
> Indeed. This is the range-diff:
>
> -- snip --
> 1:  52c8f141cbe1 ! 1:  e05e9b5e2f27 xdiff: provide indirection to git functions
>     @@ xdiff/git-xdiff.h (new)
>      +#ifndef GIT_XDIFF_H
>      +#define GIT_XDIFF_H
>      +
>     ++#include "git-compat-util.h"
>     ++
>      +#define xdl_malloc(x) xmalloc(x)
>      +#define xdl_free(ptr) free(ptr)
>      +#define xdl_realloc(ptr,x) xrealloc(ptr,x)
>     @@ xdiff/xdiffi.c: static void xdl_mark_ignorable_lines(xdchange_t *xscr, xdfenv_t
>
>       ## xdiff/xinclude.h ##
>      @@
>     + #if !defined(XINCLUDE_H)
>       #define XINCLUDE_H
>
>     - #include "git-compat-util.h"
>     +-#include "git-compat-util.h"
>      +#include "git-xdiff.h"
>       #include "xmacros.h"
>       #include "xdiff.h"
>       #include "xtypes.h"
>     -@@
>     - #include "xdiffi.h"
>     - #include "xemit.h"
>     +
>     + ## xdiff/xmerge.c ##
>     +@@ xdiff/xmerge.c: static int xdl_cleanup_merge(xdmerge_t *c)
>     + 		if (c->mode == 0)
>     + 			count++;
>     + 		next_c = c->next;
>     +-		free(c);
>     ++		xdl_free(c);
>     + 	}
>     + 	return count;
>     + }
>     +@@ xdiff/xmerge.c: static void xdl_merge_two_conflicts(xdmerge_t *m)
>     + 	m->chg1 = next_m->i1 + next_m->chg1 - m->i1;
>     + 	m->chg2 = next_m->i2 + next_m->chg2 - m->i2;
>     + 	m->next = next_m->next;
>     +-	free(next_m);
>     ++	xdl_free(next_m);
>     + }
>
>     --
>     - #endif /* #if !defined(XINCLUDE_H) */
>     + /*
> -- snap --
>
> My ACK from
> https://lore.kernel.org/git/nycvar.QRO.7.76.6.2202171644090.348@tvgsbejvaqbjf.bet/
> still holds. Junio could you please add it before merging it down to
> `next`?

Not so fast.  I still do not see a strong reason to support
xdl_malloc() and other wrappers.

Is the expectation for other projects when using the unified code,
they do not use xdiff/git-xdiff.h and instead add
xdiff/frotz-xdiff.h that defines xdl_malloc() and friends with the
infrastructure they provide as part of the Frotz project (and the
Xyzzy project would do the same with xdiff/xyzzy-xdiff.h header for
them), making "git" the first among equal other consumers?

If that is the direction this indirection is aiming for, stating it
clearly may be a start of a not-so-bad justification, but then the
hardcoded inclusion of "git-xdiff.h" in xdiff/xinclude.h still
contradicts with it, which may want to be fixed.