As per #1216094: We call too many things 'language', clean that up we are converting schemas and parallel APIs to use 'langcode' instead of 'language' where a language code is involved. The use of 'language' inconsistently for language objects and language codes in Drupal 7 keeps throwing people off the cliff and makes understanding the API much harder. The attach patch is for path module and is a possibly rough start. Let's see how it fairs with a complete test suite.
No upgrade path yet.
Comment | File | Size | Author |
---|---|---|---|
#19 | drupal8.path-language-langcode.19.patch | 29.96 KB | sun |
#18 | path-language-to-langcode-18.patch | 30.06 KB | Gábor Hojtsy |
#14 | path-language-to-langcode-14.patch | 30.06 KB | Gábor Hojtsy |
#12 | path-language-to-langcode-12.patch | 29.97 KB | Gábor Hojtsy |
#10 | path-language-to-langcode-10.patch | 29.97 KB | Gábor Hojtsy |
Comments
Comment #1
Gábor HojtsyTagging.
Comment #3
sunCleaning up bogus/duplicate "API cleanup" term.
Comment #4
Gábor HojtsyAdded update code in update.inc to fix bootstrap test failure.
Comment #6
Gábor HojtsyMore langcode misses in tests.
Comment #8
Gábor HojtsyMore langcode misses in locale.test and making the update conditional on existence of url_alias.
Comment #10
Gábor HojtsyThe path.test that breaks is under simpletest/tests, huh, this was misleading :) Fixes for those too.
Comment #12
Gábor HojtsyA tiny language fallback error in drupal_lookup_path() caused many tests to fail. With a one char change there we should be in much better shape :)
Comment #14
Gábor HojtsyThe update.inc bootstrap preparation code can run in multiple requests through the update, so we need to be more specific about testing for the right conditions, the existence of the url_alias table and the language column too. Also, the index creation in the update was not correct, now it is created correctly there too. This should now pass all tests.
Comment #15
Gábor HojtsyTagging up so we can find the langcode API changes later.
Comment #16
Gábor Hojtsy#14: path-language-to-langcode-14.patch queued for re-testing.
Comment #17
Gábor Hojtsy#14: path-language-to-langcode-14.patch queued for re-testing.
Comment #18
Gábor HojtsyPatch did not apply anymore due to #1357918: Missing update for language_default in language langcode update landing. Minor change required to apply again. No other changes.
Comment #19
sunAs with #1357918-35: Missing update for language_default in language langcode update, I'd like to see a regular database update here.
Attached patch
1) moves the schema change into a regular system module update
2) tweaks drupal_lookup_path() to not look up any URL aliases in MAINTENANCE_MODE (which is long overdue).
Comment #20
Gábor HojtsyWell, that maintenance mode chnange really has far reaching effects. Eg, if you put your site into maintenance mode, and your menu links would suddenly change. Let's do that in its own issue, not under the covers of language API changes unless it is explicitly related.
Comment #21
Gábor HojtsyIn other words, the
(db_table_exists('url_alias') && db_field_exists('url_alias', 'language'))
condition in update_prepare_d8_bootstrap() would have done fine for a D8->D8 update just as well (it has a condition passing fine on existing D8 sites) without introducing side effects for when a site goes to maintenance mode to not use any path aliases. I think that is leaping to conclusions too fast there.Comment #22
sunNote that
MAINTENANCE_MODE
!=variable_get('maintenance_mode')
.MAINTENANCE_MODE
is only defined in install.php, update.php, and authorize.php.Without this change, drupal_lookup_path() blows up with a PDOStatement exception right on the first update.php page, since drupal_path_initialize() as well as as any call to url() attempts to query {url_alias} for any url() being used in the update script, but the 'langcode' column does not exist yet. Thus, you don't even reach the stage of executing module database updates.
Comment #23
Gábor HojtsyYes, thats why it was in the bootstrap prepare update function, right? I think both methods are valid to some extent. Also both supply a D8 -> D8 upgrade path as explained above. But your solution has side effects (although not to the extent that I originally understood).
Comment #24
Gábor HojtsyOk, well, either way works technically. Catch/Dries will choose from the two for their liking.
Comment #25
Dries CreditAttribution: Dries commentedI committed Gabor's patch in #18 to 8.x because sun's patch in #19 may have unintended side effects (according to Gabor).
I'm marking this issue 'needs work' though, so we can continue to discuss the changes proposed by sun and follow-up if necessary.
Comment #26
Gábor HojtsyAdded changelog at http://drupal.org/node/1413790.
Comment #27
Gábor HojtsyMoving to fixed due to lack of discussion.