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

Add PHP 7.3 preview (xdebug in rc version) #32

Merged
merged 4 commits into from Feb 13, 2019

Conversation

3 participants
@andrerom
Copy link
Member

andrerom commented Dec 14, 2018

Calling this preview as memcached is held up from being released on PHP 7.3 atm, see inline link.

@andrerom andrerom requested a review from vidarl Dec 14, 2018

@andrerom andrerom referenced this pull request Dec 14, 2018

Merged

[Travis] Add testing on PHP 7.3 #2504

4 of 4 tasks complete
@andrerom

This comment has been minimized.

Copy link
Member Author

andrerom commented Dec 18, 2018

First failure: difference in libzip usage during build, could have given it options to use bundled version but UPGRADE suggests not doing that

Second failure: Blackfire not found for 7.3, 1h after the build they tweeted support was added.

Third failure: xdebug, trying beta channel even if that is from september, but results in app_1 | Segmentation fault (core dumped)

Summary:

@sodabrew

This comment has been minimized.

Copy link

sodabrew commented Dec 21, 2018

@andrerom andrerom force-pushed the php7.3 branch from 96be1d5 to 7ad72f8 Feb 11, 2019

@andrerom andrerom requested a review from mnocon Feb 12, 2019

## TODO: The tag aliases here will not be needed once eZ Platform uses PHP_IMAGE variable within Dockerfile's from section
docker tag ez_php:latest "ezsystems/php:7.2-v1"
docker tag ez_php:latest "ezsystems/php:7.1-v1"
docker tag ez_php:latest "ezsystems/php:7.0-v1"

This comment has been minimized.

@andrerom

andrerom Feb 12, 2019

Author Member

FYI: This is removed as it's not needed anymore as the configurable injection of this is now part of both 1.13 and 2.x releases:
ezsystems/ezplatform@cb67948

@andrerom

This comment has been minimized.

Copy link
Member Author

andrerom commented Feb 12, 2019

@mnocon @vidarl I think this is ready for review, even if xdebug is still unstable (rc) I think we can go ahead here to start to test out PHP 7.3 in other repos.

@mnocon

mnocon approved these changes Feb 13, 2019

Copy link
Member

mnocon left a comment

Looks good to me.

One question that came to my mind, not really related to this PR:
To start using Node on Travis, I've simply set PHP_IMAGE to the Node flavour (https://github.com/ezsystems/ezplatform/pull/345/files#diff-f579cccc964135c7d644c7b2d3b0d3ecR13).

Does it mean, that for v2 testing this line https://github.com/ezsystems/docker-php/pull/32/files#diff-6757d9ef6e7f3b3c85ea7eb11b9af47fR84 should be changed and PHP_IMAGE should be set based on EZ_VERSION (ez_php:latest for v1 and ez_php:latest-node for testing on version 2.5)? If yes I can create a follow-up for that.

@andrerom andrerom changed the title Add PHP 7.3 preview (w/o memcached) Add PHP 7.3 preview (xdebug in rc version) Feb 13, 2019

@andrerom andrerom merged commit 22cd9e8 into master Feb 13, 2019

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@andrerom andrerom deleted the php7.3 branch Feb 13, 2019

@sodabrew

This comment has been minimized.

Copy link

sodabrew commented Feb 13, 2019

Is the text specifying w/o memcached still accurate? I see the memcached extension is built in this Dockerfile.

@andrerom

This comment has been minimized.

Copy link
Member Author

andrerom commented Feb 13, 2019

@sodabrew Github usability bug, I renamed the PR title to update state, then hit merge and the title of the merge commit reflect the old title of the PR. Reported it to Github several months ago ;)

@sodabrew

This comment has been minimized.

Copy link

sodabrew commented Feb 13, 2019

The README still says no memcached at https://github.com/ezsystems/docker-php#images.

@andrerom

This comment has been minimized.

Copy link
Member Author

andrerom commented Feb 13, 2019

The README still says no memcached however.

Good catch: e7534b3

One question that came to my mind

@mnocon hmm,
what I had in mind was that we use node image in these two places:

So just fixing the env variables won't do.
I didn't check, but does Docker in these places allow us to concat? So we have a PHP_BASE_IMAGE env/arg that we add "-node" or "-dev" to when we need a specific flavour (?)

@mnocon

This comment has been minimized.

Copy link
Member

mnocon commented Feb 13, 2019

Yes, it is possible. Something like:

(base-prod.yml)
version: '3.3'
# Simple single server setup for prod

services:
    app:
      build:
        context: .
        dockerfile: ${APP_DOCKER_FILE}
        args:
         - PHP_IMAGE
(.env)
PHP_IMAGE=ezsystems/php:7.2-v1
APP_DOCKER_FILE=Dockerfile-app
(Dockerfile-app)
ARG PHP_IMAGE=ezsystems/php:7.2-v1-dev
FROM ${PHP_IMAGE}-node

RUN php -v
RUN node -v

builds the image with Node flavour.

I'll amend my PR with that and ping you there, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.
You signed in with another tab or window. Reload to refresh your session. You signed out in another tab or window. Reload to refresh your session.