Skip to content

fix: guard PHP-FPM log dir setup against unset PHP_LOG_DIR#177

Merged
yaronkoren merged 1 commit into
masterfrom
fix-174-php-log-dir-guard
Jun 26, 2026
Merged

fix: guard PHP-FPM log dir setup against unset PHP_LOG_DIR#177
yaronkoren merged 1 commit into
masterfrom
fix-174-php-log-dir-guard

Conversation

@cicalese

Copy link
Copy Markdown
Contributor

Summary

Fixes #174. The PHP-FPM log-dir block in run-all.sh references PHP_LOG_DIR, which is never defined in the image. The empty value made the mountpoint test run against / (always a mountpoint), so the else branch ran chgrp -R www-data '' and chmod -R g=rwX '' — printing cannot access '': No such file or directory on every container boot since #420.

Fix

Guard the block so it is a no-op when PHP_LOG_DIR is unset. The Apache and MediaWiki log-dir blocks are unchanged (their vars are defined and they work).

Chose to guard rather than delete because PHP-FPM logs are a legitimate future input for the Observability profile — tracked in CanastaWiki/Canasta-CLI#839. The guard stops the boot-time noise now and preserves the scaffolding for that wire-up.

Testing

bash -n clean. With PHP_LOG_DIR unset the block logs a single skip line and runs no chgrp/chmod; the errors no longer appear in boot logs.

PHP_LOG_DIR is never defined in the image, so the PHP-FPM log-dir block
expanded to an empty path: the mountpoint test ran against "/" (always a
mountpoint), taking the else branch and running chgrp/chmod on '', which
printed 'cannot access: No such file or directory' on every container boot.

Guard the block so it is a no-op until PHP_LOG_DIR is defined, eliminating
the boot-time log noise without changing behavior for the Apache and
MediaWiki log dirs.
@cicalese cicalese requested a review from yaronkoren June 22, 2026 14:58
@github-actions

Copy link
Copy Markdown

🐳 The image based on 5b829f30 commit has been built with 1.43.8-20260622-177 tag as ghcr.io/canastawiki/canasta-base:1.43.8-20260622-177

@cicalese

cicalese commented Jun 22, 2026

Copy link
Copy Markdown
Contributor Author

@naresh-kumar-babu This issue originated with this commit in July 2024. What is missing from this PR to satisfy the goals of the original commit or to wire this functionality into the Observability profile?

@naresh-kumar-babu

Copy link
Copy Markdown
Collaborator

@naresh-kumar-babu This issue originated with this commit in July 2024. What is missing from this PR to satisfy the goals of the original commit or to wire this functionality into the Observability profile?

We might need to add an Observability profile definition that mounts the log volume from run-all.sh

@cicalese

cicalese commented Jun 22, 2026

Copy link
Copy Markdown
Contributor Author

@naresh-kumar-babu This issue originated with this commit in July 2024. What is missing from this PR to satisfy the goals of the original commit or to wire this functionality into the Observability profile?

We might need to add an Observability profile definition that mounts the log volume from run-all.sh

Should that go in this PR or a separate PR? Put another way, what else needs to happen for this to be a complete solution for your original commit versus additional work that needs to happen in Canasta-CLI (CanastaWiki/Canasta-CLI#839)?

@naresh-kumar-babu

Copy link
Copy Markdown
Collaborator

@naresh-kumar-babu This issue originated with this commit in July 2024. What is missing from this PR to satisfy the goals of the original commit or to wire this functionality into the Observability profile?

We might need to add an Observability profile definition that mounts the log volume from run-all.sh

Should that go in this PR or a separate PR? Put another way, what else needs to happen for this to be a complete solution for your original commit versus additional work that needs to happen in Canasta-CLI (CanastaWiki/Canasta-CLI#839)?

I don't have full context on the code as it is originally from wikiteq (CanastaWiki/Canasta#388). I just worked on the code migration from wikiteq branch to canasta and made sure that it worked as expected.

@yaronkoren yaronkoren merged commit df14ca5 into master Jun 26, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Container entrypoint chgrp on empty variable

3 participants