Move language listing functionality from locale.module to a new language.module

Also, #1301148: Stop pretending we have configuration translation for languages either needs to be implemented in this issue or I need to postpone this one on that too. Once that and the above land, we'll have a pristine language list managed with no fuss hopefully :) That can be separated without pain from the rest of locale.

Here is a very initial take on this patch, which will show how big of a task this is.

1. We need to use this opportunity to rename the table to take ownership of the new schema. The old languages table will need to be dropped.
2. Therefore all uses of the languages table needed updates.
3. I've just moved the language schema and the save and delete API functions and still this patch became pretty darn big due to the above baggage. Broke out the locale / ui translation related code to hook implementations to react to the language changes.

Did not move any of the admin UI for this and no update function yet since we are trying to get these patches land for that before that can happen:

#1260860: Rework language list admin user interface
#1296566: Improve usability of add language screen
#1301148: Stop pretending we have configuration translation for languages
#1301460: Decouple domain/path language negotiation storage from language config storage

Although I took care to update most stuff, this might or might not fail in a huge fire when the tests run :)

Surprisingly it mostly fails on the native name thing, which is not yet removed, so should be in the schema still. Added back there.

Update for remaining test fail to make locale uninstall test pass for now. We cannot solve all things in one patch.

Rerolled for #1260510: Introduce a language_load($langcode) and other patches being landed. Because the painful interconnection with negotiation and localization, I still consider this dependent on #1301148: Stop pretending we have configuration translation for languages and #1301460: Decouple domain/path language negotiation storage from language config storage but the attached patch should be a sneak peak at what is coming. (Once we have those two patches landing, we can move the admin pages as well and update the schema here and put in an update function :).

At this point only these two patches hold this down AFAIS:

#1301460: Decouple domain/path language negotiation storage from language config storage and
#1323176: Decouple UI translation language information from language list schema

Let's work on those and get this in!

Ok, here is a quick reroll since all dependencies are now committed. To reiterate, the major goal of this patch is to separate the language management functionality itself from the negotiation/localization/date format, etc. heap of code. Whether we want to move negotiation with this as well I think we can even discuss later, this is already a huge patch which has LOTS of advantages.

- renames 'languages' table to 'language' as per current Drupal code style (@todo lacks update code)
- renames locale_language_save/delete to language_save/delete (language_load() was already lacking the locale_ prefix)
- renames related hooks alike
- moves language admin forms from locale.admin.inc to language.admin.inc and renames them for consistency (locale_language_* and locale_languages_* to language_admin_*)
- untangles interface and language related code, like moving interface translation cache rebuilds in language_save and language_delete to hook implementations, move the language add screen batch sets to interface translation from language management

I think this in itself is a huge improvement. What I believe is @todo for this issue:

- move tests to language.test that are relevant for language list management only
- make sure tests properly pass (they might or might not at this point)
- add an update function for the table rename

What should be done in follow up patches:

- rename the language.language column to language.langcode
- do multiple passes on the code moved to clean up messages, etc. I did lots of fixes for %code => %langcode, %locale => %language, etc, but there might be more
- discuss whether to move language negotiation to this module or to a new module as previously discussed in #1293304: Break up locale.module, but how?
- work on the user and node language selectors which are now in locale module, but should not be, this is being worked on in #540294: Move node language settings from Locale to Node module, #1280996: New language_select element type for form API et. al.

This is a huge patch in itself (and I think a huge improvement), so I think we should keep it focused, therefore the plans for followups.

Fixed a few more languages table names missed.

Minor issues caused test failures, now fixed. There should still be a test failure due to the missing upgrade path (yay for having test coverage for that now).

Here are some fixes to tests (to update them to latest standardized message text used in module). Also, added one-line table rename update path, which is needed to be able to do bootstrap. I doubt this will now work well with the locale update functions, since those then assume a table named 'languages' vs. a table named language. We maybe need to move the whole thing to the update.inc code instead of update functions I guess. Added interdiff from last patch too.

Wup, inline edits broke PHP.

One more syntax error :)

Dressing this up with a db_table_exists(). We cannot assume a languages table would exist on all databases obviously.

Wups, that patch had no change there... So now adding the db_table_exists() for real.

