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

Compatibility with openswoole 22 #110

Open
acelaya opened this issue Jan 22, 2023 · 10 comments · May be fixed by #128
Open

Compatibility with openswoole 22 #110

acelaya opened this issue Jan 22, 2023 · 10 comments · May be fixed by #128
Assignees
Labels
Enhancement New feature or request

Comments

@acelaya
Copy link
Contributor

acelaya commented Jan 22, 2023

Feature Request

Q A
New Feature yes
RFC no
BC Break ideally, no

Summary

I have tried a project where I use this library with openswoole, and using openswoole 22.0.0, there seems to be calls to now removed functions (So far I have identified Fatal error: Uncaught Error: Call to undefined function swoole_set_process_name() in mezzio/mezzio-swoole/src/Event/ProcessNameTrait.php:33).

I believe this is the first version that trully deviates from swoole and might require considering how hard would be to continue supporting both.

The changelog does not mention the removal of this particular function, so I wonder how many other undocumented changes there are.

@acelaya acelaya added the Enhancement New feature or request label Jan 22, 2023
@acelaya
Copy link
Contributor Author

acelaya commented Jan 22, 2023

Maybe using openswoole/ide-helper:22.0.0 helps identifying which functions/methods have been removed or changed.

Also, this version has built-in support for psr-7. Might simplify integrating it.

@mairo744
Copy link

mairo744 commented Mar 2, 2023

I think it is more difficult than only removed swoole_set_process_name - https://openswoole.com/article/v22-0-0-released.

All classes changed at least namespace - example Swoole\Http\Server to OpenSwoole\Http\Server.

Will mezzio-swoole support also openswoole?

@froschdesign
Copy link
Member

@mairo744

Will mezzio-swoole support also openswoole?

It is already building on this: https://docs.mezzio.dev/mezzio-swoole/v4/intro/#swoole

Should modifications to the latest versions be necessary, then any help is welcome!

@Ocramius
Copy link
Member

Ocramius commented Mar 2, 2023

Worth checking this too, as a smoke test: laminas/laminas.dev#40

edit: that swoole from dev container is not actually used by automation or deployments.

@mrVrAlex
Copy link
Contributor

@mairo744

All classes changed at least namespace - example Swoole\Http\Server to OpenSwoole\Http\Server.

Namespace \Swoole is still supported in v22.x (look release notes)

@mrVrAlex
Copy link
Contributor

Maybe it can help someone right now. Can be solved error above with some hack, just call (before server start)

WorkerStartListener::$setProcessName = '\OpenSwoole\Util::setProcessName';
ServerStartListener::$setProcessName = '\OpenSwoole\Util::setProcessName';

bacause $setProcessName static & public property - it was worked solution. )

More right way (specially for this library) write own function swoole_set_process_name (wrapper on `OpenSwoole\Util::setProcessName`) and include it in composer autoload.
With IF conditional then can make support and swoole / openswoole / 4.x / 5.x / 22.x

@dcl20p
Copy link

dcl20p commented Apr 7, 2023

This is how I quickly solved the problem without having to interfere with the library:

<?php
use OpenSwoole\Util;
// Override the swoole_set_process_name function
if (!function_exists('swoole_set_process_name')) {
  function swoole_set_process_name($name) {
      Util::setProcessName($name);
  }
}

Can check the version of swoole > 4.6.0 to override:
if (version_compare(swoole_version(), '4.6.0', '>=')) { // Override }

@madalinignisca
Copy link

I would like to know if the focus will be 1st on OpenSwoole, and only keep compatibility with Swoole or even remove support for it.

Case: Newcomers to Mezzio, reading the official docs are guided for OpenSwoole.

I looked over the testing setup in the project, and noticed that it is testing against Swoole and not OpenSwoole.

Wouldn't it be OK to refactor the tests to use OpenSwoole? They do have an official PPA that works on top of Sury's PHP PPA currently used.

If the focus in code will be always 1st on Swoole, then the docs should be changed so less experienced people will not get in issues mezzio-swoole.

@weierophinney
Copy link
Contributor

@madalinignisca We (the Laminas TSC) have been discussing splitting the repo so that this one targets only Swoole, and a new one targets specifically OpenSwoole. The reason is because:

  • The two have diverged in API and features.
  • The two are progressing at different speeds (OpenSwoole has not had new features, other than PHP 8.3 support, in over a year).

We do not currently have a timeline for this. It will require first a new minor (deprecating OpenSwoole usage here), and then a new major (removing support for OpenSwoole here).

@kirkmadera
Copy link

Is there any status update on this? The current state of the package is broken being that the docs recommend OpenSwoole, but there are major updates with OpenSwoole that need to be addressed, like the namespace change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants