Remove stub update functions and renumber the remaining ones

This is not critical in the traditional sense, but it is critical based on the same logic that led people to mark #898558: Clean-up the upgrade path: node as critical: If we don't renumber these functions now (before the D7 HEAD-to-HEAD upgrade path is officially supported), we won't be able to do it all.

Here's a patch. Kind of mindless, but it seems to work :)

Notes:

1. Although I removed most stub functions, I deliberately left a few behind - these are ones that had comments like "Update moved to update_fix_d7_requirements". The reason is that things which happen in update_fix_d7_requirements() never get documented for the administrator anywhere else, so we might as well leave them behind to document it. Perhaps for consistency we should kill those too though; there are a lot of things that happen in update_fix_d7_requirements() that we already don't tell people about in the detailed upgrade notes.
2. This patch necessarily renames the system_schema_cache_7054() helper function to system_schema_cache_7040(), since the update function it is associated with has also been renumbered. This may constitute a (tiny) API change, but I think it's worth doing to keep the numbering intact.
3. I left behind openid_update_6000(), although I'm not sure why it's in the codebase - aren't we only supporting updates for sites that are running the latest D6? However, it seems to have been added recently in #886982: Incomplete verification of assertions and is related to a security issue, so I didn't remove it for now.
4. I deliberately left the comment.module updates alone here, since that is soon to be taken care of by #898520: Clean-up the upgrade path: comment anyway.

The HEAD to HEAD module already needs an issue as a result of previous rounds of update function renumbering that were committed to core. It will need it even more if this patch is committed :)

The existing issue for the HEAD to HEAD module is here: #909338: Fix the schema versions in the {system} table for dblog and comment update function renumbering (and now locale too!)

Interesting. Although if you look at my patch, I'm not sure I removed anything that wasn't just pure cruft at this point - i.e., probably long past the point where it was useful to anyone. And it's consistent with some of the cleanups that have gone in elsewhere, for individual modules.

Also, if we don't do something like this then we'll have weird gaps in the update function numbering that makes it look like core developers don't know how to count :)

Perhaps it would make sense to combine the two approaches, though.

 /**
  * Adds 'delivery_callback' field to the {menu_router} table to allow a custom
  * function to be used for final page rendering and sending to browser.
+ *
+ * Moved to update_fix_d7_requirements().
  */
 function system_update_7041() {
-  // Moved to update_fix_d7_requirements().
 }

Hm, this looks a little like magic, though... Dumb question: Is there a way to check if the function body is empty of code (regardless of whether or not it has comments in it)? Seems like that's ideally what we want to check.

Also, if the consensus is to not bother renumbering the functions, then I think this isn't critical anymore (since it can be done at any time).

Hm, I realized that this actually introduces a worse bug than it fixes, because it introduces a mismatch in the reported number of updates to be run (which everyone sees, not just the 2% of people who open up the fieldset to look at the details).

For example, updating from Drupal 6 to Drupal 7 it tells me "118 pending updates" before I run them, but then the progress bar shows 130 updates once they actually start running - the difference being due to the stub functions. (In an edge case, this could lead to even more confusion - if somehow you wind up with a site where the stub updates are the only ones that need to run, then your status report page will keep telling you your database is out-of-date but when you go to run updates it will tell you that you don't have any.)

I think there is no simple way to fix that, so probably we should roll this back and try something else. I think it's simplest if we just delete the obsolete functions. I don't think there's anywhere else in the code where we deliberately keep old, useless functions around, is there? So no need to do it here. And they're still available via CVS, of course, for developers who want to see the history.

Does that sound right? I'm not proposing renumbering the update functions anymore as I did originally (as it seems like most people think that's too much code churn now), just removing the 10 or so that are empty, as the simplest way to make them not appear anymore.

***

By the way, not that it matters much with the above plan, but I don't understand why tokenizing the file would have been necessary to find out if the function body only consists of code comments? (The code already defines $body as an array containing all lines of code in the function, so it seems like we could just look at the individual lines in that array and see which ones began with a comment marker, etc.)

@mikejoconnor, thanks, but your patch file looks much smaller than my patch from comment #2. Did you deliberately leave parts of it out?

***

I'm still theoretically in favor of doing a full renumbering, but most other people above sounded opposed. Maybe we should come to consensus before writing more code. The choices are:

a. Revert #14 and just remove the stub functions (but leave the numbering incorrect).
b. Revert #14, remove the stub functions, and renumber the remaining ones.

Note that to do (b) properly, we also need some changes along the lines of those in my original patch such as these:

-    $schema['cache_path'] = system_schema_cache_7054();
+    $schema['cache_path'] = system_schema_cache_7040();

which might be considered a minor pseudo-API change at this point.