Buildroot Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [Buildroot] [PATCH 1/2] apache: add customization of MPM
@ 2016-09-10 19:17 Fabrice Fontaine
  2016-09-10 19:17 ` [Buildroot] [PATCH v3,2/2] php: add apache support Fabrice Fontaine
  2016-09-10 21:30 ` [Buildroot] [PATCH 1/2] apache: add customization of MPM Yann E. MORIN
  0 siblings, 2 replies; 6+ messages in thread
From: Fabrice Fontaine @ 2016-09-10 19:17 UTC (permalink / raw)
  To: buildroot

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>
---
 package/apache/Config.in | 40 ++++++++++++++++++++++++++++++++++++++++
 package/apache/apache.mk | 14 +++++++++++++-
 2 files changed, 53 insertions(+), 1 deletion(-)

diff --git a/package/apache/Config.in b/package/apache/Config.in
index 0814a17..3b25988 100644
--- a/package/apache/Config.in
+++ b/package/apache/Config.in
@@ -14,6 +14,46 @@ config BR2_PACKAGE_APACHE
 
 	  http://httpd.apache.org
 
+if BR2_PACKAGE_APACHE
+
+config BR2_PACKAGE_APACHE_EVENT
+	bool
+
+config BR2_PACKAGE_APACHE_PREFORK
+	bool
+
+config BR2_PACKAGE_APACHE_WORKER
+	bool
+
+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"
+	select BR2_PACKAGE_APACHE_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"
+	select BR2_PACKAGE_APACHE_PREFORK
+	help
+	  Implements a non-threaded, pre-forking web server
+
+config BR2_PACKAGE_APACHE_MPM_WORKER
+	bool "worker"
+	select BR2_PACKAGE_APACHE_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..a3f49a1 100644
--- a/package/apache/apache.mk
+++ b/package/apache/apache.mk
@@ -20,6 +20,18 @@ APACHE_CONF_ENV= \
 	ap_cv_void_ptr_lt_long=no \
 	PCRE_CONFIG=$(STAGING_DIR)/usr/bin/pcre-config
 
+ifeq ($(BR2_PACKAGE_APACHE_EVENT),y)
+APACHE_MPM = "event"
+endif
+
+ifeq ($(BR2_PACKAGE_APACHE_PREFORK),y)
+APACHE_MPM = "prefork"
+endif
+
+ifeq ($(BR2_PACKAGE_APACHE_WORKER),y)
+APACHE_MPM = "worker"
+endif
+
 APACHE_CONF_OPTS = \
 	--sysconfdir=/etc/apache2 \
 	--with-apr=$(STAGING_DIR)/usr \
@@ -31,7 +43,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

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [Buildroot] [PATCH v3,2/2] php: add apache support
  2016-09-10 19:17 [Buildroot] [PATCH 1/2] apache: add customization of MPM Fabrice Fontaine
