-
Notifications
You must be signed in to change notification settings - Fork 271
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
Dockerfile optimization proposal #660
Comments
@benbrummer we can certainly start with the obsolete ones like zip/unzip |
related #663 I had to roll back the architecture if/fi blocks as they were preventing required dependencies from installing. |
Apart of the default-mysql-client, did you face any other issues? Most of them will be installed as dependencies of google-chrome-stable, so we could move them to arm64 section. The other ones are not clear to me.
|
The fonts are definitely needed to provide unicode support for PDF generation is regions like China. The others, if they are dependencies of chrome won't hurt to have in place I would have thought. |
Are these apt packages needed?
|
I'm not sure if any of these are subdependencies of anything |
If I compare the php list https://invoiceninja.github.io/en/self-host-installation/#server-requirements with the modules, which are installed in the image, the Additional modules in the image installed with install-php-extensions
@turbo124 can you tell me what are the required ones. E.g pdo_mysql vs mysqli? |
mysqli is fine for use,. |
I reduced it for a test to just
Everything seems to work. Even without opcache and redis it is quick and responsive. pdo_mysql is required, otherwise the database connection fails, mysqli seems to be unnecessary. All Laravel requirements are installed https://laravel.com/docs/11.x/deployment#server-requirements. Predis => https://laracasts.com/discuss/channels/laravel/what-is-difference-between-phpredis-and-predis-in-laravel => phpredis is not needed |
predis is more than sufficient here, i agree. |
@turbo124 I'm not able to find a usage for these modules. Do you have an example, where these modules are used in invoiceninja. Can I somehow produce an error somewhere? Modules, which seem to be unusedexif Required/used modules
|
Soap needed for vat number verification Pcntl, I believe is needed by Laravel |
composer suggest --all
Findings:imagick:
soup:
no match for exif, gmp, imagick, intl, mysqli, pcntl, saxon I think the dependency confusion, discrepancy of docs and composer feels like an issue, which should be handled in https://github.com/invoiceninja/invoiceninja. |
whilst there may be no composer related dependencies, the app directly engages the ones I have mentioned above. |
So than they should be listed here? https://github.com/invoiceninja/invoiceninja/blob/7b02668d4bd1de6462717a98a88e6ebea706eaa5/composer.json#L33 |
Not necessarily. The app can certainly work without some of these. We do not enforce all possible extensions. The app is installed on a wide range of services including shared hosting which can present challenges for meeting some dependencies. In environments where we can control the underlying extensions, then it makes sense to use them. |
Understand :-) Is there a documentation for all the possible extensions and their purpuose? https://invoiceninja.github.io/en/self-host-installation/#server-requirements should list pcntl: |
We can certainly add the list of non-required extensions into a suggest block. In regards to pdo_mysql, I believe this is automatically included (at least when using apt) when running apt-get install php-mysql pnctl is part of PHP core, not really an extension, my guess here is upstream they check for this flag and replace with a binary that has pnctl enabled or disabled. given pnctl's role in process control, and laravel does use SIG* commands for process controls, I would assume this is very much needed. |
@turbo124 I tried to to my best and combined all Information (composer.json, laravel docs, invoiceninja docs, the base dockerimages 8.2-8.4-fpm)
Additional modules listed in documentation https://invoiceninja.github.io/en/self-host-installation/#server-requirements, which are not already listed above
php-common will be installed anyway, when any php-* package is installed, and it is no php modul itself, so I think it is not needed to list in the docs. |
removed gmp requirement as we enforce bcmath. My Suggestion
|
@turbo124 I integrated yourt suggestion 806a340#diff-b98298742c5536fa90710aea60f865cdf0149b61c06288c4a0219ce4796ec807R3 Apart of a multistage implementation everything is implemented from this issue in #664. Multistage can be handled, if needed in a different issue. intl seems to be unnecessary |
Multistage is currently out of scope for this issue |
docker image seems to be quite big, so I try to identify ways to shrink it
Ideas
apt packages
required:
obsolete (?):
php modules
Misc
dockerfiles/debian/Dockerfile
Line 149 in a6d8f66
The text was updated successfully, but these errors were encountered: