[Buildroot] [PATCH v2,1/4] apache: add customization of MPM

[Buildroot] [PATCH v2,1/4] apache: add customization of MPM

[Fabrice Fontaine]

MPM can be selected between event, prefork or worker
Set worker as the default one as it was before even if event MPM is
better on system supporting thread safe polling

Signed-off-by: Fabrice Fontaine <fabrice.fontaine@orange.com>
---
Changes v1 -> v2 (after review of Yann Morin):
 - Do not use unneeded variables in Config.in and apache.mk

 package/apache/Config.in | 28 ++++++++++++++++++++++++++++
 package/apache/apache.mk | 10 +++++++++-
 2 files changed, 37 insertions(+), 1 deletion(-)

diff --git a/package/apache/Config.in b/package/apache/Config.in
index 0814a17..511876d 100644
--- a/package/apache/Config.in
+++ b/package/apache/Config.in
@@ -14,6 +14,34 @@ config BR2_PACKAGE_APACHE
 
 	  http://httpd.apache.org
 
+if BR2_PACKAGE_APACHE
+
+choice
+	prompt "Multi-Processing Module (MPM)"
+	default BR2_PACKAGE_APACHE_MPM_WORKER
+	help
+	  Select the Multi-Processing Module (MPM).
+
+config BR2_PACKAGE_APACHE_MPM_EVENT
+	bool "event"
+	help
+	  A variant of the worker MPM with the goal of consuming threads
+	  only for connections with active processing
+
+config BR2_PACKAGE_APACHE_MPM_PREFORK
+	bool "prefork"
+	help
+	  Implements a non-threaded, pre-forking web server
+
+config BR2_PACKAGE_APACHE_MPM_WORKER
+	bool "worker"
+	help
+	  Implements a hybrid multi-threaded multi-process web server
+
+endchoice
+
+endif
+
 comment "apache needs a toolchain w/ dynamic library, threads"
 	depends on BR2_USE_MMU
 	depends on BR2_STATIC_LIBS || !BR2_TOOLCHAIN_HAS_THREADS
diff --git a/package/apache/apache.mk b/package/apache/apache.mk
index e78545a..488ad56 100644
--- a/package/apache/apache.mk
+++ b/package/apache/apache.mk
@@ -20,6 +20,14 @@ APACHE_CONF_ENV= \
 	ap_cv_void_ptr_lt_long=no \
 	PCRE_CONFIG=$(STAGING_DIR)/usr/bin/pcre-config
 
+ifeq ($(BR2_PACKAGE_APACHE_MPM_EVENT),y)
+APACHE_MPM = event
+else ifeq ($(BR2_PACKAGE_APACHE_MPM_PREFORK),y)
+APACHE_MPM = prefork
+else
+APACHE_MPM = worker
+endif
+
 APACHE_CONF_OPTS = \
 	--sysconfdir=/etc/apache2 \
 	--with-apr=$(STAGING_DIR)/usr \
@@ -31,7 +39,7 @@ APACHE_CONF_OPTS = \
 	--enable-mime-magic \
 	--without-suexec-bin \
 	--enable-mods-shared=all \
-	--with-mpm=worker \
+	--with-mpm=$(APACHE_MPM) \
 	--disable-lua \
 	--disable-luajit
 
-- 
2.5.0

[Buildroot] [PATCH 2/4] php: rework selection of interfaces

[Fabrice Fontaine]

Following suggestion of Yann Morin, rework selection of php interfaces:
use booleans instead of choice to be able to select multiple interfaces
as they are not exclusive

Signed-off-by: Fabrice Fontaine <fabrice.fontaine@orange.com>
---
 package/php/Config.in | 40 +++++-----------------------------------
 package/php/php.mk    |  8 ++++----
 2 files changed, 9 insertions(+), 39 deletions(-)

diff --git a/package/php/Config.in b/package/php/Config.in
index eee8d26..c90ad4f 100644
--- a/package/php/Config.in
+++ b/package/php/Config.in
@@ -1,5 +1,6 @@
 config BR2_PACKAGE_PHP
 	bool "php"
+	select BR2_PACKAGE_PHP_SAPI_CGI if !BR2_PACKAGE_PHP_HAS_SAPI
 	help
 	  PHP  is a widely-used general-purpose scripting
 	  language that is especially suited for Web development
@@ -9,61 +10,30 @@ config BR2_PACKAGE_PHP
 
 if BR2_PACKAGE_PHP
 
-config BR2_PACKAGE_PHP_CLI
+# Helper to make sure at least one interface is selected.
+config BR2_PACKAGE_PHP_HAS_SAPI
 	bool
 
-config BR2_PACKAGE_PHP_CGI
-	bool
-
-config BR2_PACKAGE_PHP_FPM
-	bool
-
-choice
-	prompt "Interface"
-	default BR2_PACKAGE_PHP_SAPI_CGI
-	help
-	  Select the PHP interface(s).
-
 config BR2_PACKAGE_PHP_SAPI_CGI
 	bool "CGI"
 	# CGI uses fork()
 	depends on BR2_USE_MMU
-	select BR2_PACKAGE_PHP_CGI
 	help
 	  Common Gateway Interface
 
 config BR2_PACKAGE_PHP_SAPI_CLI
 	bool "CLI"
