search

TrafeX / docker-php-nginx

Docker image with PHP-FPM 8.3 & Nginx 1.24 on Alpine Linux
https://hub.docker.com/r/trafex/php-nginx
MIT License
1.33k stars 721 forks source link

Clean up nginx config #163

Open jduan00 opened 6 months ago

jduan00 commented 6 months ago

Just want to share two small proposed changes:

1) Remove /etc/nginx/fastcgi.conf as it is not used and likely cause confusion. fastcgi_params is actually used. 2) Use the following line for relevant references (ex. /etc/nginx/conf.d/default.conf, fastcgi_params)

fastcgi_param   SCRIPT_FILENAME         $request_filename;
TrafeX commented 6 months ago

Hi @jduan00,

Thank you for the tips. I've looked into it and came to the following;

  1. It's indeed not used, but it's part of the default nginx installation. I prefer to leave this alone because this container image serves as an example to adjust to your needs and I want to keep it simple and as standard as possible.
  2. That's indeed a good alternative for $document_root$fastcgi_script_name, but it's doesn't do exactly the same. I'm afraid this can create a backwards compatibility break, while it isn't a major improvement.

What I did change is removing the duplicate SCRIPT_NAME: https://github.com/TrafeX/docker-php-nginx/commit/b829e435a1125a4293e11e7441a14f811d682c26

jduan00 commented 6 months ago

Hi @TrafeX,

Thanks for the following up. It is matter of personal taste. I want to make it a min set of files within the container, thus I took the axe to get rid of anything that is not used.

Also, I appreciate you pointed out the difference about SCRIPT_FILENAME. https://serverfault.com/questions/465607/nginx-document-rootfastcgi-script-name-vs-request-filename

However, for our Laravel based PHP apps, the one works is: fastcgi_param SCRIPT_FILENAME $request_filename;

Cheers,

Jack