Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Docker Feature Request: Easy ability to change php versions for unit tests #32914

Closed
kraftbj opened this issue Sep 7, 2023 · 10 comments · Fixed by #32979
Closed

Docker Feature Request: Easy ability to change php versions for unit tests #32914

kraftbj opened this issue Sep 7, 2023 · 10 comments · Fixed by #32979

Comments

@kraftbj
Copy link
Contributor

kraftbj commented Sep 7, 2023

Impacted plugin

None / Other

What

Easily switch versions of php for the Docker container

How

Right now, if I wanted to switch php versions in Docker, I believe the best path is jetpack docker sh and use apt install to install a new version of php and required libraries.

It would be nice if there was a way to do something like jetpack docker php 8.2 or jetpack docker config --php=8.2 or whatever to switch between versions of PHP.

In addition to simply installing the basic php8.2 deb package, it would need to install the libraries that WordPress requires (e.g. php8.2-xml php8.2-mbstring php8.2-mysql).

@ice9js
Copy link
Member

ice9js commented Sep 7, 2023

Hmm... I think a --php=* option is probably the way to go. Just going by docker's best practices, I believe it's better to have a separate image for each PHP version instead of dynamically modifying an existing container. It also makes it easier to keep track of all the other little bits you mentioned.

Just thought I'd note it down for when we come back to it.

@kraftbj
Copy link
Contributor Author

kraftbj commented Sep 7, 2023

That makes sense. Do you think it'd make sense to see if possible to pull in existing php containers and separate the WordPress application from the php layer?

Of course, if an existing container doesn't work for us, we can grow our own.

@ice9js
Copy link
Member

ice9js commented Sep 7, 2023

In general yes, though looking at it right now, looks like we currently just have one image with everything on it. There is nothing inherently wrong with that, but I think it'd be good start versioning it, that way you can always pull what you need.

Official WordPress images would be a good example, with tags like 6.3.1-php8.0-apache, but you can see how that quickly that can become an issue itself. I mean, let's just take that and throw node with it's own version into the mix. Since ours is specifically meant for development, maybe we can get away with being less explicit and 'remembering' which image has what versions installed.

Another solution would be to have all PHP versions installed simultaneously which I think you can do. And we could maybe dynamically assign php to the required one.

@anomiex
Copy link
Contributor

anomiex commented Sep 7, 2023

Do you think it'd make sense to see if possible to pull in existing php containers and separate the WordPress application from the php layer?

I'm not sure what this means. But it's unlikely there are any existing base images that have PHP, Node, and pnpm all installed (and all at versions we want).

Building additional images isn't particularly hard, just time-consuming. Particularly the ARM-compatible images for Mac users.

Another solution would be to have all PHP versions installed simultaneously which I think you can do. And we could maybe dynamically assign php to the required one.

Debian and Ubuntu do support this. The downside is that the image would be larger. The current image (with 8.2) is 665MB when I rebuild it locally, adding 7.4, 8.0, and 8.1 increases it to 742MB. More versions would increase it further.

@ice9js
Copy link
Member

ice9js commented Sep 7, 2023

It's always a tradeoff. 100MB or even 200MB is a small price to pay if you ask me though, especially considering it's a set-it-and-forget-it type of thing. We won't be deploying these anywhere.

So I think that's a good place to start, and then at some point we can maybe work some CLI magic to install the extra versions on demand when you use them for the first time? But I'd say that's beyond the scope of the initial solution.

@anomiex
Copy link
Contributor

anomiex commented Sep 7, 2023

and then at some point we can maybe work some CLI magic to install the extra versions on demand when you use them for the first time?

That's not much harder really. Maybe even slightly easier. All it takes is a script that basically duplicates

&& export DEBIAN_FRONTEND=noninteractive \
&& apt-get update \
&& apt-get install -y \
libapache2-mod-php${PHP_VERSION} \
php${PHP_VERSION} \
php${PHP_VERSION}-bcmath \
php${PHP_VERSION}-cli \
php${PHP_VERSION}-curl \
php${PHP_VERSION}-intl \
php${PHP_VERSION}-ldap \
php${PHP_VERSION}-mbstring \
php${PHP_VERSION}-mysql \
php${PHP_VERSION}-opcache \
php${PHP_VERSION}-pgsql \
php${PHP_VERSION}-soap \
php${PHP_VERSION}-sqlite3 \
php${PHP_VERSION}-xdebug \
php${PHP_VERSION}-xml \
php${PHP_VERSION}-xsl \
php${PHP_VERSION}-zip \
&& apt-get install -y --no-install-recommends \
php${PHP_VERSION}-apcu \
php${PHP_VERSION}-gd \
php${PHP_VERSION}-imagick \
then runs a few extra commands to make the switch.

@anomiex
Copy link
Contributor

anomiex commented Sep 7, 2023

I created a bit of a proof-of-concept at #32929. I'm not really sure how usable any ability to select different PHP versions is really going to turn out to be though. Copying the caveats from the doc update in that PR:

  • Running jetpack docker down or otherwise recreating the containers will reset to the default PHP version.
  • If you're wanting the new version of PHP to be used to serve web requests, you'll need to jetpack docker stop && jetpack docker up -d or the like.
  • You may need to update Composer packages from inside the container before you can successfully run phpunit or the like, in order to install a version compatible with the new version of PHP.
    • For example, jetpack docker exec -- composer -d /usr/local/src/jetpack-monorepo/projects/plugins/jetpack update
    • Be careful not to commit any resulting changes to composer.lock files!
    • On Linux systems, doing this may result in odd ownership of files in relevant vendor/ and jetpack_vendor/ directories. Removing them (probaby using sudo) is a valid fix.

@anomiex
Copy link
Contributor

anomiex commented Sep 11, 2023

There's now a second proof-of-concept at #32979. That one adds a --php option to jetpack docker phpunit (and similar subcommands) so you can test with other PHP versions but does not otherwise try switching the "active" version of PHP and so avoids most of the caveats at the expense of being more limited in scope.

@anomiex
Copy link
Contributor

anomiex commented Sep 27, 2023

We wound up merging the second PR, adding jetpack docker phpunit --php to be able to run unit tests with a specific PHP version.

Someone who wanted to manually switch the php version inside the container with update-alternatives --config php and/or a2dismod/a2enmod (and possibly composer self-update --2.2 if going to PHP <7.2) could do so more easily now by using that (or the /var/scripts/ensure-php-version.sh script behind it) to download the PHP version. They'd still have to beware of the caveats mentioned earlier though.

@kraftbj kraftbj changed the title Docker Feature Request: Easy ability to change php versions Docker Feature Request: Easy ability to change php versions for unit tests Oct 5, 2023
@kraftbj
Copy link
Contributor Author

kraftbj commented Oct 5, 2023

The specific user story was for phpunit tests so going to update the title and considered fixed. If folks want/need more for general usage, let's do a new issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants