use phpenv #114
use phpenv #114
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0b832eba15
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Dockerfile
Outdated
| phpenv rehash \ | ||
| phpenv global ${PHP_VERSIONS%% *} |
There was a problem hiding this comment.
In this bash -lc block, phpenv rehash is immediately followed by phpenv global ... without && or ;, so the line continuation concatenates them into a single command (phpenv rehash phpenv global ...). That causes phpenv rehash to receive unexpected arguments and fail during the Docker build, preventing the image from building. Add a command separator between these two lines.
Useful? React with 👍 / 👎.
willwang-openai
force-pushed
the
use-phpenv
branch
2 times, most recently
from
e9af4e2 to
f8706d3
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f8706d3311
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| for v in $PHP_VERSIONS; do \ | ||
| phpenv install -s "${v}snapshot"; \ | ||
| done && \ |
There was a problem hiding this comment.
In the PHP install block, the for loop runs phpenv install -s "${v}snapshot" without set -e or per-iteration error handling, so if an early version fails to build (e.g., a snapshot missing for an architecture), the loop can still exit 0 as long as the last install succeeds (Dockerfile:298-300). That leaves the image missing one of the requested PHP versions and will later break phpenv global or user selection for that version without an obvious build failure. Consider adding set -e inside the bash -lc script or phpenv install ... || exit 1 to fail fast.
Useful? React with 👍 / 👎.
willwang-openai
force-pushed
the
use-phpenv
branch
from
f8706d3 to
70ac57b
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 70ac57b03a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| eval "$(phpenv init -)" && \ | ||
| for v in $PHP_VERSIONS; do \ | ||
| phpenv install -s "${v}snapshot"; \ | ||
| done && \ |
There was a problem hiding this comment.
The switch from mise install php@… to phpenv install … removes any explicit Composer installation. phpenv/php-build only compiles PHP, so unless Composer is installed separately, composer --version in verify.sh will fail and the image build breaks (and users lose Composer at runtime). If Composer isn’t provided elsewhere in the image, add an explicit Composer install step (e.g., download the PHAR to /usr/local/bin) or ensure php-build installs it.
Useful? React with 👍 / 👎.
willwang-openai
force-pushed
the
use-phpenv
branch
from
dd78da6 to
da82b2f
Compare
1 participant
No description provided.