All of 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.