JoliNotif
JoliNotif copied to clipboard
New notifer for Linux which use libnotify and PHP-FFI
Hello
This PR add another notifier, which use FFI and libnotify. Probably most of Linux notifier supported in this repository under the hood use this library.
This notifier has some advantages and disadvantages. Support for FFI in PHP vary between distributions. Sometimes we need to install dedicated package. In other hand can be slightly faster (we don't need to crate external process) and more portable (we don't need external application, only libnotify).
FFI allows us to call C code from PHP script. The FFI extension need to be installed to use this notifier. Also support for FFI is only in CLI SAPI, due to security concerns.
At the moment I tested this notifier with:
- Ubuntu 20
- Ubuntu 22
- Ubuntu 24
- Fedora 39
- OpenSUSE Tumbleweed
All existing notifiers use external commands and maybe this new notifier does not meet project goals. I decided to mark this PR as draft. I can finish this PR if that notifier can be included in this package, otherwise you can close this PR.
Hi @morawskim.
Thanks for your PR, it looks realy good. I will need to play a bit a with it but this is definitely something that could bin included in JoliNotif
Hi @morawskim Do you have some time to finish this PR? It would be great to support a FFI-based notifier for linux environments.
Yes, I will finish this PR. I should do this at the weekend.
Hello @pyrech
I wanted to write some additional unit tests, but FFI class is final, so we cannot create mock class. I could create some FFI Wrapper and inject this in constructor for tests, but IMHO this would not give any value. If you have some ideas, I am very open for discussion.
I checked some potential memory leaks lastly. Valgrind report some memory leak, but not in libnotify. Also after running 400 notifications I didn't see any issues with memory.
Thanks for your work so far. About the tests, I understand it's not ideal to unit test the FFI part. Instead maybe we could just trigger the notifier for real and ensure there is no exception (and skip the tests when the notifier is not available). It would be better than nothing. What do you think about it?
Thanks for suggestion @pyrech
Instead maybe we could just trigger the notifier for real and ensure there is no exception (and skip the tests when the notifier is not available).
I have considered this, but I was afraid that this will require a lots of dependencies and the image will be huge.
But now.... I dig deeper and we can install only libnotify4 package in Ubuntu without any recommends and suggestions.
Simple test:
Run container - docker run --rm -it -e PHP_EXTENSIONS=ffi -v $PWD:/usr/src/app thecodingmachine/php:8.3-v4-cli bash
In container install only libnotify - sudo apt-get update -y && sudo apt-get install -y --no-install-recommends --no-install-suggests libnotify4
And in output you will see:
After this operation, 3470 kB of additional disk space will be used.
Amazing, we don't need install desktop environment, only one library. I added test for initialize the FFI. I have also updated GitHub workflow.
Runt tests ./vendor/bin/simple-phpunit tests/Notifier/LibNotifyNotifierTest.php
I just rebased your PR in order to have PHPStan running in the CI. I also:
- added a note in the Changelog
- added a test
testSendNotificationWithAllOptionsthat really triggers FFI and libnotify in the CI (even if the notifier returns false but I think that's expected since no desktop is installed on the CI, so I skip the test in this case but it works on my computer).
It worked out of the box on my computer, so congrats for your PR.
Thanks a lot for your contribution @morawskim :pray: