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: Add script for selecting different PHP versions #32929

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions tools/cli/commands/docker.js
Original file line number Diff line number Diff line change
Expand Up @@ -328,6 +328,8 @@ const buildExecCmd = argv => {
opts.push( 'bash' );
} else if ( cmd === 'db' ) {
opts.push( 'mysql', '--defaults-group-suffix=docker' );
} else if ( cmd === 'select-php' ) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While switching the default version on the entire PHP container could be useful when some manual debugging is required, I'm thinking the most common use case for this will be to run unit tests.
Maybe we could also expand the jetpack docker phpunit command with a --php= option like so:

jetpack docker phpunit /some/path/to/some/tests --php=7.1

What I find particularly appealing about this is the given version would only affect the single command, so you don't have to worry about switching back to what you had before.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately it's not that simple considering the need for re-running composer to get a compatible version of phpunit (and maybe other deps, like if we keep the use of johnkary/phpunit-speedtrap), and in some cases upgrading or downgrading composer to get a compatible version of that.

We could certainly try something that would only work for jetpack docker phpunit if we want. It'd still need a script like up to line 60 of the one in this PR, and we might avoid screwing up the monorepo vendor/ by installing PHPUnit and whatever other deps out-of-tree (with a composer.json making use of .config.platform.php to get the right versions) and running from there instead.

Copy link
Contributor Author

@anomiex anomiex Sep 11, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could certainly try something that would only work for jetpack docker phpunit if we want.

Proof of concept for that is in #32979.

