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

Opis Closure 4.x is available for testing #59

Closed
msarca opened this issue Jun 9, 2020 · 21 comments
Closed

Opis Closure 4.x is available for testing #59

msarca opened this issue Jun 9, 2020 · 21 comments

Comments

@msarca
Copy link
Member

msarca commented Jun 9, 2020

Hi, everyone!

I'm happy to announce that we are about to launch a new version of Opis Closure.

This version targets PHP +7.4 and use FFI to make closures truly serializable, without wrapping them inside a SerializableClosure object. This means that you no longer need to adapt your library's logic in order to have serializable closures. One line of code and closures will become serializable.

Please give it a try and let us know if you have encountered any issues while testing.

@xepozz
Copy link

xepozz commented Jun 10, 2020

Version 4.0 does not work:
Serialization Closure is broken. $reflection->getCode() returns <?php and namespace ... sometimes

You can try to fix your bugs with running test in yiisoft/composer-config-plugin#82
Run tests: ./vendor/bin/phpunit --debug
See generated files in tests/Integration/Environment/vendor/yiisoft/composer-config-plugin-output/ (params.php or web.php).

@msarca
Copy link
Member Author

msarca commented Jun 10, 2020

@xepozz If you believe you have discovered a bug, please open a new issue so we could treat it properly. Also, you must be aware that Opis Closure 4.x is incompatible with 3.x and generates the code differently than you might expect. Our goal is to make closures serializable. If you are using this library for different purposes, than it might not be the most appropriate tool.

@msarca
Copy link
Member Author

msarca commented Jun 10, 2020

Just to be clear for everyone reading this thread: our public API consist of a single function Opis\Closure\init. Everything else is internal. Please use this library strictly for closure serialization/unserialization. If you want to analyse a closure's source code nikic/php-parser will help you do a better job.

@xepozz
Copy link

xepozz commented Jun 11, 2020

@xepozz If you believe you have discovered a bug, please open a new issue so we could treat it properly. Also, you must be aware that Opis Closure 4.x is incompatible with 3.x and generates the code differently than you might expect. Our goal is to make closures serializable. If you are using this library for different purposes, than it might not be the most appropriate tool.

We are using this package for serializing/unserializing standalone closures and closures in a properties.

@msarca
Copy link
Member Author

msarca commented Jun 11, 2020

@xepozz Please open a new issue and provide the most basic example that can be reproduced. Thanks.

@msarca
Copy link
Member Author

msarca commented Jun 11, 2020

@xepozz I did take a look at your test results and the first thing that struck me was this: Error: Class 'Environment\Serializer\CustomSerializer' not found. Incidentally that is the class where closure serialization was supposed to be happen.

@xepozz
Copy link

xepozz commented Jun 14, 2020

@xepozz I did take a look at your test results and the first thing that struck me was this: Error: Class 'Environment\Serializer\CustomSerializer' not found. Incidentally that is the class where closure serialization was supposed to be happen.

I've fixed it

@sorinsarca
Copy link
Member

We added PHP 8 support

@sorinsarca sorinsarca mentioned this issue Jul 2, 2020
@taylorotwell
Copy link
Contributor

taylorotwell commented Jul 6, 2020

A huge feature of Laravel is queued Closures with proper ORM model serialization / deserialization when the models are part of the use portion of the Closure. We accomplished this in previous versions of Opis by overwriting methods that I had PR'd to Opis...

My original PR to Opis: #21

Our overwritten methods can be seen here: https://github.com/illuminate/queue/blob/master/SerializableClosure.php

Can this still be accomplished in 4.x? Again, this is a very large and widely used feature of Laravel so if it's not planned to be supported for 4.x we will likely be forced to fork 3.x and maintain that version ourselves unfortunately. 😬

@sorinsarca
Copy link
Member

sorinsarca commented Jul 7, 2020

Can this still be accomplished in 4.x?

Not at this time, but a similar mechanism can be added in 5 minutes.

However, in 4.x there are no wrappers, meaning that transformUseVariables/resolveUseVariables handlers will apply globally.

Also, since we removed the wrappers you can view a closure as other data type, like an array (from serialization/unserialization perspective), so this should work:

$a = function () use ($laravalModel, $laravelCollection) {};
$b = [$laravelModel, $laravelCollection]; //  array serialization is handled by PHP, so you don't have transform/resolve handlers

unserialize(serialize($a)); // works because we have transform/resolve handlers
unserialize(serialize($b)); // should also work. 

If it doesn't work properly then the problem is not related to opis/closure (in my opinion, @msarca what do you think?).

Again, this is a very large and widely used feature of Laravel so if it's not planned to be supported for 4.x we will likely be forced to fork 3.x and maintain that version ourselves unfortunately.

We just added @GrahamCampbell to opis/closure team to help maintain version 3.x for laravel as long as necessary (see #63).

@taylorotwell
Copy link
Contributor

It would not be a significant problem for us for those handlers to apply globally I don't think.

@mnapoli
Copy link

mnapoli commented Jul 5, 2021

Hey, PHP-DI maintainer here 👋 We're using opis/closure too, and I'm worried about the usage of FFI: AFAIK that extension isn't installed by default, is that correct?

@taylorotwell is the FFI requirement an issue with Laravel?

@msarca
Copy link
Member Author

msarca commented Jul 5, 2021

@mnapoli We are also dependant on ext-tokenizer #92, and nobody ever complained 😅. The FFI extension is a bundled extension, just like ext-tokenizer, ext-pdo, or ext-mbstring, and I don't think it will be an issue.

@SvenRtbg
Copy link

SvenRtbg commented Jul 6, 2021

The PHP dpcumentation is still warning that the FFI extension is experimental. Using the extension requires the "libffi" library to be installed, and PHP has to be compiled using "--with-ffi".

I have my doubts that distributions will do that by default, and would consider relying on that extension to be risky.

I checked two installations I use: Ubuntu seems to have FFI enabled by default (I did not remember installing it explicitly), but Suse Linux Enterprise does not - as this is used in my production system, I'd have to investigate what is offered as optional extension there.

@SvenRtbg
Copy link

SvenRtbg commented Jul 9, 2021

I got feedback from my ops team: Suse Linux is not yet providing the FFI extension. If we'd ask them to provide it, it would go only into their latest release, which is SLES15SP3.

@thorewi
Copy link

thorewi commented Jan 21, 2022

Hi,

I guess version 4.x doesn't work with PHP 8.1, at least for me.

Dockerfile:

FROM php:8.1-fpm
RUN docker-php-source extract \
&& apt-get update \
&& apt-get install libffi-dev && docker-php-ext-configure ffi --with-ffi && docker-php-ext-install ffi \
&& docker-php-source delete

php.ini:

extension=ffi.so
ffi.enable = true

test.php

<?php

declare(strict_types=1);

require __DIR__ . '/../vendor/autoload.php';

\Opis\Closure\Library::init();

$f = fn() => 'Hello';
$data = serialize($f);
$g = unserialize($data);
echo $g(); //> Hello

Output:

tomaskudelka@tomas-air sandbox % php test.php

Fatal error: Uncaught Exception: Serialization of 'Closure' is not allowed in /var/www/html/www/index.php:11
Stack trace:
#0 /var/www/html/www/index.php(11): serialize(Object(Closure))
#1 {main}
  thrown in /var/www/html/www/index.php on line 11

With PHP 8.0 and same configuration everything works fine.

@opis opis deleted a comment from driesvints Jan 21, 2022
@msarca
Copy link
Member Author

msarca commented Jan 21, 2022

@driesvints removed your comment for being off topic

@driesvints
Copy link

@msarca sure, it's your privilege to maintain this repo how you prefer. Was just trying to help @thorewi

@thorewi
Copy link

thorewi commented Jan 21, 2022

@driesvints yes, thank you, I know the another library...

@msarca
Copy link
Member Author

msarca commented Jan 21, 2022

@thorewi will take a look and try to fix it. @driesvints I appreciate your understanding.

@sorinsarca
Copy link
Member

Version 4.0 was released without ffi dependency. It is a full rewrite of the library to support latest PHP 8.x features.

@opis opis deleted a comment from frodeborli Jan 7, 2025
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

8 participants