Re: [PATCH 1/2] test-lib: add LIBPCRE1 & LIBPCRE2 prerequisites

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Nov 23, 2017, 09:11 UTC
Message-ID: <87d149we9a.fsf@evledraar.booking.com>
In-Reply-To: <20171122202259.GC11671@aiede.mtv.corp.google.com>

On Wed, Nov 22 2017, Jonathan Nieder jotted:

Show 38 quoted lines

> Hi,
>
> Ævar Arnfjörð Bjarmason wrote:
>
>> Add LIBPCRE1 and LIBPCRE2 prerequisites which are true when git is
>> compiled with USE_LIBPCRE1=YesPlease or USE_LIBPCRE2=YesPlease,
>> respectively.
>>
>> The syntax of PCRE1 and PCRE2 isn't the same in all cases (see
>> pcresyntax(3) and pcre2syntax(3)). If test are added that test for
>> those they'll need to be guarded by these new prerequisites.
>>
>> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
>> ---
>>  t/README      | 12 ++++++++++++
>>  t/test-lib.sh |  2 ++
>>  2 files changed, 14 insertions(+)
>>
>> diff --git a/t/README b/t/README
>> index 4b079e4494..599cd9808c 100644
>> --- a/t/README
>> +++ b/t/README
>> @@ -808,6 +808,18 @@ use these, and "test_set_prereq" for how to define your own.
>>     Git was compiled with support for PCRE. Wrap any tests
>>     that use git-grep --perl-regexp or git-grep -P in these.
>>
>> + - LIBPCRE1
>> +
>> +   Git was compiled with PCRE v1 support via
>> +   USE_LIBPCRE1=YesPlease. Wrap any PCRE using tests that for some
>> +   reason need v1 of the PCRE library instead of v2 in these.
>
> Are there plans to use the LIBPCRE1 prereq?  It might be simpler to
> only have LIBPCRE2, and LIBPCRE1 can still be expressed as
>
> 	PCRE,!LIBPCRE2
>
> which I think is clearer about the intent.

I prefer to keep it as it is. It's more obvious to me to have a 1=1 mapping between the ${USE,NO}_* variables and the prerequisites, and it's future-proof if there's ever a PCRE v3, since tests that use this will mean v1 specifically, not just any non-v2 version (although now v1 is the only one).