-	select BR2_PACKAGE_PHP_CLI
+	select BR2_PACKAGE_PHP_HAS_SAPI
 	help
 	  Command Line Interface
 
 config BR2_PACKAGE_PHP_SAPI_FPM
 	bool "FPM"
 	depends on BR2_USE_MMU
-	select BR2_PACKAGE_PHP_FPM
+	select BR2_PACKAGE_PHP_HAS_SAPI
 	help
 	  PHP-FPM (FastCGI Process Manager)
 
-config BR2_PACKAGE_PHP_SAPI_CLI_CGI
-	bool "CGI and CLI"
-	# CGI uses fork()
-	depends on BR2_USE_MMU
-	select BR2_PACKAGE_PHP_CLI
-	select BR2_PACKAGE_PHP_CGI
-	help
-	  Command line and Common gateway interfaces
-
-config BR2_PACKAGE_PHP_SAPI_CLI_FPM
-	bool "FPM and CLI"
-	depends on BR2_USE_MMU
-	select BR2_PACKAGE_PHP_CLI
-	select BR2_PACKAGE_PHP_FPM
-	help
-	  Command line and PHP-FPM (FastCGI Process Manager)
-
-endchoice
-
 source "package/php/Config.ext"
 
 endif
diff --git a/package/php/php.mk b/package/php/php.mk
index 7df10be..97e3396 100644
--- a/package/php/php.mk
+++ b/package/php/php.mk
@@ -77,9 +77,9 @@ else
 PHP_CONF_ENV += ac_cv_func_dlopen=no ac_cv_lib_dl_dlopen=no
 endif
 
-PHP_CONF_OPTS += $(if $(BR2_PACKAGE_PHP_CLI),,--disable-cli)
-PHP_CONF_OPTS += $(if $(BR2_PACKAGE_PHP_CGI),,--disable-cgi)
-PHP_CONF_OPTS += $(if $(BR2_PACKAGE_PHP_FPM),--enable-fpm,--disable-fpm)
+PHP_CONF_OPTS += $(if $(BR2_PACKAGE_PHP_SAPI_CLI),,--disable-cli)
+PHP_CONF_OPTS += $(if $(BR2_PACKAGE_PHP_SAPI_CGI),,--disable-cgi)
+PHP_CONF_OPTS += $(if $(BR2_PACKAGE_PHP_SAPI_FPM),--enable-fpm,--disable-fpm)
 
 ### Extensions
 PHP_CONF_OPTS += \
@@ -280,7 +280,7 @@ PHP_CONF_OPTS += \
 PHP_DEPENDENCIES += jpeg libpng freetype
 endif
 
-ifeq ($(BR2_PACKAGE_PHP_FPM),y)
+ifeq ($(BR2_PACKAGE_PHP_SAPI_FPM),y)
 define PHP_INSTALL_INIT_SYSV
 	$(INSTALL) -D -m 0755 $(@D)/sapi/fpm/init.d.php-fpm \
 		$(TARGET_DIR)/etc/init.d/S49php-fpm
-- 
2.5.0

[Buildroot] [PATCH 3/4] php: fix pthread detection

[Fabrice Fontaine]

Following suggestion of Yann Morin, move fix on pthread detection in a
detected patch and add more comment on issue: without this fix, thread
safety option (--enable-maintainer-zts) will not work

Signed-off-by: Fabrice Fontaine <fabrice.fontaine@orange.com>
---
 package/php/php.mk | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/package/php/php.mk b/package/php/php.mk
index 97e3396..c4ff249 100644
--- a/package/php/php.mk
+++ b/package/php/php.mk
@@ -47,6 +47,20 @@ define PHP_BUILDCONF
 endef
 PHP_PRE_CONFIGURE_HOOKS += PHP_BUILDCONF
 
+# PHP assumes pthreads are not working when cross-compiling, indeed configure
+# sets pthreads_working to no if cross_compiling is set to yes (line 5888)
+# If pthreads_working is set to no, thread safety option
+# (--enable-maintainer-zts) will not be available
+ifeq ($(BR2_TOOLCHAIN_HAS_THREADS),y)
+PHP_CONF_ENV += ac_cv_pthreads_lib=pthread
+
+define PHP_FIX_THREADS_DETECTION
+	$(SED) 's/pthreads_working\=no/pthreads_working\=yes/' $(@D)/configure
+endef
+
+PHP_PRE_CONFIGURE_HOOKS += PHP_FIX_THREADS_DETECTION
+endif
+
 ifeq ($(BR2_ENDIAN),"BIG")
 PHP_CONF_ENV += ac_cv_c_bigendian_php=yes
 else
-- 
2.5.0

[Buildroot] [PATCH v4,4/4] php: add apache support

[Fabrice Fontaine]

Continue work started by Bernd Kuhls in
https://patchwork.ozlabs.org/patch/437544/

Signed-off-by: Bernd Kuhls <bernd.kuhls@t-online.de>
Signed-off-by: Fabrice Fontaine <fabrice.fontaine@orange.com>
---
Changes v3 -> v4 (after review of Yann Morin):
 - Do not select apache but depends on it
 - Move pthread detection fix in a separate patch
 - Remove unneeded configuration options such as --with-config-file-path
 - Do not allow php to update apache configuration file by removing -a
   argument from apxs call

