search

wyveo / nginx-php-fpm

Nginx + PHP-FPM 8.2.x / 8.1.x / 8.0.x / 7.4.x / 7.3.x / 7.2.x / 7.1.x / 7.0.x + Composer built on Debian (Bullseye/Buster) image
https://hub.docker.com/r/wyveo/nginx-php-fpm
MIT License
343 stars 248 forks source link

Chown format #15

Closed manuasir closed 3 years ago

manuasir commented 3 years ago

https://github.com/wyveo/nginx-php-fpm/blob/74261ab67d8520b9c66ae275b528f33859a1b4f2/start.sh#L8

Shouldn't this be chown nginx:nginx ?

colinwilson commented 3 years ago

The period is supported as a separator with chown. See info chown:

Some older scripts may still use ‘.’ in place of the ‘:’ separator.
POSIX 1003.1-2001 (*note Standards conformance::) does not require
support for that, but for backward compatibility GNU ‘chown’ supports
‘.’ so long as no ambiguity results.  New scripts should avoid the use
of ‘.’ because it is not portable, and because it has undesirable
results if the entire OWNER‘.’GROUP happens to identify a user whose
name contains ‘.’.

But yes, this should be updated to nginx:nginx. Will update all branches soon.

Thanks for pointing it out.

manuasir commented 3 years ago

Thanks for your quick reply! I just opened a PR to the master branch. Just let me know what branches do you want the change point to and I'll open one for each branch. Otherwise, feel free to cherry-pick the commit.

colinwilson commented 3 years ago

Yep. your PR should be applied to all branches. You're welcome to open one for each branch. Thanks again.

manuasir commented 3 years ago

Done!

colinwilson commented 3 years ago

Awesome. Thanks for contributing. Much appreciated.