@ 2016-09-10 19:17 ` Fabrice Fontaine
  2016-09-10 22:08   ` Yann E. MORIN
  2016-09-10 21:30 ` [Buildroot] [PATCH 1/2] apache: add customization of MPM Yann E. MORIN
  1 sibling, 1 reply; 6+ messages in thread
From: Fabrice Fontaine @ 2016-09-10 19:17 UTC (permalink / raw)
  To: buildroot

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 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 | 17 +++++++++++++++++
 package/php/php.mk    | 31 +++++++++++++++++++++++++++++++
 2 files changed, 48 insertions(+)

diff --git a/package/php/Config.in b/package/php/Config.in
index eee8d26..5f5a976 100644
--- a/package/php/Config.in
+++ b/package/php/Config.in
@@ -9,6 +9,9 @@ config BR2_PACKAGE_PHP
 
 if BR2_PACKAGE_PHP
 
+config BR2_PACKAGE_PHP_APACHE
+	bool
+
 config BR2_PACKAGE_PHP_CLI
 	bool
 
@@ -24,6 +27,20 @@ choice
 	help
 	  Select the PHP interface(s).
 
+config BR2_PACKAGE_PHP_SAPI_APACHE
+	bool "Apache"
+	select BR2_PACKAGE_APACHE
+	select BR2_PACKAGE_PHP_APACHE
+	depends on !BR2_STATIC_LIBS # apache
+	depends on BR2_TOOLCHAIN_HAS_THREADS # apache
+	depends on BR2_USE_MMU # apr
+	help
+	  Apache module
+
+comment "apache module needs a toolchain w/ dynamic library, threads"
+	depends on BR2_USE_MMU
+	depends on BR2_STATIC_LIBS || !BR2_TOOLCHAIN_HAS_THREADS
+
 config BR2_PACKAGE_PHP_SAPI_CGI
 	bool "CGI"
 	# CGI uses fork()
diff --git a/package/php/php.mk b/package/php/php.mk
index 7df10be..083cd58 100644
--- a/package/php/php.mk
+++ b/package/php/php.mk
@@ -81,6 +81,37 @@ 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)
 
+ifeq ($(BR2_PACKAGE_PHP_APACHE),y)
+PHP_DEPENDENCIES += apache
+
+PHP_CONF_OPTS += \
+	--with-apxs2=$(STAGING_DIR)/usr/bin/apxs \
+	--with-config-file-path=/etc/apache2 \
+	--with-config-file-scan-dir=/etc/apache2/php \
+	--localstatedir=/var
+
+# PHP assumes pthreads are not working when cross-compiling,
+# needed for Apache-MPM event or worker
+ifeq ($(BR2_PACKAGE_APACHE_EVENT)$(BR2_PACKAGE_APACHE_WORKER),y)
+PHP_CONF_ENV += ac_cv_pthreads_lib=pthread
+PHP_CONF_OPTS += --enable-maintainer-zts
+
+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
+
+# PHP sets a full path with $(TARGET_DIR) for libphp module instead of a
+# relative path
+define PHP_FIX_MODULE_PATH
+	$(SED) 's:$(TARGET_DIR)/usr/::' $(TARGET_DIR)/etc/apache2/httpd.conf
+endef
+
+PHP_POST_INSTALL_TARGET_HOOKS += PHP_FIX_MODULE_PATH
+endif
+
 ### Extensions
 PHP_CONF_OPTS += \
 	$(if $(BR2_PACKAGE_PHP_EXT_SOCKETS),--enable-sockets) \
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [Buildroot] [PATCH 1/2] apache: add customization of MPM
  2016-09-10 19:17 [Buildroot] [PATCH 1/2] apache: add customization of MPM Fabrice Fontaine
  2016-09-10 19:17 ` [Buildroot] [PATCH v3,2/2] php: add apache support Fabrice Fontaine
@ 2016-09-10 21:30 ` Yann E. MORIN
  2016-09-11 11:08   ` Arnout Vandecappelle
  1 sibling, 1 reply; 6+ messages in thread
From: Yann E. MORIN @ 2016-09-10 21:30 UTC (permalink / raw)
  To: buildroot

Fabrice, All,

On 2016-09-10 21:17 +0200, Fabrice Fontaine spake thusly:
> 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>
> ---
>  package/apache/Config.in | 40 ++++++++++++++++++++++++++++++++++++++++
>  package/apache/apache.mk | 14 +++++++++++++-
>  2 files changed, 53 insertions(+), 1 deletion(-)
> 
> diff --git a/package/apache/Config.in b/package/apache/Config.in
> index 0814a17..3b25988 100644
> --- a/package/apache/Config.in
> +++ b/package/apache/Config.in
> @@ -14,6 +14,46 @@ config BR2_PACKAGE_APACHE
>  
>  	  http://httpd.apache.org
>  
> +if BR2_PACKAGE_APACHE
> +
> +config BR2_PACKAGE_APACHE_EVENT
> +	bool
> +
> +config BR2_PACKAGE_APACHE_PREFORK
> +	bool
> +
> +config BR2_PACKAGE_APACHE_WORKER
> +	bool

Why do you have those symbols?
See below...

> +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"
> +	select BR2_PACKAGE_APACHE_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"
> +	select BR2_PACKAGE_APACHE_PREFORK
> +	help
> +	  Implements a non-threaded, pre-forking web server
> +
> +config BR2_PACKAGE_APACHE_MPM_WORKER
> +	bool "worker"
> +	select BR2_PACKAGE_APACHE_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..a3f49a1 100644
> --- a/package/apache/apache.mk
> +++ b/package/apache/apache.mk
> @@ -20,6 +20,18 @@ APACHE_CONF_ENV= \
>  	ap_cv_void_ptr_lt_long=no \
>  	PCRE_CONFIG=$(STAGING_DIR)/usr/bin/pcre-config
>  
> +ifeq ($(BR2_PACKAGE_APACHE_EVENT),y)
> +APACHE_MPM = "event"
> +endif
> +
> +ifeq ($(BR2_PACKAGE_APACHE_PREFORK),y)
> +APACHE_MPM = "prefork"
> +endif
> +
> +ifeq ($(BR2_PACKAGE_APACHE_WORKER),y)
> +APACHE_MPM = "worker"
> +endif

You can re-use the symbols from the choice here:

        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

(No need to test the last case, since the symbols are from a choice: one
and only one can be enabled at a time.

Also, no need to quote the value.

Regards,
Yann E. MORIN.

PS. See ya on Monday! ;-)

>  APACHE_CONF_OPTS = \
>  	--sysconfdir=/etc/apache2 \
>  	--with-apr=$(STAGING_DIR)/usr \
> @@ -31,7 +43,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 mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