Changes v2 -> v3 (after review of Thomas Petazzoni):
 - Remove unneeded php-04-apache.patch
 - Fix pthread detection if Apache MPM is event or worker
 - Update pthread detection mechanism (--enable-pthreads does not exist
   anymore)
 - Remove unneeded --oldincludedir
 - Fix php module path

 package/php/Config.in |  7 +++++++
 package/php/php.mk    | 22 ++++++++++++++++++++++
 2 files changed, 29 insertions(+)

diff --git a/package/php/Config.in b/package/php/Config.in
index c90ad4f..e195fd4 100644
--- a/package/php/Config.in
+++ b/package/php/Config.in
@@ -14,6 +14,13 @@ if BR2_PACKAGE_PHP
 config BR2_PACKAGE_PHP_HAS_SAPI
 	bool
 
+config BR2_PACKAGE_PHP_SAPI_APACHE
+	bool "Apache"
+	depends on BR2_PACKAGE_APACHE
+	select BR2_PACKAGE_PHP_HAS_SAPI
+	help
+	  Apache module
+
 config BR2_PACKAGE_PHP_SAPI_CGI
 	bool "CGI"
 	# CGI uses fork()
diff --git a/package/php/php.mk b/package/php/php.mk
index c4ff249..7abd012 100644
--- a/package/php/php.mk
+++ b/package/php/php.mk
@@ -95,6 +95,28 @@ PHP_CONF_OPTS += $(if $(BR2_PACKAGE_PHP_SAPI_CLI),,--disable-cli)
 PHP_CONF_OPTS += $(if $(BR2_PACKAGE_PHP_SAPI_CGI),,--disable-cgi)
 PHP_CONF_OPTS += $(if $(BR2_PACKAGE_PHP_SAPI_FPM),--enable-fpm,--disable-fpm)
 
+ifeq ($(BR2_PACKAGE_PHP_SAPI_APACHE),y)
+PHP_DEPENDENCIES += apache
+PHP_CONF_OPTS += --with-apxs2=$(STAGING_DIR)/usr/bin/apxs
+
+# Enable thread safety option if Apache MPM is event or worker
+ifeq ($(BR2_PACKAGE_APACHE_MPM_EVENT)$(BR2_PACKAGE_APACHE_MPM_WORKER),y)
+PHP_CONF_OPTS += --enable-maintainer-zts
+endif
+
+# php uses apxs from staging directory to install libphp dynamic library and
+# update /etc/apache2/httpd.conf through the -a option, here is the full
+# command line: "apxs -S LIBEXECDIR='$(INSTALL_ROOT)/usr/modules'
+# -S SYSCONFDIR='$(INSTALL_ROOT)/etc/apache2' -i -a -n php7
+# This does not work as apxs sets the full path of the library and not the
+# relative one so remove -a option from apxs call
+define PHP_REMOVE_APXS_UPDATE_CONF
+	$(SED) 's/-i -a -n php/-i -n php/' $(@D)/configure
+endef
+
+PHP_PRE_CONFIGURE_HOOKS += PHP_REMOVE_APXS_UPDATE_CONF
+endif
+
 ### Extensions
 PHP_CONF_OPTS += \
 	$(if $(BR2_PACKAGE_PHP_EXT_SOCKETS),--enable-sockets) \
-- 
2.5.0

[Buildroot] [PATCH v2,1/4] apache: add customization of MPM

[Arnout Vandecappelle]

On 12-09-16 00:26, Fabrice Fontaine wrote:
> MPM can be selected between event, prefork or worker
> Set worker as the default one as it was before even if event MPM is
> better on system supporting thread safe polling
> 
> Signed-off-by: Fabrice Fontaine <fabrice.fontaine@orange.com>

Reviewed-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>

 Perhaps the default should change to 'event', however?

 Regards,
 Arnout

