Skip to content
This repository has been archived by the owner on Oct 19, 2023. It is now read-only.

Add support for php7.4 and 8.0 #533

Merged
merged 1 commit into from
Oct 6, 2021

Conversation

frost-byte
Copy link
Contributor

Fix for #529
Enable Verbose Debugging for gcloud build
Authorize service account in install_test_dependencies.sh
Remove Unsupported Versions < 7.3
Update PHP versions to latest releases
Update PHPUnit to v9.5.4 for all tests
Build and include librabbitmq as library for amqp extension
Use Datastax's libuv1 library package
Use updated pecl extension versions
Disable apcu_bc extension for newer versions
Update Couchbase to v3
Switch to container-structure-test
Use the ev extension when testing for shared, disabled extension
Make the container for the Test Runner base image configurable using env
Update check_versions.sh to include substition for container registry
Use apcu extension in php80 instead of apcu_bc
Use bionic package for libvips
Disable build of sodium extension, it is included natively
Use latest release from github for pq extension
Use latest release from github for raphf extension
Create xmlrpc extension
Update tests to use v5 of symfony/browser-kit
Disable stackdriver_debugger for php8

See previous pr for some additional details

Fix for GoogleCloudPlatform#529
Enable Verbose Debugging for gcloud build
Authorize service account in install_test_dependencies.sh
Remove Unsupported Versions < 7.3
Update PHP versions to latest releases
Update PHPUnit to v9.5.4 for all tests
Build and include librabbitmq as library for amqp extension
Use Datastax's libuv1 library package
Use updated pecl extension versions
Disable apcu_bc extension for newer versions
Update Couchbase to v3
Switch to container-structure-test
Use the ev extension when testing for shared, disabled extension
Make the container for the Test Runner base image configurable using env
Update `check_versions.sh` to include substition for container registry
Use apcu extension in php80 instead of apcu_bc
Use bionic package for libvips
Disable build of sodium extension, it is included natively
Use latest release from github for pq extension
Use latest release from github for raphf extension
Create xmlrpc extension
Update tests to use v5 of `symfony/browser-kit`
Disable stackdriver_debugger for php8
@tiptopcoder
Copy link

Thanks for this PR, do we have any estimation when this PR will be merged and docker image will be released?

I'm in need of upgrading to 7.4 to leverage some new features.

@@ -43,19 +43,20 @@ We currently support the latest patch version of 5.6, 7.0, 7.1, and 7.2. See
* SeasLog
* stackdriver_debugger
* stomp
* suhosin
* suhosin (5.6)

Choose a reason for hiding this comment

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

Since 5.6 isn't supported anymore wouldn't this be removed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have no idea what the policy is regarding removing support for previous versions.
If it was not removed when support was added for 7.+ then why should I be the one to remove it?

@@ -12,7 +12,7 @@ PNAME="gcp-php${SHORT_VERSION}-apcu"
if [ ${SHORT_VERSION} == '56' ]; then
download_from_pecl apcu 4.0.11
else
download_from_pecl apcu
download_from_pecl apcu 5.1.20

Choose a reason for hiding this comment

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

Why lock the version?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How can I ensure that the latest version will be selected? Is it possible to specify a range of versions that can be used? (Similar to how composer or npm/yarn allows you to specify a minimum version, for example)

@@ -6,7 +6,7 @@ echo "Building apm for gcp-php${SHORT_VERSION}"

PNAME="gcp-php${SHORT_VERSION}-apm"

if [ ${SHORT_VERSION} == '56' ]; then
if [ ${SHORT_VERSION} == '56' ] || [ ${SHORT_VERSION} == '80' ]; then
echo "apm extension only for PHP 7.0+, relies on removed json.h for PHP 5.6"

Choose a reason for hiding this comment

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

Seems like this message should be updated to say it doesn't work on 8.0, as well. It only references 5.6 but with the additional condition it's no longer accurate.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I could change the message to say apm extension only for PHP 7.*, relies on removed json.h for PHP 5.6

@@ -9,6 +9,6 @@ echo "Building mongodb for gcp-php${SHORT_VERSION}"
PNAME="gcp-php${SHORT_VERSION}-mongodb"

# Download the source
download_from_pecl mongodb
download_from_pecl mongodb 1.9.1

Choose a reason for hiding this comment

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

Why lock the version here as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

See my comment above about apcu

@donmccasland
Copy link
Member

Reviewing this PR. While running the package-builder cloudbuild:
gcloud builds submit . --config=build-packages.yaml --substitutions _PHP_VERSION=8.0.8-1,_GOOGLE_PROJECT_ID=php-mvm-a-280519,_BUCKET=php-mvm-a --timeout=600m

We get this error:
Step #6 - "package-build": ./configure --build=x86_64-linux-gnu --prefix=/usr --includedir=${prefix}/include --mandir=${prefix}/share/man --infodir=${prefix}/share/info --sysconfdir=/etc --localstatedir=/var --disable-silent-rules --libdir=${prefix}/lib/x86_64-linux-gnu --libexecdir=${prefix}/lib/x86_64-linux-gnu --disable-maintainer-mode --disable-dependency-tracking --prefix=/opt/php80 --verbose --with-config-file-path=/opt/php80/lib --with-config-file-scan-dir=/opt/php80/lib/ext.enabled:/opt/php80/lib/conf.d --enable-sysvsem --enable-sysvshm --enable-sysvmsg --disable-cgi --enable-bcmath=shared --enable-calendar=shared --enable-exif=shared --enable-fpm --enable-ftp=shared --enable-intl=shared --enable-mbstring --enable-mysqlnd --enable-opcache-file --enable-pcntl --enable-shared --enable-shmop=shared --enable-soap=shared --enable-sockets --enable-libxml --enable-zip --with-bz2 --with-curl --with-gettext=shared --with-external-gd=shared --with-gd=shared --with-gmp --with-freetype --with-freetype-dir=/usr --with-jpeg --with-jpeg-dir=/usr --with-mcrypt --with-pdo_sqlite=shared,/usr --without-pdo-sqlite=shared,/usr --with-pdo-pgsql --with-pear --with-pgsql --with-xmlrpc=shared --with-libxml --with-xsl=shared --with-fpm-user=www-data --with-fpm-group=www-data --with-mysql=mysqlnd --with-mysqli=mysqlnd --with-pdo-mysql=mysqlnd --with-openssl --with-readline --with-recode --with-sodium=/usr --with-png-dir=/usr --with-webp --with-webp-dir=/usr --with-xpm --with-xpm-dir=shared --with-zip --with-zlib --with-zlib-dir=/usr
Step #6 - "package-build": configure: error: invalid package name: pdo-sqlite=shared,/usr

@frost-byte
Copy link
Contributor Author

This might be due to the sqlite3 package not being installed?

Perhaps libsqlite3-0 needs to be added to php-base/Dockerfile?

@donmccasland
Copy link
Member

This might be due to the sqlite3 package not being installed?

Perhaps libsqlite3-0 needs to be added to php-base/Dockerfile?

Actually this in the package-builder/Dockerfile, which definitely has libsqlite3-0 in it already. I think potentially the issue is that we've got conflicting options for ./configure:
--with-pdo_sqlite=shared,/usr --without-pdo-sqlite=shared,/usr

It is either that OR the fact that the pdo_sqlite page (https://www.php.net/manual/en/ref.pdo-sqlite.php) says:
"As of PHP 7.4.0 » libsqlite ≥ 3.5.0 is required.:

@frost-byte
Copy link
Contributor Author

Actually this in the package-builder/Dockerfile, which definitely has libsqlite3-0 in it already. I think potentially the issue is that we've got conflicting options for ./configure:
--with-pdo_sqlite=shared,/usr --without-pdo-sqlite=shared,/usr

It is either that OR the fact that the pdo_sqlite page (https://www.php.net/manual/en/ref.pdo-sqlite.php) says:
"As of PHP 7.4.0 » libsqlite ≥ 3.5.0 is required.:

I was referring to that as well, I think the --with-pdo_sqlite=shared,/usr option is only necessary if you want to maintain support for versions before 7.4. I wasn't sure how to proceed in that regard, even though I did remove most of the references to versions before 7.3.

That was one of the bigger challenges or questions I had - what is the philosophy or approach to maintaining previous versions that are no longer supported?

@jaakkom
Copy link

jaakkom commented Aug 9, 2021

@donmccasland When do we get review :)

@andreladocruz
Copy link

@donmccasland When do we get review :) +1

@jaakkom
Copy link

jaakkom commented Oct 6, 2021

@tiptopcoder @andreladocruz @derekherman please review someone :)

Copy link

@andreladocruz andreladocruz left a comment

Choose a reason for hiding this comment

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

For me... it's approved =)

@donmccasland
Copy link
Member

Hey folks, thanks to frost-byte for this massive patch! I just finished reviewing it and everything looks right. Let's merge it and try to get a build of this up.

@donmccasland donmccasland merged commit 3d8f934 into GoogleCloudPlatform:master Oct 6, 2021
@andreladocruz
Copy link

@donmccasland, news about these new builds?

Still missing php 7.4 and 8.0 in flex environment.

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

Successfully merging this pull request may close these issues.

6 participants