search

zokradonh / kopano-docker

Unofficial Kopano Docker images for all Kopano services.
MIT License
59 stars 36 forks source link

Allow to install extra packages #440

Closed peterfromthehill closed 3 years ago

peterfromthehill commented 3 years ago

The string in line #20 is always zero, we should watch the return value of mkdir.

fbartels commented 3 years ago

Hi @peterfromthehill,

thanks for your PR. That check broke when upgrading the base to Debian Buster. There are however a few more places where the code needs to be adapted. Can you include them in your pr?

https://github.com/zokradonh/kopano-docker/blob/d514ef44cb7278210bd10854737d5d03e75a8382/kdav/start.sh#L17 https://github.com/zokradonh/kopano-docker/blob/d514ef44cb7278210bd10854737d5d03e75a8382/zpush/start.sh#L46 https://github.com/zokradonh/kopano-docker/blob/d514ef44cb7278210bd10854737d5d03e75a8382/core/start-service.sh#L35

PS: In general I would recommend to rather build a container that has the additional packages built in. An example for a Dockerfile to do this can be found at https://github.com/zokradonh/kopano-docker/blob/d514ef44cb7278210bd10854737d5d03e75a8382/webapp/Dockerfile.plugins. Including the packages directly in the container image saves you from unexpected behaviour if packages have been updated in between restarts of the container.

fbartels commented 3 years ago

@peterfromthehill can you give an indication when you could update your pr? If you are not able to do it this week I would make the relevant changes myself in your pr.

peterfromthehill commented 3 years ago

@fbartels: Done :). Sorry for the delay, i had some off days.

fbartels commented 3 years ago

@peterfromthehill thanks for your changes. And no need to be sorry, just wanted to close the pr and was therefore checking in.