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 Alpine variant #259

Merged
merged 13 commits into from
Mar 1, 2018
Merged

Add Alpine variant #259

merged 13 commits into from
Mar 1, 2018

Conversation

J0WI
Copy link
Contributor

@J0WI J0WI commented Feb 11, 2018

This adds a php-fpm-alpine variant with the same dependencies installed as the Debian variants, but without Cron (#253). The entrypoint.sh is not fully POSIX compatible, so there are some extra dependencies for it.

@J0WI
Copy link
Contributor Author

J0WI commented Feb 11, 2018

Only 64-bit works so far. @tianon do you know how I can build APCu on 32-bit?

bash \
coreutils \
rsync

Copy link
Member

Choose a reason for hiding this comment

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

I think you should merge the top 3 RUN commands into a single one to keep it a similar as possible to the debian one.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This way is better for build caching. See also other images in the docker-library.
I can adjust the Debian images if required.

Copy link
Member

@tilosp tilosp left a comment

Choose a reason for hiding this comment

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

We have to get this to build on other platforms than amd64.
@tianon and @yosifkit any idea?

@tilosp
Copy link
Member

tilosp commented Feb 12, 2018

we should also update generate-stackbrew-library.sh similar to docker-library/wordpress#223 because debian supports different architectures than alpine.

@tianon
Copy link
Contributor

tianon commented Feb 12, 2018

/usr/local/include/php/ext/pcre/php_pcre.h:29:18: fatal error: pcre.h: No such file or directory

Adding pcre-dev to .build-deps appears to get past that error on i386.

@tianon
Copy link
Contributor

tianon commented Feb 12, 2018

(https://ram.tianon.xyz/post/2017/12/26/dockerize-compiled-software.html is a writeup about how I usually solve this type of issue generally when Dockerizing software)

@J0WI
Copy link
Contributor Author

J0WI commented Feb 13, 2018

trigger Travis again

@J0WI J0WI closed this Feb 13, 2018
@J0WI J0WI reopened this Feb 13, 2018
@J0WI
Copy link
Contributor Author

J0WI commented Feb 13, 2018

Travis is not building for some reason.

@tianon thanks for the hint. But I'm still confused why this is working on amd64 without pcre-dev.

we should also update generate-stackbrew-library.sh similar to docker-library/wordpress#223 because debian supports different architectures than alpine.

I think this is not related to this PR. I'd like to add only Alpine images here.

@J0WI
Copy link
Contributor Author

J0WI commented Feb 13, 2018

Travis is done now. Only fetching from keyserver failed once, which is acceptable.

@tilosp
Copy link
Member

tilosp commented Feb 13, 2018

I think this is not related to this PR. I'd like to add only Alpine images here.

It is related to this pr. If we merge this now the Jenkins server will try to build the alpine variant on arm32v5 and arm32v7 which will fail because there is no base image.

@J0WI
Copy link
Contributor Author

J0WI commented Feb 14, 2018

Okay, I can do this. But the bashbrew dependency makes it a bit tricky. I didn't found a proper way to add bashbrew to Travis, but I don't want to hardcode the architectures again.

@tianon
Copy link
Contributor

tianon commented Feb 15, 2018

FWIW, up-to-date pre-built binaries of bashbrew can be found at https://doi-janky.infosiftr.net/job/bashbrew/lastSuccessfulBuild/artifact/bin/

@J0WI
Copy link
Contributor Author

J0WI commented Feb 22, 2018

@tilosp can you help me out with the bashbrew dependency on Travis?

@J0WI
Copy link
Contributor Author

J0WI commented Feb 23, 2018

@tilosp thanks for fixing it. I would like to request a review again.

@tilosp
Copy link
Member

tilosp commented Feb 27, 2018

@J0WI i added the cron.sh. This adds only 62 Bytes.

Copy link
Member

@tilosp tilosp left a comment

Choose a reason for hiding this comment

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

LGTM

@tilosp tilosp requested a review from tianon February 27, 2018 20:13
@tilosp
Copy link
Member

tilosp commented Feb 28, 2018

@tianon please review

@tilosp tilosp merged commit f0e9f27 into nextcloud:master Mar 1, 2018
@tilosp
Copy link
Member

tilosp commented Mar 1, 2018

Thanks a lot for your work!

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

Successfully merging this pull request may close these issues.

3 participants