I assume that we are now in code cleanup phase. I use PhpStorm to find possible bugs and problems in the Drupal 8 core. Normally I will not alter code flow. If I find problems in a file, which I will not fix, but find suspicious. I'll mention it in the text here.
Checked: update.inc file.
I found two problems in that file. One which I tried to fix and a second which may be tolerated.
The first is this:
$modules = drupal_get_installed_schema_version(NULL, FALSE, TRUE);
foreach ($modules as $module => $schema_version) {
// Skip uninstalled and incompatible modules.
if ($schema_version == SCHEMA_UNINSTALLED || update_check_incompatibility($module)) {
continue;
}
the function drupal_get_installed_schema_version() may return an integer. And an integer is not a correct type to do a foreach. Therefore I added an if condition to prevent that here, because it would fail with an error. If that is not intended a throw may be better.
Secondly:
function update_retrieve_dependencies() {
$return = array();
// Get a list of installed modules, arranged so that we invoke their hooks in
// the same order that module_invoke_all() does.
foreach (Drupal::keyValue('system.schema')->getAll() as $module => $schema) { // <-- outer $module
if ($schema == SCHEMA_UNINSTALLED) {
// Nothing to upgrade.
continue;
}
$function = $module . '_update_dependencies';
// Ensure install file is loaded.
module_load_install($module);
if (function_exists($function)) {
$result = $function();
// Each implementation of hook_update_dependencies() returns a
// multidimensional, associative array containing some keys that
// represent module names (which are strings) and other keys that
// represent update function numbers (which are integers). We cannot use
// array_merge_recursive() to properly merge these results, since it
// treats strings and integers differently. Therefore, we have to
// explicitly loop through the expected array structure here and perform
// the merge manually.
if (isset($result) && is_array($result)) {
foreach ($result as $module => $module_data) { // <-- inner $module
foreach ($module_data as $update => $update_data) {
foreach ($update_data as $module_dependency => $update_dependency) {
// If there are redundant dependencies declared for the same
// update function (so that it is declared to depend on more than
// one update from a particular module), record the dependency on
I got a warning for that piece of code, that two foreach loops which are cascaded are using the variable name $module. Which may result in data loss or unexpected results. This seems also acceptable in the case. I do not like that and it makes it harder to understand.
| Comment | File | Size | Author |
|---|---|---|---|
| #1 | drupal_core-code_clean_up_update_inc-1965406.patch | 8.19 KB | ro-no-lo |
Comments
Comment #1
ro-no-lo commentedComment #2
enhdless commentedPatch failed to apply, it should be rerolled.
Comment #10
andypostThis file will be removed #3012523: Convert the update.inc file functions to a class