-
Notifications
You must be signed in to change notification settings - Fork 403
delete source/downloads after building in script, not in dockerfile #2000
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
Conversation
| GITHUB_TOKEN=$(cat /run/secrets/github-token) ./build-static.sh && \ | ||
| if [ -n "${BUILD_PACKAGES}" ]; then \ | ||
| ./build-packages.sh; \ | ||
| fi; \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
getting rid of this entirely all together in another PR
|
|
||
| WORKDIR /usr/local/src/php | ||
| RUN git clone --branch=PHP-8.4 https://github.com/php/php-src.git . && \ | ||
| # --enable-embed is only necessary to generate libphp.so, we don't use this SAPI directly |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why removing this comment?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because we use libphp.so embed SAPI directly and therefore directly rely on --enable-embed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comment is still relevant. We don't use the embed SAPI, but we need to use this flag to build because it builds libphp as a side effect
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| # --enable-embed is only necessary to generate libphp.so, we don't use this SAPI directly | |
| # --enable-embed is necessary to generate libphp.so, but we don't use the SAPI directly |
.editorconfig
Outdated
|
|
||
| [*.sh] | ||
| indent_style = tab | ||
| tab_width = 2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not 4 as for Dockerfiles?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good question, I'm happy to change it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've no strong opinion on this but maybe should we use the same value everywhere for consistency?
edit: also adding a .editorconfig, my ides keeps turning tabs into spaces