> ---
> Changes v1 -> v2 (after review of Yann Morin):
>  - Do not use unneeded variables in Config.in and apache.mk
> 
>  package/apache/Config.in | 28 ++++++++++++++++++++++++++++
>  package/apache/apache.mk | 10 +++++++++-
>  2 files changed, 37 insertions(+), 1 deletion(-)
> 
> diff --git a/package/apache/Config.in b/package/apache/Config.in
> index 0814a17..511876d 100644
> --- a/package/apache/Config.in
> +++ b/package/apache/Config.in
> @@ -14,6 +14,34 @@ config BR2_PACKAGE_APACHE
>  
>  	  http://httpd.apache.org
>  
> +if BR2_PACKAGE_APACHE
> +
> +choice
> +	prompt "Multi-Processing Module (MPM)"
> +	default BR2_PACKAGE_APACHE_MPM_WORKER
> +	help
> +	  Select the Multi-Processing Module (MPM).
> +
> +config BR2_PACKAGE_APACHE_MPM_EVENT
> +	bool "event"
> +	help
> +	  A variant of the worker MPM with the goal of consuming threads
> +	  only for connections with active processing
> +
> +config BR2_PACKAGE_APACHE_MPM_PREFORK
> +	bool "prefork"
> +	help
> +	  Implements a non-threaded, pre-forking web server
> +
> +config BR2_PACKAGE_APACHE_MPM_WORKER
> +	bool "worker"
> +	help
> +	  Implements a hybrid multi-threaded multi-process web server
> +
> +endchoice
> +
> +endif
> +
>  comment "apache needs a toolchain w/ dynamic library, threads"
>  	depends on BR2_USE_MMU
>  	depends on BR2_STATIC_LIBS || !BR2_TOOLCHAIN_HAS_THREADS
> diff --git a/package/apache/apache.mk b/package/apache/apache.mk
> index e78545a..488ad56 100644
> --- a/package/apache/apache.mk
> +++ b/package/apache/apache.mk
> @@ -20,6 +20,14 @@ APACHE_CONF_ENV= \
>  	ap_cv_void_ptr_lt_long=no \
>  	PCRE_CONFIG=$(STAGING_DIR)/usr/bin/pcre-config
>  
> +ifeq ($(BR2_PACKAGE_APACHE_MPM_EVENT),y)
> +APACHE_MPM = event
> +else ifeq ($(BR2_PACKAGE_APACHE_MPM_PREFORK),y)
> +APACHE_MPM = prefork
> +else
> +APACHE_MPM = worker
> +endif
> +
>  APACHE_CONF_OPTS = \
>  	--sysconfdir=/etc/apache2 \
>  	--with-apr=$(STAGING_DIR)/usr \
> @@ -31,7 +39,7 @@ APACHE_CONF_OPTS = \
>  	--enable-mime-magic \
>  	--without-suexec-bin \
>  	--enable-mods-shared=all \
> -	--with-mpm=worker \
> +	--with-mpm=$(APACHE_MPM) \
>  	--disable-lua \
>  	--disable-luajit
>  
> 

-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

[Buildroot] [PATCH 2/4] php: rework selection of interfaces

[Arnout Vandecappelle]

 Hi Fabrice,

On 12-09-16 00:26, Fabrice Fontaine wrote:
> Following suggestion of Yann Morin, rework selection of php interfaces:
> use booleans instead of choice to be able to select multiple interfaces
> as they are not exclusive

 Maybe add something like:

The choice was introduced in commit fcdc9f891 to make sure at least one
SAPI option was selected. Here, we use an auxiliary symbol
BR2_PACKAGE_PHP_HAS_SAPI and CGI will be selected if this symbol is not set.

> 
> Signed-off-by: Fabrice Fontaine <fabrice.fontaine@orange.com>
> ---
>  package/php/Config.in | 40 +++++-----------------------------------
>  package/php/php.mk    |  8 ++++----
>  2 files changed, 9 insertions(+), 39 deletions(-)
> 
> diff --git a/package/php/Config.in b/package/php/Config.in
> index eee8d26..c90ad4f 100644
> --- a/package/php/Config.in
> +++ b/package/php/Config.in
> @@ -1,5 +1,6 @@
>  config BR2_PACKAGE_PHP
>  	bool "php"
> +	select BR2_PACKAGE_PHP_SAPI_CGI if !BR2_PACKAGE_PHP_HAS_SAPI
>  	help
>  	  PHP  is a widely-used general-purpose scripting
>  	  language that is especially suited for Web development
> @@ -9,61 +10,30 @@ config BR2_PACKAGE_PHP
>  
>  if BR2_PACKAGE_PHP
>  
> -config BR2_PACKAGE_PHP_CLI
> +# Helper to make sure at least one interface is selected.
> +config BR2_PACKAGE_PHP_HAS_SAPI

 The name is not ideal, because it's actually _HAS_SAPI_EXCEPT_CGI. But that's
impossible to understand, so perhaps extend the comment with
# All SAPI options except CGI will select this symbol.

>  	bool
>  
> -config BR2_PACKAGE_PHP_CGI
> -	bool
> -
> -config BR2_PACKAGE_PHP_FPM
> -	bool
> -
> -choice
> -	prompt "Interface"

 You should keep this as a comment, or alternatively change the prompts below to
include "Interface". Now it is just

     [*] php
     -*-   CGI
     [ ]   CLI
     [ ]   FPM
           Extensions  --->

which is not descriptive enough IMHO.

> -	default BR2_PACKAGE_PHP_SAPI_CGI
> -	help
> -	  Select the PHP interface(s).
> -
>  config BR2_PACKAGE_PHP_SAPI_CGI
>  	bool "CGI"
>  	# CGI uses fork()
>  	depends on BR2_USE_MMU
> -	select BR2_PACKAGE_PHP_CGI
>  	help
>  	  Common Gateway Interface
>  
>  config BR2_PACKAGE_PHP_SAPI_CLI
>  	bool "CLI"
> -	select BR2_PACKAGE_PHP_CLI
> +	select BR2_PACKAGE_PHP_HAS_SAPI
>  	help
>  	  Command Line Interface
>  
>  config BR2_PACKAGE_PHP_SAPI_FPM
>  	bool "FPM"
>  	depends on BR2_USE_MMU
> -	select BR2_PACKAGE_PHP_FPM
> +	select BR2_PACKAGE_PHP_HAS_SAPI
>  	help
>  	  PHP-FPM (FastCGI Process Manager)

 Isn't FPM mutually exclusive with CGI?

 If yes, it could be tricky to code that in Config.in without a dependency cycle
due to HAS_SAPI...

>  
> -config BR2_PACKAGE_PHP_SAPI_CLI_CGI
> -	bool "CGI and CLI"
> -	# CGI uses fork()
> -	depends on BR2_USE_MMU
> -	select BR2_PACKAGE_PHP_CLI
> -	select BR2_PACKAGE_PHP_CGI

 Any user-settable config symbol that is removed should move to
Config.in.legacy. Something like this:

config BR2_PACKAGE_PHP_SAPI_CLI_CGI
	bool "PHP CGI and CLI option is split up"
	select BR2_PACKAGE_PHP_SAPI_CLI
	select BR2_PACKAGE_PHP_SAPI_CGI
	help
	  The PHP Interface options have been split up into a
	  separate option for each interface.

> -	help
> -	  Command line and Common gateway interfaces
> -
> -config BR2_PACKAGE_PHP_SAPI_CLI_FPM

 Similar for this one.


 Regards,
 Arnout

> -	bool "FPM and CLI"
> -	depends on BR2_USE_MMU
> -	select BR2_PACKAGE_PHP_CLI
> -	select BR2_PACKAGE_PHP_FPM
> -	help
> -	  Command line and PHP-FPM (FastCGI Process Manager)
> -
> -endchoice
> -
>  source "package/php/Config.ext"
>  
>  endif
> diff --git a/package/php/php.mk b/package/php/php.mk
> index 7df10be..97e3396 100644
> --- a/package/php/php.mk
> +++ b/package/php/php.mk
> @@ -77,9 +77,9 @@ else
>  PHP_CONF_ENV += ac_cv_func_dlopen=no ac_cv_lib_dl_dlopen=no
>  endif
>  
> -PHP_CONF_OPTS += $(if $(BR2_PACKAGE_PHP_CLI),,--disable-cli)
> -PHP_CONF_OPTS += $(if $(BR2_PACKAGE_PHP_CGI),,--disable-cgi)
> -PHP_CONF_OPTS += $(if $(BR2_PACKAGE_PHP_FPM),--enable-fpm,--disable-fpm)
> +PHP_CONF_OPTS += $(if $(BR2_PACKAGE_PHP_SAPI_CLI),,--disable-cli)
> +PHP_CONF_OPTS += $(if $(BR2_PACKAGE_PHP_SAPI_CGI),,--disable-cgi)
> +PHP_CONF_OPTS += $(if $(BR2_PACKAGE_PHP_SAPI_FPM),--enable-fpm,--disable-fpm)
>  
>  ### Extensions
>  PHP_CONF_OPTS += \
> @@ -280,7 +280,7 @@ PHP_CONF_OPTS += \
>  PHP_DEPENDENCIES += jpeg libpng freetype
>  endif
>  
> -ifeq ($(BR2_PACKAGE_PHP_FPM),y)
> +ifeq ($(BR2_PACKAGE_PHP_SAPI_FPM),y)
>  define PHP_INSTALL_INIT_SYSV
>  	$(INSTALL) -D -m 0755 $(@D)/sapi/fpm/init.d.php-fpm \
>  		$(TARGET_DIR)/etc/init.d/S49php-fpm
> 

-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

[Buildroot] [PATCH 3/4] php: fix pthread detection

[Arnout Vandecappelle]

On 12-09-16 00:26, Fabrice Fontaine wrote:
> Following suggestion of Yann Morin, move fix on pthread detection in a
> detected patch and add more comment on issue: without this fix, thread
> safety option (--enable-maintainer-zts) will not work

 This commit message makes no sense. I guess you wanted to say that in this v2,
you split off the pthread detection into a separate patch. But that should be in
a non-commit comment (see below).

 Better use the comment you added below as the commit message.

> 
> Signed-off-by: Fabrice Fontaine <fabrice.fontaine@orange.com>

 You can add below your Sob

---
v2: move to a separate patch (Yann Morin)

> ---
>  package/php/php.mk | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/package/php/php.mk b/package/php/php.mk
> index 97e3396..c4ff249 100644
> --- a/package/php/php.mk
> +++ b/package/php/php.mk
> @@ -47,6 +47,20 @@ define PHP_BUILDCONF
>  endef
>  PHP_PRE_CONFIGURE_HOOKS += PHP_BUILDCONF
>  
> +# PHP assumes pthreads are not working when cross-compiling, indeed configure
> +# sets pthreads_working to no if cross_compiling is set to yes (line 5888)
> +# If pthreads_working is set to no, thread safety option
> +# (--enable-maintainer-zts) will not be available
> +ifeq ($(BR2_TOOLCHAIN_HAS_THREADS),y)
> +PHP_CONF_ENV += ac_cv_pthreads_lib=pthread
> +
> +define PHP_FIX_THREADS_DETECTION
> +	$(SED) 's/pthreads_working\=no/pthreads_working\=yes/' $(@D)/configure
> +endef

 I'd prefer an upstreamable patch that fixes the issue. The problem is that the
configure script violates a rule of AC_CACHE_VAL: "The commands-to-set-it must
have no side effects except for setting the variable cache-id, see below". But
they also set pthreads_working, which is wrong. It completely defeats the
purpose of the cache.

 So what should be done is to remove the pthreads_working assignments from the
AC_CACHE_CHECK functions, and replace them with:

if [ "x$ac_cv_pthreads_cflags" != "x" -o "x$ac_cv_pthreads_lib" != "x" ]; then
	pthreads_working=yes
fi


 Regards,
 Arnout


> +
> +PHP_PRE_CONFIGURE_HOOKS += PHP_FIX_THREADS_DETECTION
> +endif
> +
>  ifeq ($(BR2_ENDIAN),"BIG")
>  PHP_CONF_ENV += ac_cv_c_bigendian_php=yes
>  else
> 

-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

[Buildroot] [PATCH v4,4/4] php: add apache support

[Arnout Vandecappelle]

On 12-09-16 00:26, Fabrice Fontaine wrote:
> Continue work started by Bernd Kuhls in
> https://patchwork.ozlabs.org/patch/437544/
> 
> Signed-off-by: Bernd Kuhls <bernd.kuhls@t-online.de>
> Signed-off-by: Fabrice Fontaine <fabrice.fontaine@orange.com>
> ---
> Changes v3 -> v4 (after review of Yann Morin):
>  - Do not select apache but depends on it
>  - Move pthread detection fix in a separate patch
>  - Remove unneeded configuration options such as --with-config-file-path
>  - Do not allow php to update apache configuration file by removing -a
>    argument from apxs call
> 
> Changes v2 -> v3 (after review of Thomas Petazzoni):
>  - Remove unneeded php-04-apache.patch
>  - Fix pthread detection if Apache MPM is event or worker
>  - Update pthread detection mechanism (--enable-pthreads does not exist
>    anymore)
>  - Remove unneeded --oldincludedir
>  - Fix php module path
> 
>  package/php/Config.in |  7 +++++++
>  package/php/php.mk    | 22 ++++++++++++++++++++++
>  2 files changed, 29 insertions(+)
> 
> diff --git a/package/php/Config.in b/package/php/Config.in
> index c90ad4f..e195fd4 100644
> --- a/package/php/Config.in
> +++ b/package/php/Config.in
> @@ -14,6 +14,13 @@ if BR2_PACKAGE_PHP
>  config BR2_PACKAGE_PHP_HAS_SAPI
>  	bool
>  
> +config BR2_PACKAGE_PHP_SAPI_APACHE
> +	bool "Apache"
> +	depends on BR2_PACKAGE_APACHE
> +	select BR2_PACKAGE_PHP_HAS_SAPI
> +	help
> +	  Apache module
> +
>  config BR2_PACKAGE_PHP_SAPI_CGI
>  	bool "CGI"
>  	# CGI uses fork()
> diff --git a/package/php/php.mk b/package/php/php.mk
> index c4ff249..7abd012 100644
> --- a/package/php/php.mk
> +++ b/package/php/php.mk
> @@ -95,6 +95,28 @@ PHP_CONF_OPTS += $(if $(BR2_PACKAGE_PHP_SAPI_CLI),,--disable-cli)
>  PHP_CONF_OPTS += $(if $(BR2_PACKAGE_PHP_SAPI_CGI),,--disable-cgi)
>  PHP_CONF_OPTS += $(if $(BR2_PACKAGE_PHP_SAPI_FPM),--enable-fpm,--disable-fpm)
>  
> +ifeq ($(BR2_PACKAGE_PHP_SAPI_APACHE),y)
> +PHP_DEPENDENCIES += apache
> +PHP_CONF_OPTS += --with-apxs2=$(STAGING_DIR)/usr/bin/apxs
> +
> +# Enable thread safety option if Apache MPM is event or worker
> +ifeq ($(BR2_PACKAGE_APACHE_MPM_EVENT)$(BR2_PACKAGE_APACHE_MPM_WORKER),y)
> +PHP_CONF_OPTS += --enable-maintainer-zts
> +endif
> +
> +# php uses apxs from staging directory to install libphp dynamic library and
> +# update /etc/apache2/httpd.conf through the -a option, here is the full
> +# command line: "apxs -S LIBEXECDIR='$(INSTALL_ROOT)/usr/modules'
> +# -S SYSCONFDIR='$(INSTALL_ROOT)/etc/apache2' -i -a -n php7
> +# This does not work as apxs sets the full path of the library and not the
> +# relative one so remove -a option from apxs call

 Isn't the proper solution to add apxs to APACHE_CONFIG_SCRIPTS? Or is the
staging dir encoded in a way that our sed script doesn't find it?

 Regards,
 Arnout

> +define PHP_REMOVE_APXS_UPDATE_CONF
> +	$(SED) 's/-i -a -n php/-i -n php/' $(@D)/configure
> +endef
> +
> +PHP_PRE_CONFIGURE_HOOKS += PHP_REMOVE_APXS_UPDATE_CONF
> +endif
> +
>  ### Extensions
>  PHP_CONF_OPTS += \
>  	$(if $(BR2_PACKAGE_PHP_EXT_SOCKETS),--enable-sockets) \
> 

-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

[Buildroot] [PATCH v2,1/4] apache: add customization of MPM

[Thomas Petazzoni]

Hello,

On Mon, 12 Sep 2016 00:26:05 +0200, Fabrice Fontaine wrote:

> +ifeq ($(BR2_PACKAGE_APACHE_MPM_EVENT),y)
> +APACHE_MPM = event
> +else ifeq ($(BR2_PACKAGE_APACHE_MPM_PREFORK),y)
> +APACHE_MPM = prefork
> +else

I've changed this to:

else ifeq ($(BR2_PACKAGE_APACHE_MPM_WORKER),y)

just for the sake of consistency. And then I've applied your patch.
Thanks!

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[Buildroot] [PATCH v4,4/4] php: add apache support

[Fabrice Fontaine]

Dear Arnout,

You're right that the issue is linked to apxs however adding apxs to
APACHE_CONFIG_SCRIPTS does not solve the issue.

I'll try to explain the problem. At the moment, the configuration file of
apxs, config_vars.mk, is patched in APACHE_FIX_STAGING_APACHE_CONFIG to add
STAGING_DIR as a prefix so apxs adds the correct LoadModule directive in
output/staging/etc/apache2/httpd.conf through the following command:

output/staging/usr/bin/apxs -S
LIBEXECDIR='/home/fabrice/buildroot-qemu/output/host/usr/arm-buildroot-linux-uclibcgnueabi/sysroot/usr/modules'
-S
SYSCONFDIR='/home/fabrice/buildroot-qemu/output/host/usr/arm-buildroot-linux-uclibcgnueabi/sysroot/etc/apache2'
-i -a -n php7 output/build/php-7.0.9/libs/libphp7.so

However, the same apxs in staging directory is also used to update the
output/target/etc/apache2/httpd.conf through the following command:
output/staging/usr/bin/apxs -S
LIBEXECDIR='/home/fabrice/buildroot-qemu/output/staging/usr/modules' -S
SYSCONFDIR='/home/fabrice/buildroot-qemu/output/staging/etc/apache2' -i -a
-n php7 output/build/php-7.0.9/libs/libphp7.so

In this case, the prefix is wrong and apxs adds a wrong path in the
LoadModule directive.
We could fix the issue by updating APACHE_FIX_STAGING_APACHE_CONFIG to use
the TARGET_DIR instead of the STAGING_DIR. However, Yann's point of view
was that php package should not update apache configuration file. So, I
will let you decide what should be done.

Best Regards,

 he binary is correctly patched to install

2016-09-12 1:30 GMT+02:00 Arnout Vandecappelle <arnout@mind.be>:

>
>
> On 12-09-16 00:26, Fabrice Fontaine wrote:
> > Continue work started by Bernd Kuhls in
> > https://patchwork.ozlabs.org/patch/437544/
> >
> > Signed-off-by: Bernd Kuhls <bernd.kuhls@t-online.de>
> > Signed-off-by: Fabrice Fontaine <fabrice.fontaine@orange.com>
> > ---
> > Changes v3 -> v4 (after review of Yann Morin):
> >  - Do not select apache but depends on it
> >  - Move pthread detection fix in a separate patch
> >  - Remove unneeded configuration options such as --with-config-file-path
> >  - Do not allow php to update apache configuration file by removing -a
> >    argument from apxs call
> >
> > Changes v2 -> v3 (after review of Thomas Petazzoni):
> >  - Remove unneeded php-04-apache.patch
> >  - Fix pthread detection if Apache MPM is event or worker
> >  - Update pthread detection mechanism (--enable-pthreads does not exist
> >    anymore)
> >  - Remove unneeded --oldincludedir
> >  - Fix php module path
> >
> >  package/php/Config.in |  7 +++++++
> >  package/php/php.mk    | 22 ++++++++++++++++++++++
> >  2 files changed, 29 insertions(+)
> >
> > diff --git a/package/php/Config.in b/package/php/Config.in
> > index c90ad4f..e195fd4 100644
> > --- a/package/php/Config.in
> > +++ b/package/php/Config.in
> > @@ -14,6 +14,13 @@ if BR2_PACKAGE_PHP
> >  config BR2_PACKAGE_PHP_HAS_SAPI
> >       bool
> >
> > +config BR2_PACKAGE_PHP_SAPI_APACHE
> > +     bool "Apache"
> > +     depends on BR2_PACKAGE_APACHE
> > +     select BR2_PACKAGE_PHP_HAS_SAPI
> > +     help
> > +       Apache module
> > +
> >  config BR2_PACKAGE_PHP_SAPI_CGI
> >       bool "CGI"
> >       # CGI uses fork()
> > diff --git a/package/php/php.mk b/package/php/php.mk
> > index c4ff249..7abd012 100644
> > --- a/package/php/php.mk
> > +++ b/package/php/php.mk
> > @@ -95,6 +95,28 @@ PHP_CONF_OPTS += $(if $(BR2_PACKAGE_PHP_SAPI_CLI),,-
> -disable-cli)
> >  PHP_CONF_OPTS += $(if $(BR2_PACKAGE_PHP_SAPI_CGI),,--disable-cgi)
> >  PHP_CONF_OPTS += $(if $(BR2_PACKAGE_PHP_SAPI_FPM),--
> enable-fpm,--disable-fpm)
> >
> > +ifeq ($(BR2_PACKAGE_PHP_SAPI_APACHE),y)
> > +PHP_DEPENDENCIES += apache
> > +PHP_CONF_OPTS += --with-apxs2=$(STAGING_DIR)/usr/bin/apxs
> > +
> > +# Enable thread safety option if Apache MPM is event or worker
> > +ifeq ($(BR2_PACKAGE_APACHE_MPM_EVENT)$(BR2_PACKAGE_APACHE_
> MPM_WORKER),y)
> > +PHP_CONF_OPTS += --enable-maintainer-zts
> > +endif
> > +
> > +# php uses apxs from staging directory to install libphp dynamic
> library and
> > +# update /etc/apache2/httpd.conf through the -a option, here is the full
> > +# command line: "apxs -S LIBEXECDIR='$(INSTALL_ROOT)/usr/modules'
> > +# -S SYSCONFDIR='$(INSTALL_ROOT)/etc/apache2' -i -a -n php7
> > +# This does not work as apxs sets the full path of the library and not
> the
> > +# relative one so remove -a option from apxs call
>
>  Isn't the proper solution to add apxs to APACHE_CONFIG_SCRIPTS? Or is the
> staging dir encoded in a way that our sed script doesn't find it?
>
>  Regards,
>  Arnout
>
> > +define PHP_REMOVE_APXS_UPDATE_CONF
> > +     $(SED) 's/-i -a -n php/-i -n php/' $(@D)/configure
> > +endef
> > +
> > +PHP_PRE_CONFIGURE_HOOKS += PHP_REMOVE_APXS_UPDATE_CONF
> > +endif
> > +
> >  ### Extensions
> >  PHP_CONF_OPTS += \
> >       $(if $(BR2_PACKAGE_PHP_EXT_SOCKETS),--enable-sockets) \
> >
>
> --
> Arnout Vandecappelle                          arnout at mind be
> Senior Embedded Software Architect            +32-16-286500
> Essensium/Mind                                http://www.mind.be
> G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
> LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
> GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20160913/8128dead/attachment-0001.html>

[Buildroot] [PATCH v4,4/4] php: add apache support

[Arnout Vandecappelle]

[Please don't top-post, but answer inline.]

On 13-09-16 23:13, Fabrice Fontaine wrote:
> Dear Arnout,
> 
> You're right that the issue is linked to apxs however adding apxs to
> APACHE_CONFIG_SCRIPTS does not solve the issue.
> 
> I'll try to explain the problem. At the moment, the configuration file of apxs,
> config_vars.mk <http://config_vars.mk>, is patched in
> APACHE_FIX_STAGING_APACHE_CONFIG to add STAGING_DIR as a prefix so apxs adds the
> correct LoadModule directive in output/staging/etc/apache2/httpd.conf through
> the following command:
> 
> output/staging/usr/bin/apxs -S
> LIBEXECDIR='/home/fabrice/buildroot-qemu/output/host/usr/arm-buildroot-linux-uclibcgnueabi/sysroot/usr/modules'
> -S
> SYSCONFDIR='/home/fabrice/buildroot-qemu/output/host/usr/arm-buildroot-linux-uclibcgnueabi/sysroot/etc/apache2'
> -i -a -n php7 output/build/php-7.0.9/libs/libphp7.so
> 
> However, the same apxs in staging directory is also used to update the
> output/target/etc/apache2/httpd.conf through the following command:
> output/staging/usr/bin/apxs -S
> LIBEXECDIR='/home/fabrice/buildroot-qemu/output/staging/usr/modules' -S
> SYSCONFDIR='/home/fabrice/buildroot-qemu/output/staging/etc/apache2' -i -a -n
> php7 output/build/php-7.0.9/libs/libphp7.so

 I guess here you meant target instead of staging...

> 
> In this case, the prefix is wrong and apxs adds a wrong path in the LoadModule
> directive.

 So that's the crux of the matter. apxs is smart enough to subsitute away prefix:

        my $dir = $CFG_LIBEXECDIR;
        $dir =~ s|^$CFG_PREFIX/?||;

However, for the target install, LIBEXECDIR is overridden to the target
directory, but PREFIX remains as staging directory.

 I've done a quick test, and passing an additional -S PREFIX $(TARGET_DIR)/usr
for the target install seems to fix it. Now, is there a way to push this option
down to the target install (only)?


> We could fix the issue by updating APACHE_FIX_STAGING_APACHE_CONFIG to use the
> TARGET_DIR instead of the STAGING_DIR. However, Yann's point of view was that
> php package should not update apache configuration file. So, I will let you
> decide what should be done.

 IMHO, Yann was wrong in that. It's very normal that package a that depends on
package b will update some file that was originally generated by package b.

 That said, it is very convenient to use an approach like e.g. Debian, where
each module has its own conf file, and can be enabled by linking to the conf
file from /etc/apache2/mods-enabled. But that means we have to make a httpd.conf
which is very different from upstream's.

 Regards,
 Arnout

[snip]
-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF