Last modified: 2011-12-13 21:10:27 UTC

Wikimedia Bugzilla is closed!

Wikimedia migrated from Bugzilla to Phabricator. Bug reports are handled in Wikimedia Phabricator.
This static website is read-only and for historical purposes. It is not possible to log in and except for displaying bug reports and their history, links might be broken. See T34969, the corresponding Phabricator task for complete and up-to-date bug report information.
Bug 32969 - Maps: including Validator.
Maps: including Validator.
Status: RESOLVED FIXED
Product: MediaWiki extensions
Classification: Unclassified
Maps (Other open bugs)
unspecified
All All
: Unprioritized normal (vote)
: ---
Assigned To: Jeroen De Dauw
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-12-11 19:01 UTC by Van de Bugger
Modified: 2011-12-13 21:10 UTC (History)
0 users

See Also:
Web browser: ---
Mobile Platform: ---
Assignee Huggle Beta Tester: ---


Attachments

Description Van de Bugger 2011-12-11 19:01:34 UTC
`Maps' extension requires `Validator' extension to work. If `Validator' extension is not yet enabled, `Maps' includes it:

> // Include the Validator extension if that hasn't been done yet, since it's required for Maps to work.
> if ( !defined( 'Validator_VERSION' ) ) {
>         @include_once( dirname( __FILE__ ) . '/../Validator/Validator.php' );
> }

> // Only initialize the extension when all dependencies are present.
> if ( ! defined( 'Validator_VERSION' ) ) {
>         die( '<b>Error:</b> You need to have <a href="http://www.mediawiki.org/wiki/Extension:Validator">Validator</a> installed in
 order to use <a href="http://www.mediawiki.org/wiki/Extension:Maps">Maps</a>.<br />' );
> }

This, probably, helps a little in simple cases, so wiki site admin may not worry about enabling Validator. However, in complex cases does not help but it confuses. 

I have a kind of wiki farm, and have multiple versions of extensions installed side-by-side, for example:

> Validator
> Validator-0.4.12
> Validator-0.4.13

so one wiki site can use Validator-0.4.12, another site can use Validator-0.4.13, and test site can use just Validator (from trunk). In my `LocalSettings.php' I mistakenly required `Validator-0.4.13' after `Maps', e. g.:

> require_once( ".../Maps-1.0.5" );
> require_once( ".../Validator-0.4.13" );

Instead of having clear error message "Maps extension requires Validator. Please enable it before enabling Maps." (or something similar) I have 

> Notice: Constant Validator_VERSION already defined in .../Validator-0.4.13/Validator.php on line 28 
> Fatal error: Cannot redeclare class ValidationError in .../Validator-0.4.13/includes/ValidationError.php on line 14 

Please, do *not* include `Validator' implicitly "on behalf" of administrator, but issue clear error message if `Validator' is not yet included.
Comment 1 Jeroen De Dauw 2011-12-11 20:40:08 UTC
Sorry Van, but the needs of the many outweigh the needs of the few :)

So I suggest you just remove the auto inclusion line in every version of the extension you have to avoid this.
Comment 2 Van de Bugger 2011-12-12 21:40:00 UTC
> Sorry Van, but the needs of the many outweigh the needs of the few :)

Let us consider few scenarios:

1. `Maps' extension does not include `Validator' but warns:
1a. If user included `Validator' before `Maps' -- everything is ok.
1b. If user included `Validator' after `Maps' (or did not include at all) -- s/he has a error message: "Please include `Validator' before `Maps'" -- it requires less than a minute to fix the problem.
2. `Maps' extension includes `Validator':
2a. If user included `Validator' before `Maps' -- everything is ok.
2b1. If user included *the same* `Validator' after `Maps' -- everything is ok.
2b2. If user included *another* `Validator' after `Maps' -- strange PHP errors, no clue what is the problem.

I understand that only very few users will see the case 2b2, but they will be in very bad situation. PHP (unlike to modern C++ compilers) does not show the location where the symbol was defined for the first time. Moreover, web application is not simple for debugging. Understanding what the cause of the problem may take a lot of time.

I spend a noticeable part of my life hunting problems in software, so I hate unclear error messages.

You can avoid unclear error message, but decide not to do it. Ok, that's your choice.

> So I suggest you just remove the auto inclusion line in every version of the
> extension you have to avoid this.

Jeroen, you see that I already know the cause of the problem, so the suggestion is useless to me. I have put a comment to my `LocalSettings.php':

#################################################
# `Validator' must be included before `Maps'!!! #
#################################################

But somebody else can face this problem in future, and get stuck. You are leaving them with no help.

Praemonitus praemunitus.
Comment 3 Jeroen De Dauw 2011-12-13 18:31:45 UTC
I really disagree that this is more important then automatic inclusion of the extension. Such dependencies are unusual, so this is saving a lot of people not familiar with MediaWiki hassle.

In any case, your concern is probably addressed by r106053.

To be honest, these checks just suck. It's better then nothing (which is what you get with most extensions), but MediaWiki ought to have a decent extension registration and loading mechanism that takes care of dependencies, compat issues, ect.
Comment 4 Van de Bugger 2011-12-13 20:39:55 UTC
Thanks, it is better than nothing. Could you also update

http://mapping.referata.com/wiki/Template:Maps_installation

One sentence would be enough, like you done it for API keys:

> For using Google Maps or Yahoo! Maps, you must enter their API keys. You need
> to add them in LocalSettings.php, ***after*** the inclusion of Maps. 

Thanks.
Comment 5 Van de Bugger 2011-12-13 20:51:48 UTC
BTW, I do not object against automatic inclusion until it makes no problem is something goes wrong or just in non-standard way. If automatic inclusion produces clear error messages (like modern C compilers e. g. "redefinition of macro XXX in file:line, XXX was defined in file:line") I will happy to have such great tool. But if automatic inclusion produces just a little help for normal case but generates unclear and misleading error messages, I do not like it.

And I agree, enabling an extension in MediaWiki sucks. I tried to wrote a function like this:

function LoadExtension( $name ) {
    include_once( "$IP/extensions/$name/$name.php" );
}

so I can

LoadExtension( 'Array' );
LoadExtension( 'Variable' );

but it does not work, because all the extensions think they are included at global scope, nobody  declares global variables. D'oh!
Comment 6 Jeroen De Dauw 2011-12-13 21:10:27 UTC
I have this, and it works just fine:

foreach( $extensions as $extension ) {
	require_once( dirname( __FILE__ ) . "/extensions/$extension/$extension.php" );
}

> Could you also update

By all means, use the edit button :)

Note You need to log in before you can comment on or make changes to this bug.


Navigation
Links