antecedent/patchwork

Maintained or abandoned ?

jrfnl opened this issue ยท 18 comments

jrfnl commented

I'm starting to run tests for various projects against PHP 8.1 and am seeing issues crop up caused by underlying Patchwork code.

I'd be happy to attempt to fix these and send in PRs for these issues, but seeing that the last commit to this repo appears to be from 2019, I'm wondering whether this repo is still actively maintained or has been abandoned.

Wouldn't want to waste my time, kind of thing.

jrfnl commented

Bump

Hi @jrfnl,

I'm sorry for the delay here. As for the issue, I must now make a decision.

As of 2021, there is quite a range of competitors, if that's even the word one would use for OSS. Go! AOP was the first to get working at this idea; the package antedates Patchwork, I think. Next, there's badoo/soft-mocks, php-mock/php-mock, and some more. Some of the authors have arguably made better engineering choices than I did; some of them also seem to be in a better position to maintain their libraries.

I assume you (@jrfnl, and any reader here) are familiar with the ecosystem that I mention in the preceding paragraph. I would like to know if we could consider Patchwork superseded by any of the other packages. Could you help by sharing your experience?

Specifically, with Patchwork, I took a different approach here and there, and as a result:

  • it will capture calls no matter how they are made (relative name, (fully) qualified name, $var(), call_user_func($var)), both for user-defined and internal functions,
  • it can redefine some language constructs (new, eval, die etc.)

I don't think there's anything else, really; and I think that Go AOP supports both of these too, to an extent at least. (As to the preceding point, I must do some more research.)

I would, therefore, like to ask @jrfnl and all other users of Patchwork if you rely on either of these two features. In general, I do see a future for Patchwork, but if it turns out that these two are only marginal advantages, it would be prudent for me to discontinue the package.

Thanks in advance!


PS: I would also like @gmazzap to weigh in, given that they own brain/monkey, the Composer package that generates the most traffic for Patchwork, it seems :)

jrfnl commented

@antecedent Thank you for your thoughtful response.

My question is heavily based on the use of BrainMonkey as well, so I too, would be very interested to hear @gmazzap's opinion.

Whatever you decide, thank you for all your time and effort on creating and maintaining Patchwork.

My offer still stands (to send in patches for the immediate issue (PHP 8.1)), which would allow people some more time to switch over, if that is what they will need to do.

Thank you for the offer; I would indeed appreciate some help with PHP 8.1! Meanwhile, there are test cases that already broke with PHP 7 PHP 8.0. I'll start by opening some new issues, and I'll try and get working on them soon.

jrfnl commented

@antecedent Looking at things now. While I'm at it - I notice you are running the tests via Scrutinizer and only up to PHP 7.3. Would you like me to turn on testing against PHP 7.4 - 8.1 ? And possibly move the running of the tests to GH Actions ?

jrfnl commented

Submitted three patches so far, bringing test failures on PHP 8.1 down from 20 to 5.

I've also run the BrainMonkey tests against PHP 8.1 with these three patches in place and while it is no 100% guarantee that everything works, the tests pass.

jrfnl commented

Of the remaining 5 test failures, the first three are related to PHP 8.0 and already failing on that. At least one of these failures seems to be related to #103, another to #104.

The two remaining PHP 8.1 issues are in regards to traits:

Deprecated: Calling static trait method FooTrait::speak is deprecated, it should only be called on a class using the trait

Related RFC: https://wiki.php.net/rfc/deprecations_php_8_1#accessing_static_members_on_traits

jrfnl commented

Send in another PR to fix the trait issue mentioned above.

jrfnl commented

@antecedent Saw you've switched over the running of the tests to GH Actions. I've send in a PR with some tweaks and to get the tests running on PHP 8.1.

The test failures as of PHP 7.0 are weird as I can't seem to reproduce those locally. Are you already looking into these or would you like me to look at those ?

Thank you so much for everything that you are doing here!

As for the two mysterious failures, I ended up in the very same situation. And I'm not sure if I'm prepared to be looking at them any more for today :/

jrfnl commented

@antecedent Happy to help. Thanks for the quick merges. I've figured out the mystery failures and send in a PR to fix that, but that exposed another failure ๐Ÿ˜•

Might as well be done! I've wrapped all of this up into v2.1.13.

jrfnl commented

@antecedent You're a star! Thank you so much for working with me on this to get it done!

Sorry @jrfnl @antecedent received notifications about this and wanted to find some minutes to read properly and answer.

Came here on a Sunday when finally found some time to answer... and I see you two already solved it.

All my gratitude to both of you ๐Ÿ™‡๐Ÿปโ€โ™‚๏ธ You both are stars โญ

jrfnl commented

@gmazzap Well, I think your answer to the question posed by @antecedent would still be appreciated.

What we've done now is basically "buy time". Both for @antecedent to take a decision on the future of the package and in case it would be abandoned, for implementors to have some time still to switch to another implementation without running into trouble with PHP 8.1 straight away.

Ok, let's answer then :)

@antecedent Because Brain Monkey is used mostly in WP the possibility to capture all references to functions (no matter how they are done) is very important.

Regarding language constructs, I don't think Brain Monkey will ever want to use that functionality.

When I decided to use Patchwork, Go! AOP was already there (not sure which one was created first). But besides Go! AOP doing more than I needed, it added too many dependencies for my taste, and things have gotten worse with time. Not to mention that to find a version compatible with the PHP version supported by WordPress I would need to go back in time and use version 1.x, which looks like abandoned and didn't receive commits for 5 years.

I also looked at php-mock/php-mock, but it only relies on namespace fallback policy, something that is "too limited" IMO for Brain Monkey. Especially call_user_func usages is (sadly) still pretty popular in WP space.

I honestly did not know badoo/soft-mocks, but at a quick glance, the API/functionality would be fine (very nice the possibility to re-define constants), but the limitations when working with PHPUnit is something that I really would like to avoid.

In short, these days, Patchwork would be still my pick: it doing a single thing well, working with a very large PHP version range, with zero dependencies (both userland and on PHP extensions), and high compatibility, is something that none of the "competitors" seems to have. For this reason, I'm not sure your "engineering decisions" were worse ;)

So I can't surely force you to maintain this, but I really hope you will. If abandoning support for replacing language constructs would help you to keep things simpler and easier to maintain, you've my blessing, if it counts anything :)

Thank you for the informative response, @gmazzap. I really needed to put Patchwork's situation in perspective, and you helped me to :) And @jrfnl, thank you for steering the discussion in the right direction; I had to move away from all these matters for a few days.

I intend to keep maintaining the library.

jrfnl commented

Closing this issue with the conclusion that at this moment, the library can still be regarded as maintained. Thank you @antecedent !