Moved all update functions to update.inc that affect the language table / negotiation in bootstrap, to make D8 bootstrap ready with the new language list. What we do with the existing update functions is question of #1333898: Support unstable2unstable upgrades? [policy, no patch] (I'm not clear what was/is the decision there). I removed the updates instead of making them no-ops for now. I'll cross-link this issue with that.

Seems like the only remaining fail is about the path prefix not getting properly assigned to non-default English, which I locally tracked down to English not being in the path prefix array even with an empty string. That seems to be due to the fact that now the language list is managed before locale is enabled, so locale cannot just assume that all languages are in the prefix/domain arrays. When locale is enabled, it should prefill those values for all the languages configured. I don't have time to fix that right now, anybody coming around sooner, feel free to grab and do so :)

Patch updated with a locale_enable() that does the prefix/domain data matching as needed when locale is enabled (either the first time or any other time later). It removes prefixes and domains that are for removed languages and adds ones for languages added in the meantime. In case locale is enabled and disabled independently from language module, which is clearly what we want to support.

Submitted #1357918: Missing update for language_default in language langcode update as a followup for this issue to clean up the schema, and hopefully get it to its final D8 form at last for the language list :) Yay. The other followup suggestions seem to be covered with issues already.

So with the update code done and the passing tests (including update tests), we should talk about any review feedback for the patch + whether we want to move tests around here. I think its a huge-huge patch, so maybe we want to do that in a followup too, but comments are absolutely welcome.

Yeah, we don't add any new functionality here, we merely separate what we have to establish a more logical organization for the code. The existing tests should cover what we have. We do need to review if we have any tests that are solely testing the language functionality, and move those here and/or add specific tests for just that and remove some (at that point extraneus) coverage from locale module I think. because we are already above 80k, this looks like doable in a separate patch, since we don't need to add any new tests, its just about reorganizing the tests too.

The update code is in there, the bot would not be green without it now that we have locale in the update schema :) Looking forward for your review! :)

@sun:

- DONE: solved phpdoc

- FOLLOWUP: we don't have pure language tests, discussed above, locale and others test language extensively, so the patch is tested well; in terms of test organization I'd prefer to do it in a followup, this is an 80k+ patch, hard to juggle.

- FOLLOWUP: language count as cache entry, makes sense, agreed on a followup

- DISCUSS: on language_save() and language_uninstall(), they don't really do the same thing; language_save() does not re-run the language initialization process, it merely clears the language cache; langugae_uninstall() does need to rerun the language initialization, because potentially removed language might have been selected before; in fact language_delete() should do that too (it only clears the language_list cache like language_save()); initializing the language system just reruns the initialization of globals, it does not clear the language list cache

- DONE: fixed @file summary

- DONE: renamed list tab

- FOLLOWUP: yes, using entity-like paths might be good here, either is fine

- FOLLOWUP: should drupal_write_record() use db merge on update anyway?

- DONE: using form_load_include()

- DONE: modified phpdoc on locale_enable() to explain this is merely syncing config with the reality of language lists; since languages and negotiation are separate with this patch (they might become one again later, depending on ongoing discussion), it is possible people remove a language and add a couple and we need to sync our config to that; as for accidentally removing language, if you remove your language, nodes in that language will turn to LANGUAGE_NONE, your UI localizations for that language will vanish, etc. so loosing your path prefix config is the least of your issues; looks node module or locale module has no grace for people accidentally removing language; they should keep backups and watch their clicks

- DONE: modified the cache operations as requested

Will post same feedback for @plach's comment and then post the patch, ups, need to change workplaces now, sorry.

@plach:

- DONE: moved language update to its own function

- DISCUSS: right, language is not enabled; I don't yet know how to do that, if we enable the module, we cannot do that in this early in the bootstrap phase I think, we should do it later; however, once we enable the module, the language table schema will be attempted to be recreated and its already there... because calling hook_schema is automated we cannot make it not be called AFAIR; so we need the language table/data early in the bootstrap to be able to bootstrap D8 but we cannot enable a module that early in the bootstrap, can we? and even then how do we skip the schema?

- DISCUSS(?)/FOLLOWUP(?): I changed %name to %language since more existing text used %language. It it also a more universally reusable pattern for other cases where we talk about multiple things. t('%language translation of the post has been deleted') would look odd with %name, right? So just trying to establish a common practice here; we can also use %name in language module but would need to use %language in more general settings, so it looked like it makes sense to be consistent here

- DONE: modified to "Prepare a language code list" vs. "Prepare the language code list", since its not THE language code list, its A language code list

- DONE: removed empty line

- DONE: included locale.inc in locale.enable although I'm not sure its strictly necessary there (its included otherwise already, but it does not hurt to be explicit)

- FOLLOWUP: I'd rather not deal with locale batches here, let's focus on the language module here :)

Great find with update_module_enable(), now using that. Ok on the locale.inc inclusion, thanks for all the manual testing too. For the third point, I removed it because the two submit hooks are now one and its just three lines, so adding yet another layer of functions there did not seem like its worth it for 3 lines. It did make lots of sense when it was two separate submit hooks, now its just a couple lines form_altered in. We can add it back but I don't think its needed.

Ok, well, that function then became locale_translate_add_language_set_batch() and went into bulk.inc, so we need to include that now anyway from the submit function before calling it. Looks better? (We lost two lines in the submit handler and got an API function instead).

Moved the db_table_exists() to the language bootstrap function. I think inside or outside makes equal sense. Left the two nested ifs originally there in anticipation of other such changes needed with more ifs on the same level.

On the module.inc problem, maybe this function is not intended for this use, but as far as I can think about this, what it does makes quite lot of sense to be used in this context. Except module.inc not yet being available. The function used only uses drupal_static and cache related things, which are bootstraped by the time it is called (the bootstrap fix code bootstraps to the DB level), so just adding include for module.inc could fix this for good.

Let's look at this one!

What if we use system_rebuild_module_data() instead first? Based on code review, it seems to be runnable with just a DB bootstrap. What do you think?

@catch, @plach: ok, well, this is getting a bit out of scope here... At the stage where this code fix needs to run, we do *know* that the language module is not yet in the system table, so we can just insert it, no? Or we should track down how the core/ path change is implemented in regards to the system table, and get that run earlier and get it include the newly found modules too? This should either be simple code or code already done AFAIS.

Talked to @catch in IRC, he underscored we should fix this generally here. Ok, well, here is an updated patch which copies the code that looks like is needed and reuses functions that look like are fine as-is. This is obviously pretty fragile to change in the future, since it uses functions from all over file.inc, common.inc, system.module, etc. Any non-update compatible changes in those functions would break the update. I've code reviewed the function tree involved and they seem to have no code that would stop them working on the update, but more sets of eyes would be needed to double check (and test).

Attached an interdiff and the complete patch.

Lol, yeah, it would be great if we could improve Drupal with patches of 0 bytes. Here is the big boy instead.

