feat(embed): add embed features, add test example which run php inside it

[joelwurtz]

This add a new features "embed" which links result with the libphp.so library available if php-embed is installed

This should allow in the future to test code directly in the wanted file, there is still a lot to do but this is a small PoC so we can iterate on that and have a better test suite

[ptondereau]

Amazing!

This PHP module should be compiled with the ./configure --enable-embed but the CI is currently using a pre-compiled version of PHP from: https://github.com/shivammathur/setup-php,
that why the CI is failing here

[joelwurtz]

Not sure if enable embed is needed as it's only to build the libphp.so file from what i understand.

And yes the CI is failing because it needs this shared object ant it's not available on https://github.com/shivammathur/setup-php,

I will do a PR on this repo to add php-embed, in the meantime we can avoid --all-features and run embed test with test on the Dockerfile ?

[ptondereau]

Not sure if enable embed is needed as it's only to build the libphp.so file from what i understand.

And yes the CI is failing because it needs this shared object ant it's not available on https://github.com/shivammathur/setup-php,

I will do a PR on this repo to add php-embed, in the meantime we can avoid --all-features and run embed test with test on the Dockerfile ?

Sounds good to me, but I'm curious that if the embed binding is included in the wrapper, does the feature flags really work here? I mean, for example, I don't want to build extra bindings that are only used in the test even if I've disabled the embed flag. Can you take a look?

[joelwurtz]

Not sure i follow, from what i understand :

• You use embed if you want to create a binary that use php behind (like a new sapi, or testing this crate)
• You don't use embed if you want to compile / build an extension

I assume if you enable embed and use php_module it will not work / it would be dangerous as you would run php with a lib that use bindings from libphp (not sure which function the module will use, one from php ? one from libphp ?)

Also didn't know that test were run in parallel inside rust, which means i have to force thread count to 1 when testing (or have php embed compiled with zts)

[ptondereau]

IMHO, I don't want a developer who is building a rust extension with this lib, to have libphp.so to build its extension (even in the CI).

My first guest would be to create a new crate in the workspace with the required bindings + libphp with only test cases. But I'm not very confident and let, maybe @danog has a word on it.

It would be dangerous as you would run php with a lib that use bindings from libphp (not sure which function the module will use, one from php ? one from libphp ?)

I think it will use libphp only when you wrap the code between php_embed_init/php_embed_shutdown

[joelwurtz]

IMHO, I don't want a developer who is building a rust extension with this lib, to have libphp.so to build its extension (even in the CI).

This will not be linked unless you have the "embed" feature specified

You could always make this feature required when you run test, i don't think there is value with a new crate when it can be a feature ? But if there was a new crate i would split this one so we could have :

• php-sys -> bindings + safe implementation for php (with the embed feature adding bindings and safe implementation for the required functions)
• php-bin -> require php-sys with embed feature to build custom sapi or executing php inside a rust binary
• php-ext -> require php-sys, to build extension with the macro system

cargo install php would then have a deps on php-bin and would allow to generate stubs / execute test (with cargo php test [file.phpt]) but it would still require the embed library at some point

[ptondereau]

This will not be linked unless you have the "embed" feature specified

It's ok for me then, lezgo!

About the crate splitting, let's wait and discuss it in a separate issue, but TBH, I would love to split bindings + safe wrappers from the macro. The problem is: There is an initial refactor of the macro system that needs to be done (see: #99)

[joelwurtz]

I updated the CI so the embed will be build on a specific workflow (as it's not available with setup-php)

I also added a guard around the embed as it's not thread safe (in nts), but thread safe version can be added if it's available, i'm not sure where this PR should stop, i think it should bring the basic setup for test and rest could be added in new PRs.

[ptondereau]

LGTM! I think we have a good first iteration here

[joelwurtz]

I dont think it's ready yet, i'm rewriting the logic for the embed as it's initialized only once, which means php code in some test can pollute other test

like a test setting a variable $foo, another test may use this variable latter which will be already defined

I'm actually rewriting the API so someone will do the following :

Embed::run_fn(|| {
    let result = Embed::eval("'foo';");

    assert!(result.is_ok());

    let zval = result.as_ref().unwrap();

    assert!(zval.is_string());
    assert_eq!(zval.string().unwrap(), "foo");
});

And this function will ensure that it's correctly initialized / shutdown at each run

[joelwurtz]

Ready for last review

[ptondereau]

Thank you for the contribution!