^ permalink raw reply	[flat|nested] 6+ messages in thread
* [Buildroot] [PATCH v3,2/2] php: add apache support
  2016-09-10 19:17 ` [Buildroot] [PATCH v3,2/2] php: add apache support Fabrice Fontaine
@ 2016-09-10 22:08   ` Yann E. MORIN
  2016-09-10 22:32     ` Yann E. MORIN
  0 siblings, 1 reply; 6+ messages in thread
From: Yann E. MORIN @ 2016-09-10 22:08 UTC (permalink / raw)
  To: buildroot

Fabrice, All,

On 2016-09-10 21:17 +0200, Fabrice Fontaine spake thusly:
> 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 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 | 17 +++++++++++++++++
>  package/php/php.mk    | 31 +++++++++++++++++++++++++++++++
>  2 files changed, 48 insertions(+)
> 
> diff --git a/package/php/Config.in b/package/php/Config.in
> index eee8d26..5f5a976 100644
> --- a/package/php/Config.in
> +++ b/package/php/Config.in
> @@ -9,6 +9,9 @@ config BR2_PACKAGE_PHP
>  
>  if BR2_PACKAGE_PHP
>  
> +config BR2_PACKAGE_PHP_APACHE
> +	bool
> +
>  config BR2_PACKAGE_PHP_CLI
>  	bool
>  
> @@ -24,6 +27,20 @@ choice
>  	help
>  	  Select the PHP interface(s).
>  
> +config BR2_PACKAGE_PHP_SAPI_APACHE
> +	bool "Apache"

So the apache "interface" is exclusive to all others?

In the end, I seem to recall I was the one suggesting we do a choice
here (see commit fcdc9f8). In retrospect, that was a very bad
suggestion.

Instead, we should remove that choice, turn back in possible interfaces
to individual booleans, and just ensure that at least one is enabled,
lilke we do in other packages (e.g. weston):

    config BR2_PACKAGE_PHP
        bool "php"
        slect BR2_PACKAGE_PHP_SAPI_CGI if !BR2_PACKAGE_PHP_HAS_SAPI

    config BR2_PACKAGE_PHP_HAS_SAPI
        bool

    config BR2_PACKAGE_PHP_SAPI_CGI
        bool "CGI
        # No select of BR2_PACKAGE_PHP_HAS_SAPI, or circular dependency
        # would ensue. Don't add this comment in actual code! ;-)

    config BR2_PACKAGE_PHP_SAPI_CLI
        bool "CLI"
        select BR2_PACKAGE_PHP_HAS_SAPI

    config BR2_PACKAGE_PHP_SAPI_FPM
        bool "FPM"
        select BR2_PACKAGE_PHP_HAS_SAPI

    config BR2_PACKAGE_PHP_SAPI_APACHE
        bool "Apache"
        select BR2_PACKAGE_PHP_HAS_SAPI

Of course, if some interfaces are incompatible one with the others, then
add proper dependencies (but be carefull of circular deps! ;-) ).

Oh, and splitting the choice should be a separate patch, to come before
this one, please.

> +	select BR2_PACKAGE_APACHE
> +	select BR2_PACKAGE_PHP_APACHE
> +	depends on !BR2_STATIC_LIBS # apache
> +	depends on BR2_TOOLCHAIN_HAS_THREADS # apache
> +	depends on BR2_USE_MMU # apr

"depends on" go before "select", please. Also, I'd prefer the
architecture dependencies come first (i.e. MMU before threads and
static).

> +	help
> +	  Apache module
> +
> +comment "apache module needs a toolchain w/ dynamic library, threads"
> +	depends on BR2_USE_MMU
> +	depends on BR2_STATIC_LIBS || !BR2_TOOLCHAIN_HAS_THREADS
> +
>  config BR2_PACKAGE_PHP_SAPI_CGI
>  	bool "CGI"
>  	# CGI uses fork()
> diff --git a/package/php/php.mk b/package/php/php.mk
> index 7df10be..083cd58 100644
> --- a/package/php/php.mk
> +++ b/package/php/php.mk
> @@ -81,6 +81,37 @@ 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)
>  
> +ifeq ($(BR2_PACKAGE_PHP_APACHE),y)
> +PHP_DEPENDENCIES += apache
> +
> +PHP_CONF_OPTS += \
> +	--with-apxs2=$(STAGING_DIR)/usr/bin/apxs \
> +	--with-config-file-path=/etc/apache2 \
> +	--with-config-file-scan-dir=/etc/apache2/php \
> +	--localstatedir=/var
> +
> +# PHP assumes pthreads are not working when cross-compiling,
> +# needed for Apache-MPM event or worker
> +ifeq ($(BR2_PACKAGE_APACHE_EVENT)$(BR2_PACKAGE_APACHE_WORKER),y)
> +PHP_CONF_ENV += ac_cv_pthreads_lib=pthread
> +PHP_CONF_OPTS += --enable-maintainer-zts
> +
> +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

Why do you need to tweak the configure file? Provide more explanations,
please (in the commit log and/or by expanding the comment above).

Also, it probably does not have to be conditional to the Apache SAPI to
be enabled, does it? If so, then this tweak (if at all needed) should
not depend on the Apache SAPI, but really on BR2_TOOLCHAIN_HAS_THREADS.

In any case, this thread "fix" should have been in a separate patch, to
come before the Apache SAPI.

> +# PHP sets a full path with $(TARGET_DIR) for libphp module instead of a
> +# relative path
> +define PHP_FIX_MODULE_PATH
> +	$(SED) 's:$(TARGET_DIR)/usr/::' $(TARGET_DIR)/etc/apache2/httpd.conf
> +endef

That the php package changes the content of a file installed by the
apache package is not good, no-no... :-/

Care to explain a bit more in details what happens here?

Is it because we pass --with-config-file-path=/etc/apache2 ?

If so, I think it would be better not to do so, and only install a
config fragment that has to be sourced by the main apache config file, a
bit like is done in distros.

Regards,
Yann E. MORIN.

> +PHP_POST_INSTALL_TARGET_HOOKS += PHP_FIX_MODULE_PATH
> +endif
> +
>  ### Extensions
>  PHP_CONF_OPTS += \
>  	$(if $(BR2_PACKAGE_PHP_EXT_SOCKETS),--enable-sockets) \
> -- 
> 2.5.0
> 
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

^ permalink raw reply	[flat|nested] 6+ messages in thread
* [Buildroot] [PATCH v3,2/2] php: add apache support
  2016-09-10 22:08   ` Yann E. MORIN
@ 2016-09-10 22:32     ` Yann E. MORIN
  0 siblings, 0 replies; 6+ messages in thread
From: Yann E. MORIN @ 2016-09-10 22:32 UTC (permalink / raw)
  To: buildroot

Fabrice, All,

On 2016-09-11 00:08 +0200, Yann E. MORIN spake thusly:
> On 2016-09-10 21:17 +0200, Fabrice Fontaine spake thusly:
> > 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 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 | 17 +++++++++++++++++
> >  package/php/php.mk    | 31 +++++++++++++++++++++++++++++++
> >  2 files changed, 48 insertions(+)
> > 
> > diff --git a/package/php/Config.in b/package/php/Config.in
> > index eee8d26..5f5a976 100644
> > --- a/package/php/Config.in
> > +++ b/package/php/Config.in
> > @@ -9,6 +9,9 @@ config BR2_PACKAGE_PHP
> >  
> >  if BR2_PACKAGE_PHP
> >  
> > +config BR2_PACKAGE_PHP_APACHE
> > +	bool
> > +
> >  config BR2_PACKAGE_PHP_CLI
> >  	bool
> >  
> > @@ -24,6 +27,20 @@ choice
> >  	help
> >  	  Select the PHP interface(s).
> >  
> > +config BR2_PACKAGE_PHP_SAPI_APACHE
> > +	bool "Apache"
> > +	select BR2_PACKAGE_APACHE
> > +	select BR2_PACKAGE_PHP_APACHE
> > +	depends on !BR2_STATIC_LIBS # apache
> > +	depends on BR2_TOOLCHAIN_HAS_THREADS # apache
> > +	depends on BR2_USE_MMU # apr
> 
> "depends on" go before "select", please. Also, I'd prefer the
> architecture dependencies come first (i.e. MMU before threads and
> static).

Furthermore, I think this should not select Apache, but depend on it.

Php is "just" a module for Apache, so I find it a bit excessive that php
would select Apache.

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

^ permalink raw reply	[flat|nested] 6+ messages in thread
* [Buildroot] [PATCH 1/2] apache: add customization of MPM
  2016-09-10 21:30 ` [Buildroot] [PATCH 1/2] apache: add customization of MPM Yann E. MORIN
@ 2016-09-11 11:08   ` Arnout Vandecappelle
  0 siblings, 0 replies; 6+ messages in thread
From: Arnout Vandecappelle @ 2016-09-11 11:08 UTC (permalink / raw)
  To: buildroot



On 10-09-16 23:30, Yann E. MORIN wrote:
> Fabrice, All,
> 
> On 2016-09-10 21:17 +0200, Fabrice Fontaine spake thusly:
>> 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>
>> ---
>>  package/apache/Config.in | 40 ++++++++++++++++++++++++++++++++++++++++
>>  package/apache/apache.mk | 14 +++++++++++++-
>>  2 files changed, 53 insertions(+), 1 deletion(-)
>>
>> diff --git a/package/apache/Config.in b/package/apache/Config.in
>> index 0814a17..3b25988 100644
>> --- a/package/apache/Config.in
>> +++ b/package/apache/Config.in
>> @@ -14,6 +14,46 @@ config BR2_PACKAGE_APACHE
>>  
>>  	  http://httpd.apache.org
>>  
>> +if BR2_PACKAGE_APACHE
>> +
>> +config BR2_PACKAGE_APACHE_EVENT
>> +	bool
>> +
>> +config BR2_PACKAGE_APACHE_PREFORK
>> +	bool
>> +
>> +config BR2_PACKAGE_APACHE_WORKER
>> +	bool
> 
> Why do you have those symbols?
> See below...
> 
>> +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"
>> +	select BR2_PACKAGE_APACHE_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"
>> +	select BR2_PACKAGE_APACHE_PREFORK
>> +	help
>> +	  Implements a non-threaded, pre-forking web server
>> +
>> +config BR2_PACKAGE_APACHE_MPM_WORKER
>> +	bool "worker"
>> +	select BR2_PACKAGE_APACHE_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..a3f49a1 100644
>> --- a/package/apache/apache.mk
>> +++ b/package/apache/apache.mk
>> @@ -20,6 +20,18 @@ APACHE_CONF_ENV= \
>>  	ap_cv_void_ptr_lt_long=no \
>>  	PCRE_CONFIG=$(STAGING_DIR)/usr/bin/pcre-config
>>  
>> +ifeq ($(BR2_PACKAGE_APACHE_EVENT),y)
>> +APACHE_MPM = "event"
>> +endif
>> +
>> +ifeq ($(BR2_PACKAGE_APACHE_PREFORK),y)
>> +APACHE_MPM = "prefork"
>> +endif
>> +
>> +ifeq ($(BR2_PACKAGE_APACHE_WORKER),y)
>> +APACHE_MPM = "worker"
>> +endif
> 
> You can re-use the symbols from the choice here:
> 
>         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

 You could also add the following to Config.in:

config BR2_PACKAGE_APACHE_MPM_TYPE
	string
	default "event" if BR2_PACKAGE_APACHE_MPM_EVENT
	default "prefork" if BR2_PACKAGE_APACHE_MPM_PREFORK
	default "worker" if BR2_PACKAGE_APACHE_MPM_WORKER

and then...

> 
> (No need to test the last case, since the symbols are from a choice: one
> and only one can be enabled at a time.
> 
> Also, no need to quote the value.
> 
> Regards,
> Yann E. MORIN.
> 
> PS. See ya on Monday! ;-)
> 
>>  APACHE_CONF_OPTS = \
>>  	--sysconfdir=/etc/apache2 \
>>  	--with-apr=$(STAGING_DIR)/usr \
>> @@ -31,7 +43,7 @@ APACHE_CONF_OPTS = \
>>  	--enable-mime-magic \
>>  	--without-suexec-bin \
>>  	--enable-mods-shared=all \
>> -	--with-mpm=worker \
>> +	--with-mpm=$(APACHE_MPM) \

	--with-mpm=$(call qstrip,$(BR2_PACKAGE_APACHE_MPM_TYPE)) \


 [Not saying that you _have_ to do it like that, just pointing out a pattern we
use in a few other places.]

 Regards,
 Arnout

>>  	--disable-lua \
>>  	--disable-luajit
>>  
>> -- 
>> 2.5.0
>>
>> _______________________________________________
>> buildroot mailing list
>> buildroot at busybox.net
>> http://lists.busybox.net/mailman/listinfo/buildroot
> 

-- 
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

^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-09-11 11:08 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-09-10 19:17 [Buildroot] [PATCH 1/2] apache: add customization of MPM Fabrice Fontaine
2016-09-10 19:17 ` [Buildroot] [PATCH v3,2/2] php: add apache support Fabrice Fontaine
2016-09-10 22:08   ` Yann E. MORIN
2016-09-10 22:32     ` Yann E. MORIN
2016-09-10 21:30 ` [Buildroot] [PATCH 1/2] apache: add customization of MPM Yann E. MORIN
2016-09-11 11:08   ` Arnout Vandecappelle

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox