[FrankenPHP] Make php-cli functionality available in embed build#21354
[FrankenPHP] Make php-cli functionality available in embed build#21354henderkes wants to merge 2 commits intophp:masterfrom
Conversation
export do_php_cli() without main() method in embed sapi
|
Forgot to add this here: php/frankenphp#1757 |
|
8.5 is the right branch for this? I would somehow aim to separate the cli things to only sapi/cli (for example, installing cli header from the sapi/embed sounds a bit strange) and sapi/embed to only embed. I'm not that familiar with the PHP C API to suggest something useful here but current changes are not that optimal yet, IMHO. Or perhaps moving these common C files (objects later on in the build phase) into the php-src/main and those being used by both sapi/cli and sapi/embed? That I think would sound a bit more logical from what is the aim to achieve here if I understand everything here correctly. |
I targeted master first, but it would be nice if we could get this into php 8.5 (or earlier, 8.3 in support window?).
I agree, but it's kind of what is happening here. Building embed now also builds cli (minus main function, which wouldn't be in the include api anyhow). That being said, the API doesn't need to install cli headers at all, including |
|
Check for example, |
|
Thanks, I'll have a look. It will definitely be a larger refactor though. |
|
Because also for another reason this would need to be refactored. These sapi/cli files assume that checks like HAVE_SETPROCTITLE, HAVE_PS_STRINGS, HAVE_SYS_PSTAT_H have been performed (for example used in the sapi/cli/ps_title.c). And without building sapi/cli they wouldn't be performed at the current build system code. |
|
In worst case scenario, it would be simplest probably to even copy&paste these CLI related files to sapi/embed and duplicate them there instead. But from this PR I see there are a lot of these CLI related files involved... And their headers then also. So, yes. I think that making some core-like part would make the most sense here (meaning going into the main directory or something). |
|
Alternatively, we could make this a part that's only available when built when --enable-cli is used. It would make the whole thing very simple at least and we wouldn't be installing cli sapi headers from the embed build. What do you think? If not, I'll move the shared logic (basically everything in cli sapi) into a shared folder and cli sapi becomes nothing more but a main function calling |
To me this would sound like different libphp.{so,a} would be produced when building with cli SAPI and without making it difficult to consume and understand why some parts sometimes are available and some aren't. No, I think that then it is better to use sapi/cli sources like in the PR. I still think providing these parts of cli sapi should be done somehow more globally like in the main directory. Does anyone else have any suggestions here? |
This is not a bugfix and as such is inappropriate everywhere except master. |
That's fair, I'll rewrite the PR on
Yes, if nobody comes up with a different idea, that's the direction I'll take. Will check out FastCGI sharing when I get to it. It's too late in the day now. |
|
I checked out how the FastCGI stuff is shared, but I don't think that's a good option here. It's not just a single file that'd be shared here, it's all the feature checks and essentially all of its handling. It would just result in moving half the |
|
Huh, sorry for (automatically) adding everyone, that wasn't my intention. I only changed base to master and will put this on hold as a reference draft for myself now. |
Background: we've long wanted to include full php-cli functionality in frankenphp, but only have a emulation of it with
frankenphp php-clithat only supports -r and direct execution of files. Getting the full functionality would be beneficial, because it would allow using frankenphp's go extensions in a cli context.This PR:
sapi/cli/php_cli.c::main()to::do_php_cli()do_php_cli(), except when compiling embed sapisapi/embed/cliinsteaddo_php_cli()insapi/cli.handsapi/php_embed.hand definesHAVE_EMBED_CLIfor convenience (could remove, just makes things simpler in frankenphpTested on Linux and Windows so far. I copied the windows build script parts from
sapi/clias best as I could, testing compilation was successful and the symbols are extern inphp8embed.lib, just likephp_embed_init/shutdown.cc @alexandre-daubois @withinboredom