opts.push( 'select-php', argv.version );
} else if ( cmd === 'phpunit' ) {
// @todo: Make this scale.
console.warn( chalk.yellow( 'This currently only run tests for the Jetpack plugin.' ) );
Expand Down Expand Up @@ -624,6 +626,18 @@ export function dockerDefine( yargs ) {
builder: yargExec => defaultOpts( yargExec ),
handler: argv => execDockerCmdHandler( argv ),
} )
.command( {
command: 'select-php <version>',
description:
'Select the version of PHP for use inside the container. See documentation for important notes!',
builder: yargCmd => {
yargCmd.positional( 'version', {
describe: 'The version to select, or "default".',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, can we maybe put a list of supported version numbers in there? I personally find little things like that quite helpful, even if it's just to check the expected format. It's a small list anyway.

Adding some basic validation on that would be cool too.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not here without manually hard-coding them. It depends on which versions are available from https://deb.sury.org/.

I didn't bother with validation here since the exec'ed command itself does what would be the same validation.

type: 'string',
} );
},
handler: argv => execDockerCmdHandler( argv ),
} )
.command( {
command: 'phpunit',
description: 'Run PHPUnit tests inside container',
Expand Down
38 changes: 11 additions & 27 deletions tools/docker/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,13 @@ ENV LC_ALL en_US.UTF-8

WORKDIR /tmp

# Record ARGs
RUN \
echo "PHP_VERSION=$PHP_VERSION" > /etc/docker-args.sh \
&& echo "NODE_VERSION=$NODE_VERSION" >> /etc/docker-args.sh \
&& echo "COMPOSER_VERSION=$COMPOSER_VERSION" >> /etc/docker-args.sh \
&& echo "PNPM_VERSION=$PNPM_VERSION" >> /etc/docker-args.sh

# Install basic packages, including Apache.
RUN --mount=type=cache,target=/var/lib/apt/lists/,sharing=private \
export DEBIAN_FRONTEND=noninteractive \
Expand Down Expand Up @@ -41,31 +48,12 @@ RUN --mount=type=cache,target=/var/lib/apt/lists/,sharing=private \
RUN a2enmod rewrite

# Install requested version of PHP.
COPY ./bin/select-php.sh /usr/local/bin/select-php
RUN chmod +x /usr/local/bin/select-php
COPY ./config/php.ini /var/lib/jetpack-config/php.ini
RUN --mount=type=cache,target=/var/lib/apt/lists/,sharing=private \
: "${PHP_VERSION:?Build argument PHP_VERSION needs to be set and non-empty.}" \
&& export DEBIAN_FRONTEND=noninteractive \
&& 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 \
&& /usr/local/bin/select-php "$PHP_VERSION" \
&& find /var/ -name '*-old' -delete && rm -rf /var/log/dpkg.log /var/log/alternatives.log /var/log/apt/ ~/.launchpadlib

# Install requested version of Composer.
Expand Down Expand Up @@ -109,10 +97,6 @@ RUN mkdir /usr/local/src/psysh \
# Copy a default config file for an apache host.
COPY ./config/apache_default /etc/apache2/sites-available/000-default.conf

# Copy a default set of settings for PHP (php.ini).
COPY ./config/php.ini /etc/php/${PHP_VERSION}/mods-available/jetpack-wordpress.ini
RUN phpenmod jetpack-wordpress

# Copy single site htaccess to /var/lib/jetpack-config. run.sh will move it to the site's base dir if there's none present.
COPY ./config/htaccess /var/lib/jetpack-config/htaccess
COPY ./config/htaccess-multi /var/lib/jetpack-config/htaccess-multi
Expand Down
19 changes: 19 additions & 0 deletions tools/docker/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,25 @@ wp> get_bloginfo( 'name' );

Note that each `wp shell` session counts as a single request, causing unexpected situations with WP cache. You might want to run [`wp_cache_flush()`](https://developer.wordpress.org/reference/functions/wp_cache_flush/) between requests you expect to get cached by WordPress.

### Changing PHP versions

You can select different versions of PHP. For example, to use PHP 8.0 inside the container:

```sh
jetpack docker select-php 8.0
```

If you're already inside the container after using `jetpack docker sh`, you can use `select-php 8.0` there too.

Note some caveats:

* 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.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have a couple of questions here:

  • Do we expect anything else other than PHPUnit or its dependencies to cause issues?
  • If so, it seems to me these are potentially compatibility issues that need addressing.
  • For phpunit itself, maybe it would be interesting to consider using PHAR releases instead of composer. That way we can also have multiple versions installed if need be.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we expect anything else other than PHPUnit or its dependencies to cause issues?

Some of changelogger's Symfony deps probably will too. At least some PHPCS sniffs may crash with particularly old PHP as well.

If so, it seems to me these are potentially compatibility issues that need addressing.

There's nothing we can do about upstream projects deciding to drop support for older versions of PHP more aggressively than we do.

For phpunit itself, maybe it would be interesting to consider using PHAR releases instead of composer. That way we can also have multiple versions installed if need be.

That would make it harder for everything outside of the docker container to run phpunit, as then they'd have to download the appropriate phar instead of letting Composer handle it.

We already have things set up so Composer can choose the correct version of phpunit to install based on the PHP version. The problem is that Composer has to be run with that PHP version to do that, and that composer.lock can only lock one version (and checking that into the repo is usually recommended).

* 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.

## MySQL database

You can see your database files via local file system at `./tools/docker/data/mysql`
Expand Down
93 changes: 93 additions & 0 deletions tools/docker/bin/select-php.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
#!/bin/bash

set -eo pipefail

source /etc/docker-args.sh

VER=$1
if [[ "$1" == default ]]; then
VER="$PHP_VERSION"
elif [[ ! "$1" =~ ^[0-9]+\.[0-9]+$ ]]; then
cat <<-EOF
USAGE: $0 <version>

<version> may be "default" or a two-part version number like "$PHP_VERSION".
EOF
exit 1
fi

export DEBIAN_FRONTEND=noninteractive

# Determine packages to install.
PKGS=(
"libapache2-mod-php${VER}"
"php${VER}"
"php${VER}-bcmath"
"php${VER}-cli"
"php${VER}-curl"
"php${VER}-intl"
"php${VER}-ldap"
"php${VER}-mbstring"
"php${VER}-mysql"
"php${VER}-opcache"
"php${VER}-pgsql"
"php${VER}-soap"
"php${VER}-sqlite3"
"php${VER}-xdebug"
"php${VER}-xml"
"php${VER}-xsl"
"php${VER}-zip"
)
NO_RECOMMENDS_PKGS=(
"php${VER}-apcu"
"php${VER}-gd"
"php${VER}-imagick"
)

# php-json is built in in 8.0+.
if [[ "$VER" == [57].* ]]; then
PKGS+=( "php${VER}-json" )
fi

# Install selected packages.
printf '\e[1m== Installing PHP %s ==\e[0m\n' "$VER"
apt-get update -q
apt-get install -qy "${PKGS[@]}"
apt-get install -qy --no-install-recommends "${NO_RECOMMENDS_PKGS[@]}"

# Enable our custom config for the new version.
[[ -e "/etc/php/${VER}/mods-available/jetpack-wordpress.ini" ]] || ln -s /var/lib/jetpack-config/php.ini "/etc/php/${VER}/mods-available/jetpack-wordpress.ini"
phpenmod -v "$VER" jetpack-wordpress

# Upgrade or downgrade Composer if necessary.
if command -v composer &> /dev/null; then
printf '\n\e[1m== Installing Composer ==\e[0m\n'
if [[ "$VER" == 5.6 || "$VER" == 7.[01] ]]; then
CV=2.2.18
else
CV="$COMPOSER_VERSION"
fi
# Execute with whichever version of PHP is newer.
if php -r 'exit( version_compare( PHP_VERSION, $argv[1], ">" ) ? 0 : 1 );' "$PHP_VERSION"; then
composer self-update "$CV"
else
"php$VER" "$(command -v composer)" self-update "$CV"
fi
fi

# Select the new version to be used for stuff.
printf '\n\e[1m== Setting PHP %s as default ==\e[0m\n' "$VER"
for name in php phar phar.phar; do
update-alternatives --quiet --set "$name" "/usr/bin/$name$VER"
done
if a2query -m | grep -q "^php$VER "; then
:
else
if a2query -m | grep -q '^php'; then
a2dismod 'php*'
fi
a2enmod "php$VER"

printf '\n\e[30;43mThe web server is still running the old version of PHP!\e[0m\n'
printf '\e[30;43mRestart the docker container (e.g. `jetpack docker stop && jetpack docker up -d`) to use the new version.\e[0m\n'
fi