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

PHP 8 support of php extension #456

Closed
BigMichi1 opened this issue Apr 9, 2021 · 41 comments
Closed

PHP 8 support of php extension #456

BigMichi1 opened this issue Apr 9, 2021 · 41 comments

Comments

@BigMichi1
Copy link

Expected behaviour

the php extension that is needed should also be available for PHP 8

Actual behaviour

we are using https://launchpad.net/~ondrej/+archive/ubuntu/php to have a recent up-to-date version of PHP but currently the pdlib extension is not available for PHP 8

Steps to reproduce

  1. use php-8 to serve nextcloud
  2. pdlib extension is not available

Server configuration

  • Operating system:
    Ubuntu 20.04.2 LTS

  • Pdlib version:

  • How is DLib installed: Make sure it is working correctly with this tool

  • How is PDlib installed: Make sure it is working correctly with this tool

  • PHP version:
    8.0.3-1.ubuntu20.04.1+deb.sury.org+1

  • Web server:
    Nginx

  • Database:
    PostgreSql

  • Nextcloud version:
    21.0.1

@matiasdelellis
Copy link
Owner

Hi @BigMichi1

pdlib supports php 8 since 1.0.1, but you have to compile it on your own, because I can't make packages for custom repositories.

However, I clarify that it is not yet officially supported by the application. Most likely it will work, but it is not supported yet. See #444

@hunterzero99
Copy link

I'd like to expand this bug report to include errors that seem to be associated with php8. I get the following when trying to run the scan:

Faces found: 0. Image will be skipped because of the following error: Error during image resize

I see mention of this in other bug reports that have been closed out (#429 for example).

Is there any way I can get more debug logging out of the run to see where in the resize process things fail?

@gchmurka123
Copy link

I have the same problem, I am unable to debug why I have the same error:
Faces found: 0. Image will be skipped because of the following error: Error during image resize

@Kitt3120
Copy link

Kitt3120 commented Aug 29, 2021

Same problem. Using dlib-cuda 19.22-1 and php-pdlib 1.0.2-4 from the AUR with Nextcloud 22.1.1 and php 8.0.10.

Processing image /mnt/Media3/NextcloudData/username/files/Kamera Upload/Pixel 3 XL/2021/08/PXL_20210810_121912205.MP.jpg
        Faces found: 0. Image will be skipped because of the following error: Error during image resize
        RuntimeException: Error during image resize in /var/lib/nextcloud/apps/facerecognition/lib/Helper/TempImage.php:153
Stack trace:
#0 /var/lib/nextcloud/apps/facerecognition/lib/Helper/TempImage.php(125): OCA\FaceRecognition\Helper\TempImage->resizeImage()
#1 /var/lib/nextcloud/apps/facerecognition/lib/Helper/TempImage.php(71): OCA\FaceRecognition\Helper\TempImage->prepareImage()
#2 /var/lib/nextcloud/apps/facerecognition/lib/BackgroundJob/Tasks/ImageProcessingTask.php(203): OCA\FaceRecognition\Helper\TempImage->__construct()
#3 /var/lib/nextcloud/apps/facerecognition/lib/BackgroundJob/Tasks/ImageProcessingTask.php(123): OCA\FaceRecognition\BackgroundJob\Tasks\ImageProcessingTask->getTempImage()
#4 /var/lib/nextcloud/apps/facerecognition/lib/BackgroundJob/BackgroundService.php(120): OCA\FaceRecognition\BackgroundJob\Tasks\ImageProcessingTask->execute()
#5 /var/lib/nextcloud/apps/facerecognition/lib/Command/BackgroundCommand.php(138): OCA\FaceRecognition\BackgroundJob\BackgroundService->execute()
#6 /usr/share/webapps/nextcloud/3rdparty/symfony/console/Command/Command.php(255): OCA\FaceRecognition\Command\BackgroundCommand->execute()
#7 /usr/share/webapps/nextcloud/3rdparty/symfony/console/Application.php(1009): Symfony\Component\Console\Command\Command->run()
#8 /usr/share/webapps/nextcloud/3rdparty/symfony/console/Application.php(273): Symfony\Component\Console\Application->doRunCommand()
#9 /usr/share/webapps/nextcloud/3rdparty/symfony/console/Application.php(149): Symfony\Component\Console\Application->doRun()
#10 /usr/share/webapps/nextcloud/lib/private/Console/Application.php(209): Symfony\Component\Console\Application->run()
#11 /usr/share/webapps/nextcloud/console.php(99): OC\Console\Application->run()
#12 /usr/share/webapps/nextcloud/occ(11): require_once('/usr/share/weba...')
#13 {main}
yielding

@matiasdelellis
Copy link
Owner

Hi everyone,

As you can see the nextcloud folks are deliberately ignoring PHP 8 tests..

Our application makes intensive use of the OC_Image class, that is not deprecated, far from it, but it seems to fail, and they just don't want to fix it.

Hope to fix it soon, but just I don't recommend PHP 8 yet.. 😞

@matiasdelellis
Copy link
Owner

Need help..
https://github.com/nextcloud/server/blob/0447b53bda9fe95ea0cbed765aa332584605d652/lib/private/legacy/OC_Image.php#L918-L944

You have to see more info in the nextcloud logger. Please comment what it says ..

@guystreeter
Copy link
Contributor

I don't see any nextcloud log messages associated with the attempt to run the background task.
Can you tell us more about how to get the logs you're looking for?

@guystreeter
Copy link
Contributor

I put a lot of logging statements in preciseResizeNew, including this one right before the return:

                if ($process === false) {
                        $this->logger->error('resize: process is false', ['app'  => 'core']);
                } else {
                        $this->logger->error('resize: ' . get_class($process), ['app'  => 'core']);
                }

It always logs:

{"reqId":"l8gbSJ2I4S7QP7W21tZn","level":3,"time":"2021-10-25T21:03:20+00:00","remoteAddr":"","user":"--","app":"core","method":"","url":"--","message":"resize: GdImage","userAgent":"--","version":"22.2.0.2"}

So it looks like preciseResizeNew is not returning false

@guystreeter
Copy link
Contributor

I added this after the check for false:

                if (is_resource($process) === false) {
                        $this->logger->error(__METHOD__ . '() is_resource is false', ['app'  => 'core']);
                }

And it results in

{"reqId":"LnNjBvI6WYUbHLJFgr45","level":3,"time":"2021-10-25T21:12:54+00:00","remoteAddr":"","user":"--","app":"core","method":"","url":"--","message":"OC_Image::preciseResizeNew() is_resource is false","userAgent":"--","version":"22.2.0.2"}

This may be the limit of my understanding of this code. I'll be happy to help if you have more suggestions.

@guystreeter
Copy link
Contributor

is_object($process) is true. var_export($process) shows GdImage::__set_state(array(\n))

@guystreeter
Copy link
Contributor

This article https://php.watch/articles/resource-object talks about the PHP 8 transition from resources to objects. It suggests changing is_resource($x) to $x !== false as a backward-compatible check for successful object creation. Making this change everywhere in OC_Image.php got rid of the error message, but the face thumbnails show up as "broken image" icons in the UI.

@guystreeter
Copy link
Contributor

I apparently had some local problem that was causing the image display issues. They don't happen in a fresh install.
With the fixes to OC_Image.php I mentioned previously. the background job runs successfully and clustering works as expected.
The only problem I have now is that in the sidebar Persons tab, if there is a recognized person in the photo, I get

Error: Request failed with status code 500

instead of the info about the person.
I have no idea how to start debugging that.

@matiasdelellis
Copy link
Owner

Hi @guystreeter
Thank you very much for taking this job!. 😄

Error 500 is an internal error.. Probably in on FaceController.php, and you have to see something in the nextcloud log.. 😬

@matiasdelellis
Copy link
Owner

Hi @guystreeter
Test:

[matias@nube Db]$ git diff .
diff --git a/lib/Db/Face.php b/lib/Db/Face.php
index bbe373c..0699439 100644
--- a/lib/Db/Face.php
+++ b/lib/Db/Face.php
@@ -197,7 +197,7 @@ class Face extends Entity implements JsonSerializable {
                $this->markFieldUpdated('descriptor');
        }
 
-       public function setCreationTime(\DateTime $creationTime): void {
+       public function setCreationTime($creationTime): void {
                if (is_a($creationTime, 'DateTime')) {
                        $this->creationTime = $creationTime;
                } else {
[matias@nube Db]$ 

@guystreeter
Copy link
Contributor

I have created issue nextcloud/server#29466 against Nextcloud server to get OC_image.php fixed.

@guystreeter
Copy link
Contributor

@matiasdelellis Your suggested change worked. I can see the identified people in the Persons tab now.

@guystreeter
Copy link
Contributor

This PR waiting approval will fix OC_Image.php: nextcloud/server#29479

@gchmurka123
Copy link

Thanks, great job, I patched my Nextclouid 22.X version by hand and it finally worked on php 8.x

I hope they add a patch to last stable v22 (they described it as php8 compatible) ;)

@jakobroehrl
Copy link

Hi @gchmurka123,
how can I install pdlib for PHP8

@matiasdelellis
Copy link
Owner

Hi everyone,
Just comment that thanks to the investigation of @guystreeter in the next version of Nextcloud we will have support for php8 in this application.

Thanks you so much!! 😃 🎉

@Kitt3120
Copy link

Kitt3120 commented Nov 2, 2021

Nice! Have been waiting for this since php8 came out. Tried the Face Recognition app occasionally to check if it has been fixed, but eventually just found this GitHub issue and watched it. Looking forward to it, thanks to all people involved :)

@guystreeter
Copy link
Contributor

The necessary fixes have been pulled in the master on github today.
nextcloud/server#29479

@martadinata666
Copy link

martadinata666 commented Nov 3, 2021

rebuild my php-fpm to experiment this, but seems another wait for nextcloud release update, is the zip release latest-22.zip, will do?

@guystreeter
Copy link
Contributor

I was mistaken. Only part of the patch-set was pulled in the master branch. It does not include the fixes we need. :(

@FernandoMarques-Santos
Copy link

Hi everyone, Just comment that thanks to the investigation of @guystreeter in the next version of Nextcloud we will have support for php8 in this application.

Thanks you so much!! 😃 🎉

Thank you Matias and all! After the nextcloud update, the icing on the cake would be an update to the precompiled PDLib PHP libraries for PHP 8.

@BigMichi1
Copy link
Author

Maybe upvoting oerdnj/deb.sury.org#1663

@EWouters
Copy link

EWouters commented Nov 14, 2021

The Nextcloud update is now live in version 22.2.1! (nextcloud/server#29519)

@martadinata666
Copy link

martadinata666 commented Nov 14, 2021

Is this compatible after all with nextcloud 22? i got this error on enabling, #500 (comment)

Browse around got #495

/var/www/html $ ./occ app:enable facerecognition
App "Face Recognition" cannot be installed because the following dependencies are not fulfilled: PHP with a version lower than 7.4 is required.

@jakobroehrl

This comment has been minimized.

@EWouters
Copy link

Can't activate the app with PHP8 an NC 22.2.2. or NC 23 RC2

This is off-topic, but you need to follow Hard way.

@Kitt3120
Copy link

Kitt3120 commented Nov 15, 2021

Can confirm that it works!
Am on arch with Linux Kernel 5.15.2, Nextcloud 22.2.2-1 and Facerecognition v0.7.2, using the aur/dlib-sse-cuda and aur/php-pdlib packages.

@matiasdelellis
Copy link
Owner

Hi everyone,
Please, wait for Nextcloud 22.2.2 (NOT 22.2.1... see nextcloud/server#29678 and https://help.nextcloud.com/t/nc-22-2-1-performance-warning/127169/1), and should test against Facerecognition from git master ...

@guystreeter
Copy link
Contributor

I pulled NC 22.2.2 and built facerecognition from git master, and it seems to be working just fine!

@jandr
Copy link

jandr commented Nov 17, 2021

This works fine also in NC 22.2.3 by installing manually facerecognition from git. Hopefully the app from the NC repository will be updated soon to not necessarily require PHP <= 7.4, and will support PHP 8 as well!

@matiasdelellis
Copy link
Owner

Great. Thank you all very much!. 🎉 😄
Probably tomorrow I will upload an update enabling php 8.. 😬

matiasdelellis added a commit that referenced this issue Nov 20, 2021
- Initial support for php 8. See issue #456
- Add link to show photos of person on sidebar.
- Add static analysis, phpunit and lintian test using github workflow.
- Add an real OCS public API to get all persons. See PR  #512.

- Fix sidebar view when user has disable it.
- Set the Image Area slider to the maximum allowed by the model. See issue #527
- Don't try to force the setCreationTime argument to be DateTime. See PR #526
- Migrate hooks to OCP event listeners. See PR #511

- New Czech translation thanks to Pavel Borecki, and update others. Thank you so
  much everyone.
@matiasdelellis
Copy link
Owner

Just published on Nextcloud appstrore.. 😄 🎉

Please update from there and report back here.. 😉

@FernandoMarques-Santos
Copy link

Ubuntu 20.04, Nextcloud 22.2.3, PHP8.0, I had compiled pdlib to support PHP 8. Today installed the Facerecognition 0.8.5 directly from gitmaster and I confirm that everything is working well. You guys made an excellent job.

However, I got a bit confused by this thread. Are we still needing to compile the pdlib part (see the OP problem)? Or with this facerecognition 0.8.5 (from the app store) we don't need to do that anymore? Sorry I don't see how the update relates to the original problem.

Marginally related to this, are you guys using some of the apps for android? nkming version can show the faces when the facerecognition is pulled directly from the master. It is working pretty well.... butI just wished to not having to compile anything =)

@guystreeter
Copy link
Contributor

I modified by build to pull the latest Nextcloud docker image and only build dlib and pdlib. Then I downloaded and enabled facerecognition from the app store. Everything seems to be working fine.

@matiasdelellis
Copy link
Owner

Hi @FernandoMarques-Santos

The issue started as a problem that there was no pdlib for php8, but it mutated to the application support for php8 since that was the real problem.

Now Nextcloud added the necessary patches that make this application work with PHP8 and then we enable their support in last release. 😉

About pdlib, except Ubuntu Impish and Fedora 35 no distribution uses php 8 by default. This implies that practically everyone is using third-party repositories to update php, and is simply impossible for me to generate the necessary packages for third party repositories.

So, unfortunately, if you use third-party repositories, you have to compile pdlib on your own.

@matiasdelellis
Copy link
Owner

Just update package for fedora 35 with php8..

Tomorrow I will probably do the package for Ubuntu Impish.. 😉

@PrivatePuffin
Copy link

So, unfortunately, if you use third-party repositories, you have to compile pdlib on your own.

Calling the official Nextcloud Docker images out for using "third-party repositories" is pretty weird imho

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

No branches or pull requests