Skip to content

Add PHP 7.3-rc #677

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

Merged
merged 9 commits into from
Aug 14, 2018
Merged

Add PHP 7.3-rc #677

merged 9 commits into from
Aug 14, 2018

Conversation

TimWolla
Copy link
Contributor

@TimWolla TimWolla commented Jun 26, 2018

Did a build with stretch-fpm + a bunch of extensions.

image

@TimWolla
Copy link
Contributor Author

Tests fail for alpine only. A similar error message has been reported for qemu: https://bugs.alpinelinux.org/issues/5768

I don't know enough about alpine / musl / ld / PHP / libssl to advise on this.

@TimWolla
Copy link
Contributor Author

And there's also this bug report for PHP itself: https://bugs.php.net/bug.php?id=76392

@TimWolla
Copy link
Contributor Author

Probably will be fixed when php/php-src#3349 is merged.

@tianon
Copy link
Member

tianon commented Jul 9, 2018

Looks like the version of Argon2 in Stretch also isn't new enough for PHP 7.3's requirements. 😞

@tianon
Copy link
Member

tianon commented Jul 9, 2018

@TimWolla linked to php/php-src@55277a6 in IRC (chased down the commit which added this new version restriction)

@TimWolla
Copy link
Contributor Author

As discussed in IRC: I added apt pinning to buster for libargon2. I opted to pin libargon2 for both 7.2 and 7.3 running on Stretch. If the changes are fine then this PR should “go green” once PHP 7.3 RC 1 lands, because that fixes the alpine issue.

.travis.yml Outdated
- VERSION=7.3-rc VARIANT=alpine3.7/cli
- VERSION=7.3-rc VARIANT=alpine3.7/fpm
- VERSION=7.3-rc VARIANT=alpine3.7/zts
- VERSION=7.3-rc VARIANT=alpine3.6/cli
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 want alpine 3.6 / 3.7 for 7.3 or should we go full 3.8?

Copy link
Member

Choose a reason for hiding this comment

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

Yes, let's do only Alpine 3.8 for php:7.3. The plan is to migrate the others to 3.8 eventually anyway (probably by dropping 3.6).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

check.

@yosifkit
Copy link
Member

yosifkit commented Jul 12, 2018

It seems the alpine versions are still hitting https://bugs.php.net/bug.php?id=76392.

@TimWolla
Copy link
Contributor Author

TimWolla commented Jul 12, 2018 via email

@TimWolla
Copy link
Contributor Author

All the 7.3-rc variants are GREEN now 🙌 PHP 7.2 and lower is still building.

@tianon @yosifkit I believe this PR is ready to be reviewed and merged. Take a look at the two issues I referenced above, they are more or less related to the changed I did.

@khs1994
Copy link

khs1994 commented Jul 19, 2018

Alpine 3.8 include wget redis/docker-library-redis#151

@yosifkit
Copy link
Member

Given that libargon2-0-dev has some strange transition issues in Debian Buster/Sid (specifically related to libargon2-0-dev, libargon2-1-dev, and libargon2-dev, https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=902972), I am a bit hesitant to use it since it is so unstable/uncertain.

We might as well just download and compile it ourselves until it is in a stable release on Debian. cc @tianon.

Perhaps we should put the argon2 download/install in a Dockerfile part like the apache and fpm blocks?

@TimWolla
Copy link
Contributor Author

Given that libargon2-0-dev has some strange transition issues in Debian Buster/Sid (specifically related to libargon2-0-dev, libargon2-1-dev, and libargon2-dev, https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=902972), I am a bit hesitant to use it since it is so unstable/uncertain.

If I understand it correctly then libargon2-0-dev is being renamed to libargon2-dev, because Debian does not want to ship several minor versions (the 0 being superfluous then). libargon2-0dev is a virtual package pulling in libargon2-dev (libargon2-1) in sid. libargon2.1 is API compatible with libargon2.0 as I understand it.

Conclusion: The only thing that may change in the near future is the ABI, which is irrelevant here, because we compile from source.

@TimWolla
Copy link
Contributor Author

We might need to update the apt pinning from libargon2-0* to libargon2*, though.

@TimWolla
Copy link
Contributor Author

Ooookay, I actually just performed a local build against sid, which already has the changes:

In the current form the pinning will break the build once the migration to buster happens:

root@bfcc019cf041:/# apt install libargon2-0-dev
Reading package lists... Done
Building dependency tree       
Reading state information... Done
Some packages could not be installed. This may mean that you have
requested an impossible situation or if you are using the unstable
distribution that some required packages have not yet been created
or been moved out of Incoming.
The following information may help to resolve the situation:

The following packages have unmet dependencies:
 libargon2-0-dev : Depends: libargon2-0 (= 0~20160821-1+b1) but 0~20171227-0.1 is to be installed
E: Unable to correct problems, you have held broken packages.

My suggestion thus would be: Pin libargon to sid instead of buster. As buster usually is only about 10 days behind sid for most packages the pinning difference should be negligible. When libargon is pinned to sid one has to change libargon2-0-dev to libargon2-dev in the actual apt-get install invocation, since the former refers to stretch (probably a side effect of it being a virtual package in sid).

After making these changes php:7.3-cli builds fine for me:

[timwolla@/s/D/o/p/7/s/cli (php-7.3)]g d                                                                                                                                                                    00:34:07
diff --git i/7.3-rc/stretch/cli/Dockerfile w/7.3-rc/stretch/cli/Dockerfile
index 493a5f2..6d84fb5 100644
--- i/7.3-rc/stretch/cli/Dockerfile
+++ w/7.3-rc/stretch/cli/Dockerfile
@@ -17,20 +17,18 @@ RUN set -eux; \
 
 RUN    set -eux; \
        { \
-               echo 'deb http://deb.debian.org/debian buster main'; \
-               echo 'deb http://deb.debian.org/debian buster-updates main'; \
-               echo 'deb http://security.debian.org buster/updates main'; \
+               echo 'deb http://deb.debian.org/debian sid main'; \
        } >> /etc/apt/sources.list; \
        { \
                echo 'Package: *'; \
-               echo 'Pin: release n=buster'; \
+               echo 'Pin: release n=sid'; \
                echo 'Pin-Priority: 100'; \
-       } > /etc/apt/preferences.d/no-buster; \
+       } > /etc/apt/preferences.d/no-sid; \
        { \
-               echo 'Package: libargon2-0*'; \
-               echo 'Pin: release n=buster'; \
+               echo 'Package: libargon2*'; \
+               echo 'Pin: release n=sid'; \
                echo 'Pin-Priority: 990'; \
-       } > /etc/apt/preferences.d/argon-buster
+       } > /etc/apt/preferences.d/argon-sid
 
 # dependencies required for running "phpize"
 # (see persistent deps below)
@@ -122,7 +120,7 @@ RUN set -eux; \
        savedAptMark="$(apt-mark showmanual)"; \
        apt-get update; \
        apt-get install -y --no-install-recommends \
-               libargon2-0-dev \
+               libargon2-dev \
                libcurl4-openssl-dev \
                libedit-dev \
                libsodium-dev \

@TimWolla
Copy link
Contributor Author

TimWolla commented Aug 5, 2018

My suggestion thus would be: Pin libargon to sid instead of buster.

I did this, as the migration from sid to buster already happened and the build broke.

This PR is updated to the current PHP 7.3 Beta 1.

@tianon
Copy link
Member

tianon commented Aug 10, 2018

I'm definitely not thrilled about pulling from sid, and even pulling from buster causes headaches for us a lot of the time (in other images where we have to do similar things). I'm starting to wonder if it makes sense for us to just compile Argon2 from source all the time and avoid all this headache.

Also, as @yosifkit mentioned above, it's kind of odd that the templating bits for the Argon2 section aren't consistent with the other templating this repo is using.

Would you rather we take over from here and just get this across the finish line?

@TimWolla
Copy link
Contributor Author

TimWolla commented Aug 10, 2018 via email

@tianon tianon requested a review from yosifkit August 13, 2018 22:13
@yosifkit yosifkit merged commit 43b47cc into docker-library:master Aug 14, 2018
@TimWolla TimWolla deleted the php-7.3 branch August 14, 2018 21:26
@TimWolla
Copy link
Contributor Author

🙌

@renatomefi
Copy link
Contributor

That's awesome! What's the final tag? I'd guess php:7.3-rc-cli-alpine3.8, is that so?
When can it be pushed? the last push in the repo was 25 days ago!

Thanks a lot for this great work!

@tianon
Copy link
Member

tianon commented Aug 15, 2018

tianon added a commit to infosiftr/stackbrew that referenced this pull request Aug 15, 2018
- `docker`: 18.06.1-ce-rc2, `alpine:3.8` (docker-library/docker#124)
- `mariadb`: 10.3.9
- `mongo`: 4.1.2
- `php`: 7.3-rc (docker-library/php#677), 7.2/alpine3.8 (docker-library/php#688)
TimWolla added a commit to TimWolla/official-images that referenced this pull request Aug 15, 2018
This test verifies that Argon2 actually works for PHP images where it should.
This is to ensure that the Argon2 from Buster does not magically change to
become incompatible.

see: docker-library/php#677 (comment)
tianon added a commit to infosiftr/stackbrew that referenced this pull request Aug 21, 2018
- `drupal`: 8.6.0-rc1
- `ghost`: ghost-cli 1.9.1
- `memcached`: Alpine 3.8 (docker-library/memcached#40)
- `mongo`: 3.6.7, `dbPath` fixes (docker-library/mongo#294)
- `openjdk`: 11-ea+27 (Debian)
- `percona`: 5.5.61, 5.6.41
- `php`: 7.1.21, Alpine 3.8 (docker-library/php#702, docker-library/php#688), 7.3.0beta2 (docker-library/php#677), 7.2.9
- `postgres`: fix `HOME` (docker-library/postgres#481)
- `python`: `apk add --no-cache` to combat new `apk` behavior (docker-library/python#330)
- `ruby`: bundler 1.16.4
- `tomcat`: 8.5.33, 9.0.11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants