tobybatch / kimai2

Docker containers for the kimai2 web application including docker-compose and kubernetes/helm deployment.
MIT License
183 stars 97 forks source link

Call to undefined function cal_days_in_month() #317

Closed csageder closed 2 years ago

csageder commented 2 years ago

Describe the bug I have installed the newest version of the Controlling Plugin. This Plugin uses the php functioncal_days_in_month().

To Reproduce Steps to reproduce the behaviour:

  1. Start the container
  2. Execute a shell within the container
  3. Within the shell run php -a
  4. Run echo cal_days_in_month(CAL_GREGORIAN, 8, 2003);
  5. The result is the following error message:
    PHP Warning:  Uncaught Error: Call to undefined function cal_days_in_month() in php shell code:1
    Stack trace:
    #0 {main}
    thrown in php shell code on line 1

Desktop (please complete the following information):

Command used to run the container

Additional context Function should work for php 4.1+ onwards: https://www.w3schools.com/php/func_cal_cal_days_in_month.asp

tobybatch commented 2 years ago

So it seems that PHP needs to be built with calendar support enabled.

https://stackoverflow.com/questions/49612838/call-to-undefined-function-cal-days-in-month-error-while-running-from-server

I try to avoid adding too many extensions to support third party plugins. Once I start every plugin author wants a slightly different PHP set up. :)

However... Adding calendar support to a time tracking app seems justified. I'll add it to the todo list, it's not a massive change. I'll try and get it done soon-ish.

kevinpapst commented 2 years ago

Yeah or the method is replaced by a one line solution from php.net: https://www.php.net/manual/de/function.cal-days-in-month.php#38666

function days_in_month($month, $year) { 
return $month == 2 ? ($year % 4 ? 28 : ($year % 100 ? 29 : ($year % 400 ? 28 : 29))) : (($month - 1) % 7 % 2 ? 30 : 31); 
} 

@tobybatch I never needed any of those method in over 10 years of development around Kimai...

I checked a couple of systems, they all have calendar compiled in, so it is not a very exotic extension. BUT: Kimai composer does not require it. And it is not mentioned in the Store page.

Not saying that you should not add that extension, but blindly requiring new PHP extensions in a paid extension and then relying on the community to fix that problem is not "best practice". I emailed the author.

tobybatch commented 2 years ago

@kevinpapst Yeah, I saw that too. It's being used by the Controlling Plugin. We don't build PHP for the container. If it's an option form the underlying mages then it's trivial to add, else out of scope. In that case it's a feature request for the plugin.

I'll look when I get time.

j0hannesr0th commented 2 years ago

Hi @tobybatch I'm the creator of the Controlling Plugin. How are we going to resolve this?

Red pill: I'll test the suggest function of Kevin or find an other solution Blue pill: You add support for the calendar functions of PHP

Hope you are a Matrix fan, too and get the reference 😉

csageder commented 2 years ago

@tobybatch Adding the extension seam to be straight forward. If I understand the strategy right a new extension image is created, and then only the resulting files are copied to the base image

# php extension calendar
FROM ${BASE}-php-ext-base AS php-ext-cal
RUN docker-php-ext-install -j$(nproc) calendar

And adding in the base image

# PHP extension calendar
COPY --from=php-ext-cal /usr/local/etc/php/conf.d/docker-php-ext-???.ini /usr/local/etc/php/conf.d/docker-php-ext-???.ini
COPY --from=php-ext-cal /usr/local/lib/php/extensions/no-debug-non-zts-20200930/???.so /usr/local/lib/php/extensions/no-debug-non-zts-20200930/???.so

Is this right?

tobybatch commented 2 years ago

@csageder Yes that's totally right. I just wonder if the better solution isn't to remove the dependency on a custom php compilation in the plugin, using Kevin's solution.

I don't mean to be an arse, but each extension has some size to it:

www-data@86b05252f145:/usr/local/lib/php/extensions/no-debug-non-zts-20200930$ ls -lah
total 4.2M
drwxr-xr-x 1 root root 4.0K Oct 25 09:26 .
drwxr-xr-x 1 root root 4.0K Sep  3 18:28 ..
-rwxr-xr-x 1 root root 432K Oct 25 09:26 gd.so
-rwxr-xr-x 1 root root 2.0M Oct 25 09:26 intl.so
-rwxr-xr-x 1 root root 107K Oct 25 09:26 ldap.so
-rwxr-xr-x 1 root root 1.4M Sep  3 18:28 opcache.so
-rwxr-xr-x 1 root root  41K Oct 25 09:25 pdo_mysql.so
-rwxr-xr-x 1 root root 114K Sep  3 18:28 sodium.so
-rwxr-xr-x 1 root root  45K Oct 25 09:25 xsl.so
-rwxr-xr-x 1 root root  98K Oct 25 09:25 zip.so

and in if I build this one in then the next extension that need a custom build wants that one built in too and I may just as well build all ~60 of the extensions into the image.

Let me check with the other repo maintainers.

tobybatch commented 2 years ago

You could extend our image if you need a custome ext.

Something like

# apache debian php extension base
FROM php:8.0.10-apache-buster AS apache-php-ext-base
RUN apt-get update
RUN apt-get install -y \
        libldap2-dev \ 
        libicu-dev \ 
        libpng-dev \ 
        libzip-dev \ 
        libxslt1-dev \
        libfreetype6-dev

# php extension calendar
FROM apache-php-ext-base AS php-ext-cal
RUN docker-php-ext-install -j$(nproc) calendar

# PHP extension calendar
FROM kimai/kimai2:apache
COPY --from=php-ext-cal /usr/local/etc/php/conf.d/docker-php-ext-???.ini /usr/local/etc/php/conf.d/docker-php-ext-???.ini
COPY --from=php-ext-cal /usr/local/lib/php/extensions/no-debug-non-zts-20200930/???.so /usr/local/lib/php/extensions/no-debug-non-zts-20200930/???.so

This isn't tested but should work.

tobybatch commented 2 years ago

@Schrolli91 @jokay Opinions please?

root@889cddf916e0:/var/www/html# ls -lah /usr/local/lib/php/extensions/no-debug-non-zts-20200930/
...
-rwxr-xr-x 1 root root  41K Dec  8 09:20 calendar.so
...

Pro:

Con:

Schrolli91 commented 2 years ago

Users with special needs should create their own images - they can use this repo as base. Otherwise we have to ship the image with ALL possible extension.

IMHO

jokay commented 2 years ago

@tobybatch I'm sorry but I have very limited time and can't support the Docker image developmenet for kimai2 anymore.

For this case, I would say the same as @Schrolli91.

or

Schrolli91 commented 2 years ago

@tobybatch Maybe we can add a section in the docs - "how to build a custom image" - like your proposal above.

stale[bot] commented 2 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

dhoernel commented 1 year ago

The problem persists with the latest stable release: Kimai version 1.30.11 Controlling plugin version 1.23

Is there any step by step guide to resolve it or (better) a Docker image that fixes the problem?

tobybatch commented 1 year ago

Can you open a new ticket please with all the relevant info, docker command, files, logs etc. This is a closed issue.