diff mbox

[1/1] lighttpd: install fastcgi config for php

Message ID 1417559724-25091-1-git-send-email-bluemrp9@gmail.com
State Rejected
Headers show

Commit Message

Ryan Coe Dec. 2, 2014, 10:35 p.m. UTC
Signed-off-by: Ryan Coe <bluemrp9@gmail.com>
---
 package/lighttpd/fastcgi.conf |  7 +++++++
 package/lighttpd/lighttpd.mk  | 11 +++++++++++
 2 files changed, 18 insertions(+)
 create mode 100644 package/lighttpd/fastcgi.conf

Comments

Floris Bos Dec. 2, 2014, 11:56 p.m. UTC | #1
On 12/02/2014 11:35 PM, Ryan Coe wrote:
> Signed-off-by: Ryan Coe <bluemrp9@gmail.com>
> ---
>   package/lighttpd/fastcgi.conf |  7 +++++++
>   package/lighttpd/lighttpd.mk  | 11 +++++++++++
>   2 files changed, 18 insertions(+)
>   create mode 100644 package/lighttpd/fastcgi.conf
>
> diff --git a/package/lighttpd/fastcgi.conf b/package/lighttpd/fastcgi.conf
> new file mode 100644
> index 0000000..2ed8bea
> --- /dev/null
> +++ b/package/lighttpd/fastcgi.conf
> @@ -0,0 +1,7 @@
> +server.modules += ( "mod_fastcgi" )
> +
> +fastcgi.server = ( ".php" => ((
> +                     "bin-path" => "/usr/bin/php-cgi",
> +                     "socket" => "/tmp/php.socket"
> +                 )))
> +
> diff --git a/package/lighttpd/lighttpd.mk b/package/lighttpd/lighttpd.mk
> index 8f34561..e4a3020 100644
> --- a/package/lighttpd/lighttpd.mk
> +++ b/package/lighttpd/lighttpd.mk
> @@ -78,6 +78,17 @@ endef
>   
>   LIGHTTPD_POST_INSTALL_TARGET_HOOKS += LIGHTTPD_INSTALL_CONFIG
>   
> +define LIGHTTPD_INSTALL_PHP_CONFIG
> +	$(INSTALL) -D -m 0644 package/lighttpd/fastcgi.conf \
> +		$(TARGET_DIR)/etc/lighttpd/conf.d/fastcgi.conf
> +	$(SED) 's/#include \"conf.d\/fastcgi.conf\"/include \"conf.d\/fastcgi.conf\"/' \
> +		$(TARGET_DIR)/etc/lighttpd/modules.conf
> +endef
> +
> +ifeq ($(BR2_PACKAGE_PHP_SAPI_CLI_CGI)$(BR2_PACKAGE_PHP_SAPI_CGI),y)
> +	LIGHTTPD_POST_INSTALL_TARGET_HOOKS += LIGHTTPD_INSTALL_PHP_CONFIG
> +endif

You are making the assumption here that if someone selects the CGI SAPI, 
he wants to use the FastCGI interface.
Personally I would prefer plain old regular CGI above FastCGI.

Problem with the FastCGI implementation is that it creates a fixed 
number of PHP processes.
Consumes memory even when the webserver isn't executing any PHP script.
And doesn't work too well with PHP scripts that perform blocking 
operations or depend on any external server. If all PHP processes are 
occupied executing a script, the webserver cannot process any additional 
requests until a process becomes available again.


Yours sincerely,

Floris Bos
Thomas Petazzoni Dec. 7, 2014, 10:50 p.m. UTC | #2
Dear Ryan Coe,

On Tue,  2 Dec 2014 14:35:24 -0800, Ryan Coe wrote:
> Signed-off-by: Ryan Coe <bluemrp9@gmail.com>
> ---
>  package/lighttpd/fastcgi.conf |  7 +++++++
>  package/lighttpd/lighttpd.mk  | 11 +++++++++++
>  2 files changed, 18 insertions(+)
>  create mode 100644 package/lighttpd/fastcgi.conf

Seeing the feedback from Floris Bos on this patch, I marked your patch
as 'Rejected' in our patch tracking system. Please work with Floris on
a variant of your patch that takes into account also his use case
(maybe by making the fastcgi installation optional?).

Thanks!

Thomas
Ryan Coe Dec. 7, 2014, 10:56 p.m. UTC | #3
Thomas, Floris, All,

On 12/07/2014 02:50 PM, Thomas Petazzoni wrote:
> Dear Ryan Coe,
>
> On Tue,  2 Dec 2014 14:35:24 -0800, Ryan Coe wrote:
>> Signed-off-by: Ryan Coe <bluemrp9@gmail.com>
>> ---
>>   package/lighttpd/fastcgi.conf |  7 +++++++
>>   package/lighttpd/lighttpd.mk  | 11 +++++++++++
>>   2 files changed, 18 insertions(+)
>>   create mode 100644 package/lighttpd/fastcgi.conf
> Seeing the feedback from Floris Bos on this patch, I marked your patch
> as 'Rejected' in our patch tracking system. Please work with Floris on
> a variant of your patch that takes into account also his use case
> (maybe by making the fastcgi installation optional?).
>
> Thanks!
>
> Thomas

Yes, of course.  This makes sense.
Floris Bos Dec. 8, 2014, 12:19 a.m. UTC | #4
On 12/07/2014 11:50 PM, Thomas Petazzoni wrote:
> Dear Ryan Coe,
>
> On Tue,  2 Dec 2014 14:35:24 -0800, Ryan Coe wrote:
>> Signed-off-by: Ryan Coe <bluemrp9@gmail.com>
>> ---
>>   package/lighttpd/fastcgi.conf |  7 +++++++
>>   package/lighttpd/lighttpd.mk  | 11 +++++++++++
>>   2 files changed, 18 insertions(+)
>>   create mode 100644 package/lighttpd/fastcgi.conf
> Seeing the feedback from Floris Bos on this patch, I marked your patch
> as 'Rejected' in our patch tracking system. Please work with Floris on
> a variant of your patch that takes into account also his use case
> (maybe by making the fastcgi installation optional?).

Yes, a config option under the lighttpd package to optionally install 
and configure PHP/FastCGI support might be nicer, instead of making the 
assumption based on the selection of the PHP CGI SAPI.


You might also want to consider using PHP-FPM
That does allow using FastCGI while spawning/killing PHP processes 
on-demand (instead of having a static number), so shouldn't suffer from 
the problems I mentioned earlier.
But haven't tested it myself.

Another point of attention is that lighttpd currently runs as root.
Might want to add a less privileged user to the system to run 
lighttpd/php under.


Yours sincerely,

Floris Bos
diff mbox

Patch

diff --git a/package/lighttpd/fastcgi.conf b/package/lighttpd/fastcgi.conf
new file mode 100644
index 0000000..2ed8bea
--- /dev/null
+++ b/package/lighttpd/fastcgi.conf
@@ -0,0 +1,7 @@ 
+server.modules += ( "mod_fastcgi" )
+
+fastcgi.server = ( ".php" => ((
+                     "bin-path" => "/usr/bin/php-cgi",
+                     "socket" => "/tmp/php.socket"
+                 )))
+
diff --git a/package/lighttpd/lighttpd.mk b/package/lighttpd/lighttpd.mk
index 8f34561..e4a3020 100644
--- a/package/lighttpd/lighttpd.mk
+++ b/package/lighttpd/lighttpd.mk
@@ -78,6 +78,17 @@  endef
 
 LIGHTTPD_POST_INSTALL_TARGET_HOOKS += LIGHTTPD_INSTALL_CONFIG
 
+define LIGHTTPD_INSTALL_PHP_CONFIG
+	$(INSTALL) -D -m 0644 package/lighttpd/fastcgi.conf \
+		$(TARGET_DIR)/etc/lighttpd/conf.d/fastcgi.conf
+	$(SED) 's/#include \"conf.d\/fastcgi.conf\"/include \"conf.d\/fastcgi.conf\"/' \
+		$(TARGET_DIR)/etc/lighttpd/modules.conf
+endef
+
+ifeq ($(BR2_PACKAGE_PHP_SAPI_CLI_CGI)$(BR2_PACKAGE_PHP_SAPI_CGI),y)
+	LIGHTTPD_POST_INSTALL_TARGET_HOOKS += LIGHTTPD_INSTALL_PHP_CONFIG
+endif
+
 define LIGHTTPD_INSTALL_INIT_SYSV
 	$(INSTALL) -D -m 0755 package/lighttpd/S50lighttpd \
 		$(TARGET_DIR)/etc/init.d/S50lighttpd