Activating plugin that uses upgrade functions (dbDelta) fails

Test plugin that causes this error to occur

Apparently, the problem is that the wp-admin/includes/schema.php file has a line near the top, that in 2.5 bleeding, calls the $wpdb->supports_collation() function.

Just putting that function in the activate function causes this problem to occur. I am not sure why, because when I copied the body of the function and did the same thing, that was OK. ????

  // this line is OK, so it isn't the $wpdb object itself...
  $tmp = $wpdb->prefix;

  // this line is OK -- and this is the function body
  $tmp = version_compare(mysql_get_server_info($wpdb->dbh), '4.1.0', '>=');

  // this line is OK
  $tmp = $wpdb->dbh;

  // but this line causes plugin activation to have an error
  $tmp = $wpdb->supports_collation();

In fact, when I put that last line at the top of my plugin file (outside the activate function), it also caused a fatal error (though I didn't get an error message). Any idea what might be happening, or why that would cause a fatal error, when the function body of the supports_collation function doesn't? It would be helpful if I had an error message, but it doesn't give me anything useful.

I changed the title of this bug -- actually, the activation process is not working correctly (db functions are not being created).

I also changed this to a higher severity, because I think a lot of plugins that use separate DB tables use this method to keep them updated. And this used to work fine in 2.3; worked fine in 2.2 also, though you had to include a different file:

   if( file_exists( ABSPATH . 'wp-admin/includes/upgrade.php' ))
     require_once(ABSPATH . 'wp-admin/includes/upgrade.php');
   else
     require_once(ABSPATH . 'wp-admin/upgrade-functions.php');

The plugin is now being loaded within activate_plugin(). When upgrade.php is included, it is not in global context. We might need to sprinkle some "global" statements around.

Ah, that explains it!

I am now able to fix the issue with my plugin (the real one, which creates tables) loading. To do this, I had to add two global declarations in the plugin file (for variables defined at global scope and used within the activation function), and also one line to the top of schema.php saying:

global $wpdb;

I'll put in a patch for that, as it should be innocuous for normal use.

Thanks for the clue! I never would have figured out it was non-global scope causing the problems... this seems like a major change, but I think I understand why.

By the way, if there *is* an error during the calling of the plugin activation function, the plugin does get loaded twice. Do you want me to file a separate bug report on that? It means the error message is bogus (as in my report, above).

Patch that makes it possible to load dbDelta functions during plugin activation

Added a small patch that lets the dbDelta function be loaded

I feel like I'm back in C/C++.

I'm not seeing how the patch does anything that shouldn't already be available. I'm not sure that the patch fixes anything. If $wpdb isn't available then global $wpdb doesn't do anything except declare an variable that was nonexistent. If it did exist, then global $wpdb; should just acknowledge that the variable existed in global scope anyway.

That doesn't seem to be a solution and if it is, then you just turned everything I think I know upside down and I should stop programming completely because I don't know anything.

It seems unrelated, what is the execution flow? You said it includes your plugin twice, which it should not do and even if it does, you only execute the init once, so it should blow up on your function and not on the require_once().

In the case that it did execute the function twice, then it should still only include the file in require_once() one time, as would be the case with require_once(). If not, then you have a PHP bug with require_once().

Both the solution and the problem seem completely mind boggling and my brain is about to implode. Can you further explain, so that down is not up and reality is real again?

Replying to ryan:

The plugin is now being loaded within activate_plugin(). When upgrade.php is included, it is not in global context. We might need to sprinkle some "global" statements around.

This is interesting. So a plugin including another file will also be within that functions scope, unless it was inside a function, then it would be within its own function scope.

That is quite fascinating. However, the scope of it would be instead inside the plugin function, so the global should be declared there and not in the core file.

schema.php relies on the globals so they should be declared in schema.php. Declaring $wpdb and $wp_queries global at the top of schema.php seems like it would prevent these surprises. We've been bitten by $wp_queries a few times already.

The solution is not found with adding global declarations to scheme.php, but in the functions that include scheme.php from within that function's scope. The only reason you would have to declare $wpdb global is if wpdb.php hasn't been included yet and $wpdb doesn't exist, or it is within function or method scope.

The short term solution of adding global definition (when the scope is already global) is hackish at best and points the source of the problem away from the root of the problem.

I would suggest that if it is a problem, then search for scheme.php in the files and write a patch which solves the problem (the better long term solution).

I think the best solution would be to just move that function out of scheme.php into functions.php since it is useful enough to plugin developers that it should be available without having to require_once() the file. I would love to have that function, but I usually build a custom solution myself.

Fixing it for all callers by adding the global to schema.php is more defensive. We can fix this once and for all instead of every time it pops up. But, hey, I don't much care either way.

I'm basically just playing devil's advocate here, so whatever decision is not going to affect me whatsoever.

Why not just put the rest of the code in a function? You would have to make $wpdb a global and it isn't going to call the code until it is needed. The overhead is slight. You would have to make those variables globals in the functions that call them.

My argument basically breaks down, because no one is going to be looking at scheme.php anyway. I would be more comfortable in which case, when a developer did look at that file that they knew the reason why $wpdb was being declared a global, so that they don't adopt the practice as their own.

I really hate it when the function body is split up into separate files (and isn't HTML), because it makes it difficult to (initially) follow the flow.

To be honest, I could basically care less, but the people that criticize will just have one more (minor) example to throw at the WordPress world. It is in my opinion, that the only ones that will be calling wp-admin/includes/scheme.php is WordPress, which means that that 1) there shouldn't be that many files and functions including the file and 2) that the better, or best fix will be worth implementing in WordPress.

Since well, I've bought this up and argued the issue, I might as well provide the patch.

(In [6896]) Declare wpdb and wp_queries as global. Props jhodgdon. see #5860

I did the least effort fix for now. We can clean it up later.

That fixes the bug described here, so I'm closing.

Sorry, I'll provide a patch after I graduate. I don't have the time at the moment.