Savant: Revoking the 2.3.0 release

I am revoking the Savant 2.3.0 release from yesterday. The file will still be available, but I can no longer vouch that it will work everywhere for everyone, and I cannot promise that the new function provided therein (“calling of plugins as native methods”) will be in a future Savant 2.x release. I recommend reverting to 2.2.0, particularly for PHP5 users.

For 2.3.0, the only major change was that you could call plugins as native Savant functions through the magic of overload() and __call(). The use of __call() turned out to be unwise, as it exposed a serious compatibility issue between PHP4 and PHP5.

I’m a big believer in reporting failure as well as success; it’s important to let others learn from your mistakes. It’s embarrassing, but necessary. This is one of those times, I’m afraid. The use of __call() turned out to be a serious problem. Yes, overload() is marked as EXPERIMENTAL in the PHP4 documentation, and brother, do they mean it.

Read on for more information about how __call() bit me in the ass, and why aiming for simultaneous compatibility on PHP4 and PHP5 can be unexpectedly difficult when trying to do “neat stuff.”

__call() in PHP4 and PHP5

The first problem is that the arguments for __call() are different in PHP4 and PHP5 and are enforced differently.

In PHP4, you need three arguments: __call($method, $params, &$return). $method is the method called, $params is an array of parameters passed to that method, and &$return is a reference to the variable into which the method call should return a value. The “real” return value of __call() is true (indicating that the call succeeded) or false (which will trigger an error that $method does not exist). You can leave off the &$return parameter if you like, and PHP4 will not complain.

In PHP5, you need exactly two arguments: __call($method, $params), both of which act the same as in PHP4. If you have a third argument, PHP5 will issue a parsing error (which means the script will not load). Under PHP5, the return value of __call() is passed the way you would expect with a normal method, which is why you don’t need the &$return argument.

You can see already why this would cause trouble: any __call() definition that works in PHP5 will limit its PHP4 equivalent to never giving a return value (because you can’t have the &$return argument in PHP5). Similarly, any fully-functional PHP4 __call() method cannot possibly work under PHP5 (because the &$return argument will cause the PHP5 parser o choke).

__call() in PHP4.3.2 and 4.3.8

When I originally tested the __call() method in 4.3.2, it would successfully set &$return to an object. However, in 4.3.8, it would not set &$return to an object (scalar or array came back OK, but no objects).

Possible Solutions

First, I tried having alternate class definitions in the same file, and picking the definition based on phpversion(). Did not work. PHP5 chokes when parsing the file with a “bad” __call() definition, even if its inside the PHP4 portion of the if() block.

Then I tried extending a base _Savant2 class into Savant2 class proper with an include at the end of the Savant2.php file; that is, if ($php4) include ‘Savant2v4.php’; if ($php5) include ‘Savant2v5.php’. Each of the version-specific files worked in their respective environments, and I thought I had a successful solution.

However, further testing showed that PHP 4.3.8 would not set &$return to an object (necessary in Savant as it reports plugin errors using the Savant2_Error class). To my recollection, it had worked properly in 4.3.2.

For me, this was the final straw; if __call() didn’t work consistently even under 0.0.1 versions of PHP4, then it was not good for production use.

Whose Fault?

Mine, of course. First, I used overload() which is clearly marked as experimental. Then, I tested it thoroughly, but on only one version of PHP (4.3.2) and figured it would be forwards compatible (wrong!). These two errors, combined, resulted in a broken release under some versions of PHP. My goal for Savant is that it run everywhere, and this release failed to meet that goal; to boot, it broke severely under PHP5, which userbase is only growing.

I tried to fix the release, but I fear there is no elegant solution. Rather than excuse my bad planning through documentation (i.e., “Savant works this way under PHP4 but it’s not quite right, and under PHP5 it works this other way, so look out!”) I decided to pull the release. It’s only been out 24 hours, and the added functionality is convenient, not critical. (Still embarrassing and irresponsible to have released it like that in the first place, though.)

So the blame lies with me; I’m sorry if I broke your app with this release. Having said that, oh how I wish the __call() method was identical in both PHP4 and PHP5.

Are you stuck with a legacy PHP application? You should buy my book because it gives you a step-by-step guide to improving your codebase, all while keeping it running the whole time.

6 thoughts on “Savant: Revoking the 2.3.0 release

  1. See the DataObjects code for how to make it work on both.

    – I’m not sure about the returning object – I’ve not seen that before.

    Be aware that PHP4 can not call by reference, or return by reference on other methods, if you overload it.

  2. Hi, Jason — agh! It hurts my brain to envision it. 😉 No, eval() is really terribly slow and not something I like to use.

    The functionality that __call() provided was “neato” and convenient, but not mission-critical at all. In a way that makes my error worse; I compromised a working class with a broken convenience functon. 🙁 However, Alan Knowles has what looks like a good solution in DB_DataObjects; as I have wanted this particular convenience feature for Savant for more than a year, I will continue working toward it, but I can promise that I’ll make no more releases that are not thoroughly tested.

  3. I can think of two different additional approaches to make, though I have no time at this moment to try them out. They include:

    1) use func_num_args() and func_get_arg() to parse the argument list, and handle them differently based on the phpversion() returned. This way, in PHP4, you could:

    * pass the second argument as the arguments to the function/method specified in the first argument

    * expect the third argument, if given, to be a reference into which the value will be returned

    Then, in the methods/functions being called via overloading, expect one parameter, an array of arguments, and one optional parameter, a reference into which the result will be passed. Then, whenever you are ready to return a result, set the reference to the return value and return it.

    2) Save overloading for Savant 3 🙂

  4. Hi, Matthew —

    Thanks for the ideas, but it doesn’t work quite like that. Alan Knowles has a working solution, you can see it at (starting around line 174). However, it involves using eval() which just rubs my fur the wrong way, and does not address the different behavior of __cal() on different versions of PHP4.3.

    Regarding your suggestion number 2, “Save overloading for Savant 3” … ding ding ding! We have a winner! 🙂 I don’t want to bring up version 3 yet, to avoid the Osborne problem. Having said that, I’ll do v3 once PHP 5.1 is out the door, and write it for E_STRICT compliance.

  5. I’m sorry, but I can never agree with the generation of (X)HTML code. This is probably an attempt to save time while doing templating but the only clear effect I can see is an overall slowing of the View.
    I refactored the View class to have a locale Helper by default, and stripped out all the code generators. In your current implementation, a View keeps growing. Having multiple calls from helpers to view via this system is not affordable.

    Of course the mass will like to have such a system, and sure it is a great feature.

Leave a Reply

Your email address will not be published. Required fields are marked *