Buildroot Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
To: buildroot@busybox.net
Subject: [Buildroot] [PATCH 2/2] package/php: fix how external PCRE2 JIT is enabled
Date: Sat, 13 Apr 2019 21:58:44 +0200	[thread overview]
Message-ID: <20190413215844.4b12fbeb@windsurf> (raw)
In-Reply-To: <20190215232059.31440-2-panfilov.artyom@gmail.com>

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

  reply	other threads:[~2019-04-13 19:58 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-02-15 23:20 [Buildroot] [PATCH 1/2] package/pcre2: add JIT support option Artem Panfilov
2019-02-15 23:20 ` [Buildroot] [PATCH 2/2] package/php: fix how external PCRE2 JIT is enabled Artem Panfilov
2019-04-13 19:58   ` Thomas Petazzoni [this message]
2019-04-13 19:54 ` [Buildroot] [PATCH 1/2] package/pcre2: add JIT support option Thomas Petazzoni

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190413215844.4b12fbeb@windsurf \
    --to=thomas.petazzoni@bootlin.com \
    --cc=buildroot@busybox.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox