Move drupal_*_schema_*() functions into one place

+1 for schema.inc

The drupal_get_schema_versions(), drupal_get_installed_schema_version() and drupal_set_installed_schema_version() - all in intall.inc - were not in group schemaapi. I left _drupal_schema_initialize(), drupal_schema_fields_sql() and drupal_write_record() in common.inc

 includes/bootstrap.inc |   76 --------------
 includes/common.inc    |   83 +---------------
 includes/install.inc   |   98 ------------------
 includes/schema.inc    |  264 ++++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 265 insertions(+), 256 deletions(-) 

schema.inc needs to be included also at includes/install.core.php

wrong patch format

Added require_once in update.php

#7: 1180112-move_schema_functions.patch queued for re-testing.

Weird, what happened? It was working before!

I was trying to apply your patch and I couldn't, so I guessed I'd better resubmit it for test. You can see the result. Anyway, why did you opt to create a new .inc file, at the cost of including it here and there? It seems that module.inc could be a good place for the schema functions, and maybe also for _drupal_schema_initialize(), drupal_schema_fields_sql() and drupal_write_record().

A lot of these functions are needed very rarely, sometimes never on a production site, that's the kind of thing it would be worth trying to convert to a class to be lazy loaded - code that is never or almost-never loaded during the normal lifecycle of an apache process is worth lazy-loading with or without an opcode cache. However that in itself shouldn't stop us just moving the code for now.

As you can read from the description, leaving those inside schema.inc is better in long-term, specially if it's going to be lazy-loaded in the future.

I didn't find these other functions you cited, but drupal_write_record() is used quite often, so it wouldn't benefit so much to have it there, for the very same reasons.

Patch needs to be re-rolled. Doing it.

There were some changes in functions, so I tried to make adapt to those - meaning, checked all of them for changes. The class for schema cache is also being moved.

The cache_get() and cache_set() functions were changed within the last week to use the new API class in D8.

You might want to add documentation in this patch that the cache API class has its own implementation of schema() so that needed cache bins can be created dynamically. I do not know if any other major core classes have their own schema definitions.

I don't see how this documentation - which refers to some other functionality - would fit in this patch. If the documentation is added otherwise to some function we're moving, we can re-roll this patch to contain it.

#14: 1180112-move_schema_functions.patch queued for re-testing.

Just re-rolling

Should this patch include docblock corrections or should that be deferred to another issue?

For instance, at start of docblocks should be an active verb... s/Get/Gets/
Also there should be a blank line before @return doc line.
Finally, several @return doc lines are missing completely.

@Lars, well, since we're moving them around, if it's changed elsewhere it needs to be re-rolled here. Still, doc corrections get faster into code, IMO it's better handle it separated and update this ticket when that's committed.

#21: 1180112-move_schema_functions.patch queued for re-testing.

Re-rolled

Bug is really weird. Looks like schema is being written in cache with cid = ''. I don't know how this function moves could've caused it, I checked them one by one.

#27: 1180112-move_schema_functions.patch queued for re-testing.

Thanks.

Recreated this patch in http://drupalcode.org/sandbox/sun/1255586.git/shortlog/refs/heads/platfo...

Do we also want to handle Schema API functions from database.inc in this issue? And while we're at it, I think drupal_write_record() should be db_write_record().

Looks good to me, merely moves functions around.

The functions in bootstrap.inc shouldn't have been in bootstrap.inc in the first place.

Wasn't 100% sure whether drupal_write_record() should be moved as well, but as it completely relies on the other schema.inc functions, that makes sense.

There's only one stale require_once for common.inc in one of the moved functions, but that ain't really a problem and can be cleaned up later.

Possibly related: #1618346: Error: Call to undefined function drupal_get_installed_schema_version() in core/includes/update.inc, line 94