Re: [PATCH 1/2] apply: add unit tests for parse_range and rename to parse_fragment_range

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 4, 2024, 19:27 UTC
Message-ID: <xmqqsf01yllb.fsf@gitster.g>
In-Reply-To: <CAJ6X7_Uc0OdzYToJSs15+vbydraKAB8x4DPj7UsL1PKLzyY0dQ@mail.gmail.com>

Philip <philip.c.peterson@gmail.com> writes:

> On Mon, Feb 19, 2024 at 4:35 PM Junio C Hamano <gitster@pobox.com> wrote:
>>
>> "Philip Peterson via GitGitGadget" <gitgitgadget@gmail.com> writes:

It's quite a blast from a long time ago that I no longer remember.

Show 10 quoted lines

>> Alternatively we could do something like this to make the blast
>> radius of this patch smaller.
>>
>> -static int parse_range(const char *line, int len, int offset, const char *expect,
>> +#define apply_parse_fragment_range parse_range
>> +int parse_range(const char *line, int len, int offset, const char *expect,
>>                         unsigned long *p1, unsigned long *p2)
>
> From what I understand, this still creates a new extern symbol
> called parse_range.

Sorry, I misspoke. The direction of #define is the other way around. That is, we may have to give the function a name that is overly long because it needs to be externally linkable only to support for your test, but we would want to locally rename that long name down to the name currently used by the primary callers of that function, so that the patch does not have to touch these existing calling sites. After all, this function is a small implementation detail and not a part of the official apply.c API, and the only reason why we are making it extern is because some new tests want to link with it from the side.

So, in the <apply.h> header file you'll do

	/* 
	 * exposed only for tests; do not call this as it not
	 * a part of the API
	 */
	extern int apply_parse_fragment_range(...);

and then in the original file that used to have "static int parse_range(...)", you'd add

	#define parse_range apply_parse_fragment_range

near the top, after the header files are included.

Thanks.