From mboxrd@z Thu Jan  1 00:00:00 1970
From: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Date: Sat, 13 Apr 2019 21:58:44 +0200
Subject: [Buildroot] [PATCH 2/2] package/php: fix how external PCRE2 JIT
 is enabled
In-Reply-To: <20190215232059.31440-2-panfilov.artyom@gmail.com>
References: <20190215232059.31440-1-panfilov.artyom@gmail.com>
 <20190215232059.31440-2-panfilov.artyom@gmail.com>
Message-ID: <20190413215844.4b12fbeb@windsurf>
List-Id: <buildroot.busybox.net>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: buildroot@busybox.net

Hello Artem,

On Sat, 16 Feb 2019 02:20:59 +0300
Artem Panfilov <panfilov.artyom@gmail.com> wrote:

> When the external PCRE2 library is used, PHP JIT option is enabled
> based on architecture. If the external library was compiled without
> JIT support runtime will fail.
> 
> When I call "php -m" command, I have the following error message:
> PHP Fatal error:  Unable to start pcre module in Unknown on line 0
> 
> Signed-off-by: Artem Panfilov <panfilov.artyom@gmail.com>

Thanks for this patch. See some comments below.

> diff --git a/package/php/0006-pcre2-tweak-pcre2-jit-detection.patch b/package/php/0006-pcre2-tweak-pcre2-jit-detection.patch
> new file mode 100644
> index 0000000000..492abf008b
> --- /dev/null
> +++ b/package/php/0006-pcre2-tweak-pcre2-jit-detection.patch
> @@ -0,0 +1,41 @@
> +diff --git a/ext/pcre/config0.m4 b/ext/pcre/config0.m4

We need all patches to have a Signed-off-by and a description. Also,
since PHP is maintained under Git upstream I believe, the patch should
be generated by Git format-patch.

> +index b9542f0113..f4c66a8369 100644
> +--- a/ext/pcre/config0.m4
> ++++ b/ext/pcre/config0.m4
> +@@ -53,35 +53,7 @@ PHP_ARG_WITH(pcre-jit,,[  --with-pcre-jit
> Enable PCRE JIT functionality
> +     AC_DEFINE(HAVE_PCRE, 1, [ ])
> + 
> +     if test "$PHP_PCRE_JIT" != "no"; then
> +-      AC_MSG_CHECKING([for JIT support in PCRE2])
> +-      AC_RUN_IFELSE([
> +-        AC_LANG_SOURCE([[
> +-            #include <pcre2.h>
> +-            #include <stdlib.h>
> +-            int main(void) {
> +-              uint32_t have_jit;
> +-              pcre2_config_8(PCRE2_CONFIG_JIT, &have_jit);
> +-              return !have_jit;
> +-            }
> +-        ]])], [
> +-        AC_MSG_RESULT([yes])
> +-        AC_DEFINE(HAVE_PCRE_JIT_SUPPORT, 1, [])
> +-      ],
> +-      [
> +-        AC_MSG_RESULT([no])
> +-      ],
> +-      [
> +-        AC_CANONICAL_HOST
> +-        case $host_cpu in
> +-        arm*|i[34567]86|x86_64|mips*|powerpc*|sparc)
> +-          AC_MSG_RESULT([yes])
> +-          AC_DEFINE(HAVE_PCRE_JIT_SUPPORT, 1, [])
> +-          ;;
> +-        *)
> +-          AC_MSG_RESULT([no])
> +-          ;;
> +-        esac
> +-      ])
> ++      AC_DEFINE(HAVE_PCRE_JIT_SUPPORT, 1, [])

This change is not really great, as it cannot be contributed upstream.
Better approaches would be

 * Add an autoconf cache variable
   (https://www.gnu.org/software/autoconf/manual/autoconf-2.69/html_node/Caching-Results.html)
   that allows to preseed the fact that the pcre2 library is built with
   JIT support, and therefore avoid the AC_TRY_RUN check. This is fully
   backward compatible.

 * Change the semantic of the option. If --with-pcre-jit is passed,
   assume it is available. Only if it is *not* passed do the AC_TRY_RUN
   check. However, this is changing a bit the semantic of the option.

So perhaps the cache variable is the least annoying solution. Could you
have a look at implementing this ?

Thanks a lot!

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com