As is evident from the test results, query.inc is attempted to be loaded from the (old) D7 place, however, it moved to under core/* like the rest of the files. We need to rebuild the registry to be able to rebuild the module list because the module list rebuild code uses query.inc. Or we could alternatively deviate more in our custom update.inc version of the module rebuild, but I think we want to keep it consistent as much as possible with the "live" version of the code. The only change in this patch is in this hunk:

diff --git a/core/includes/update.inc b/core/includes/update.inc
index 289d27a..285038e 100644
--- a/core/includes/update.inc
+++ b/core/includes/update.inc
@@ -100,6 +100,61 @@ function update_prepare_d8_bootstrap() {
         'description' => $has_required_schema ? '' : 'Please update your Drupal 7 installation to the most recent ve
rsion before attempting to upgrade to Drupal 8',
       ),
     );
+    if ($has_required_schema) {
+      // Rebuild registry and module list to be able to bootstrap. The registry
+      // needs to be rebuilt first because rebuilding the module list requires
+      // DB classes, which will not be found based on the D7 registry given
+      // the moved files.
+      registry_update();
+      update_system_rebuild_module_data();
+
+      // Update the environment for the language bootstrap if needed.
+      update_prepare_d8_language();
+    }
+  }
+}

Ok, feedback please!

Got some feedback from catch on IRC that I should instead do what I originally wanted and just insert the language module in the system table instead of trying to invoke a family of functions that we need to duplicate just to get the exact right value. The system table is refreshed later at the end of the update anyway. Here is an interdiff (custom update versions of functions removed, custom SQL code against system table added) and the full patch.

#59: language-module-59.patch queued for re-testing.

Don't know why it could not install. This was the testbot output:

[06:39:10] Invoking operation [install]...
[06:39:12] Encountered error on [install], details:
array (
  '@reason' => 'Installing: failed to complete installation by setting admin username/password/etc.',
)
[06:39:12] Review complete. test_id=202938 result code=7 details=Array
(
    [@reason] => Installing: failed to complete installation by setting admin username/password/etc.
)

I can install locally, so asked for a retest.

And it does pass tests now for the second round, it must have been a testbot glitch. Reviews please!

@plach: well, it would only actually make the module installed if you also call update_module_enable() at the end. I agree such a utility function will be useful.

Here is an updated patch (and interdiff) for the hook_help() improvement and the new added API for module installs.

@plach: if the schema version is -1 and/or the status is 0, the module is not installed :) If the schema version is -1 for a module, it was never installed on this Drupal site before. Maybe you are looking for an update_module_insert_in_system_table()? :) Install would be a misleading name entirely if the module is not in fact installed.

Ok, update_module_add_to_system() then. If someone has a better idea please let me know. I think its pretty much unrelated to what we actually do in this patch and people can rename it later, so IMHO we should not obsess much with it :) It will be used pretty sparingly I'd hope/guess.

I'd prefer to have an easier to read code base for these run-once update functions compared to abstracting them to perfection, since upgrades are a critical part of Drupal that people struggle with sometimes. Anyway, I made the $modules = array() addition. I don't think your proposed multi-insert syntax would work, at least the docs for multi-insert say we should call fields() with a list of fields, and then repeatedly call values() (not fields()) with the values for those fields. All-in-all I think that also makes this code less readable for a performance increase that would probably be negligible. Fixed the comment wrapping as well.

I also went in and added a language.test based on the first test case in locale.module. It tests adding a predefined language, a custom language, changing default languages, enabled/disabled status, removing language (including English) etc. So it basically covers the whole UI of the language module. I cut down on this original test in locale module and instead concentrated there to the negotiation path prefix autoconfiguration that is happening there, so it got much better test coverage there now. This also surfaced a bad condition in language_list() on locale module, so it was a great thing to do here after all. (Not as a followup). I think this should satisfy @sun's concerns from above and prove that the language module works in itself as well. We'll continue with cleaning up the related pieces in followup patches as noted above, so let's get another look on this and get it in! (It is past 100k with the tests, eh).

Thanks!

The permission is merely moved from locale module, see the locale.module hunks... It is not at all new.

@Dries: I can assure you that establishing a standalone language module did not come out of thin air. There is a very detailed conversation about how to organize language functionality in #1293304: Break up locale.module, but how?, which has a 100+ discussion history of the past three months. I suggest you look at especially the use cases outlined in http://drupal.org/node/1293304#comment-5053640 and below, with real Drupal site examples and functionality breakdowns showing among others, sites where language assignment functionality is all you need. I also have detailed examples for why it is not possible to have a one-stop module solution for "the language problem" at http://drupal.org/node/1293304#comment-5329604.

There is plenty of feedback all around on that issue and in related discussions and the general feedback was that people don't want yet another required core module for them. This was discussed more at the start of that discussion, eg. see catch's summary in http://drupal.org/node/1293304#comment-5049546. In short, people suggested if we make it required, we make it so it use lazy loaded classes and other great new things that we still don't have an infrastructure for in Drupal 8 yet. That could be a fine next step to improve the UX even more later and I don't believe we should block this improvement on that.

Added http://drupal.org/node/1387546.

Followups being worked on:

- #1387586: Rename drupal_multilingual() to language_multilingual() (IMHO Novice, need to figure out the best name :)
- #1387608: Unify language_list() and locale_language_list() (badly needed, removes one of the two remaining language object related functions from locale, IMHO Novice again)
- #1357918: Missing update for language_default in language langcode update
- #1357912: Convert path language code schema to langcode (related to above)

See you at these places!

Tagging for base language system.

Minor followup: we left the API docs in locale.api.php too when the ones were moved to language.api.php. The locale.api.php items should be removed. See #1470080: Nothing in locale.api.php belongs there.