Last modified: 2010-12-13 15:33:01 UTC
I am raising this, as it is a change between 1.16 and trunk, which has broken my WikiDB extension. In my extension, I sub-class the IncludableSpecialPage class to create my own WikiDB_SpecialPage class, which my extension's special pages use. This class provides a load of common functionality, used by all my pages. It also provides a static RegisterSpecialPage() function, which just takes the special page name, and calls SpecialPage::SpecialPage() with all the necessary arguments as, aside from the name, these are the same for all pages. In trunk, this now raises a 'Call to undefined method' error. This happened in r71961, where demon removed the old PHP4 style constructors, and replaced them with the PHP5 __construct() method. In terms of fixing my own extension, I know I can just replace the call to Special::SpecialPage() with parent::__construct(), but my concern is that mine is not the only extension that will be broken by this change. I'm happy for this to be marked as WONTFIX if there is a fair degree of confidence that not many existing extensions will be broken (i.e. that my use-case is unusual, or that most extensions already use the new style). However I would strongly urge that bw-compatibility be maintained somehow if there are more than a handful of extensions that may be broken by this.
Created attachment 7872 [details] reimplement PHP4 old style constructor for extensions still using it
Comment on attachment 7872 [details] reimplement PHP4 old style constructor for extensions still using it Marking patch obsolete. Having two constructor raise a strict warning. Maybe we should rename __construct to SpecialPage to keep backward compatibility.
(In reply to comment #0) > This happened in r71961, where demon removed the old PHP4 style constructors, > and replaced them with the PHP5 __construct() method. > You left out the part where I fixed most of those in the very next revision, r71962.
Yeah. The biggest thing is gonna be if people don't upgrade extensions when they upgrade MW. But then, that's pretty much their issue, no?
(In reply to comment #4) > Yeah. The biggest thing is gonna be if people don't upgrade extensions when > they upgrade MW. But then, that's pretty much their issue, no? Some people want thinks backward and forward compatible forever. I'm not naming names though. (In reply to comment #0) > I'm happy for this to be marked as WONTFIX if there is a fair degree of > confidence that not many existing extensions will be broken (i.e. that my > use-case is unusual, or that most extensions already use the new style). > However I would strongly urge that bw-compatibility be maintained somehow if > there are more than a handful of extensions that may be broken by this. Using trunk: $ ack '(parent|SpecialPage)::SpecialPage' RecordAdmin/SpecialRecordAdmin.php 30: parent::SpecialPage( 'RecordAdmin', 'recordadmin', true, false, 'default', true ); WikiBhasha/WikiBhashaSpecial.php 35: parent::SpecialPage( 'WikiBhasha', '', true ); I just fixed those in r77400. So yeah, I'm pretty confident.
(In reply to comment #5) > (In reply to comment #4) > > Yeah. The biggest thing is gonna be if people don't upgrade extensions when > > they upgrade MW. But then, that's pretty much their issue, no? > > Some people want thinks backward and forward compatible forever. I'm not > naming names though. > Not forever, but the problem arises when people upgrade to 1.17 and suddenly one or more extensions stop working. So they go to MW.org, or the author's home-page or wherever to get the latest version, and this is the first time the extension author hears about the problem. So they look into fixing it and it takes a while to get a fix out (for whatever reason) and in the meantime the user has to disable the extension (which may not be acceptable) or roll back to 1.16 (which may not be simple). The safe way of implementing a change such as this is to make the change, but allow the old method to continue working, generating a warning if it is used. The bw-compatible hack can then be removed in 1.18 (or 1.19) once people have had a chance to upgrade their code. I'm not sure if that is possible with this particular issue, however - it seems like it might be an either/or situation. (In reply to comment #5) > I just fixed those in r77400. So yeah, I'm pretty confident. Good to see the extensions in the WMF repo are all fixed. What about third-party extensions? Any way of guaging that? Just for the record, there is no idealogy motivating me in this bug - I just want to make sure that the implications have been thought through and properly risk-assessed. I'm not trying to make trouble - I'm trying to make sure we avoid it! :-)
Could it be possible to intercept the error in MW 1.17 and output a clean error asking to upgrade the extension / fix the issue ?
In r78192, I added some magic with __call() so we can still catch callers to the old constructor name. For now we'll throw it in wfDebug(), maybe wfDeprecated() in another version or 2.
