flow-development-collection icon indicating copy to clipboard operation
flow-development-collection copied to clipboard

TASK: Separate Booting\Scripts and CLI subrequest calls

Open kitsunet opened this issue 1 year ago • 4 comments

This introduces Neos\Flow\Core\PhpCliCommandHandler to run CLI subrequests instead of using \Neos\Flow\Core\Booting\Scripts as that has grown way to big over the years and this single responsibility can be cleanly separated into a different class.

Some minimal cleanup was done along the way.

The old @api methods still exist in Scripts as deprecated shims so this should not be breaking for userland code.

kitsunet avatar Jun 23 '24 11:06 kitsunet

I am not quite sure what phpstan is telling me, but yeah, I think this makes it prettier...

kitsunet avatar Jun 23 '24 19:06 kitsunet

Nice :D Looks already better :D Have you read my thoughts over at https://github.com/neos/flow-development-collection/issues/3112 ?

Like i agree that the scripts utility is ugly but when we introduce a new api we should be sure it can last another 10 years or so :D For example, if we prefer to have rather a instance instead of static utility, the time would be now (the static $builtPhpCommand cache introduced with https://github.com/neos/flow-development-collection/pull/3119 was not meant to live that way forever, and i hoped to be able to cleanup this mess at some point :D) Also considerations to adapt symfony/process should be brought up now or never again :D (we have it installed already ... so that would not be the point :D)

composer/composer 2.6.6   requires  symfony/process (^5.4 || ^6.0 || ^7)          
symfony/console   v6.4.3  conflicts symfony/process (<5.4) 

mhsdesign avatar Jun 23 '24 19:06 mhsdesign

Yeah, it's really tricky, I am not fond of the static thing, and I guess we could make this an instance but then the cache would be totally meaningless (unless we create this super early as singleton scope and the bootstrap/objectmanager carries it around). I think the cache is a good idea because figuring all this out is expensive, in an ideal world we would even compile that into a class once for production because it shouldn't change for that same system. Anyways I am not super sure how well it works as "you have to inject this for subrequest", but then it shouldn't be a mainstream usecase anyways? So I guess going down that route would at least give us options for later, because then we can still refactor it internally to process if we want to....

kitsunet avatar Jun 23 '24 20:06 kitsunet

I think compiling this builtPhpCommand once into a cache or php class is only kindof a good idea. Currently it does different checks if triggered from cli or web and these integrity checks do seem like a good idea to avoid absolute chaos :D Carrying it around as singleton might be indeed cleaner? I mean flow was always about di and objects ... never so much (luckily) about dirty static utilities. Using symfony would provide fancy apis on top but we probably wouldnt want to expose that? And if we dont we dont need it 😂 ... i figured out that our async thing for example is more reliable than the symphony thing as this will just straight die if the main process shuts down. But lets chat about this soon?

mhsdesign avatar Jun 23 '24 20:06 mhsdesign