[PHP 7] Function each() is deprecated since PHP 7.2 [D7]

PHP 7.2 is finally released, and for anyone coming to this issue, I can confirm the patch above works well, except for one case.
In the menu.inc _menu_load_objects function, the $function variable is overwritten with the key of the same variable $function. This makes the next current call to fail because $function is now a string. I also removed the next() call because the code down there was not iterating.

I attached a patch against the current 7.x/6336b7177 plus an interdiff with patch at #3.

Hi.

After applying the patch I am getting a lot of warnings:

Warning: current() expects parameter 1 to be array, null given in _menu_load_objects() (line 579 in /var/www/html/includes/menu.inc).

Notice: Undefined variable: function_map in _menu_load_objects() (line 580 in /var/www/html/includes/menu.inc).

Warning: key() expects parameter 1 to be array, null given in _menu_load_objects() (line 580 in /var/www/html/includes/menu.inc).

Warning: Invalid argument supplied for foreach() in _menu_load_objects() (line 584 in /var/www/html/includes/menu.inc).

Warning: array_unshift() expects parameter 1 to be array, null given in _menu_load_objects() (line 600 in /var/www/html/includes/menu.inc).

Warning: call_user_func_array() expects parameter 1 to be a valid callback, no array or string given in _menu_load_objects() (line 601 in /var/www/html/includes/menu.inc).

Warning: call_user_func_array() expects parameter 2 to be array, null given in _menu_load_objects() (line 601 in /var/www/html/includes/menu.inc).

The patch at #5 was supposed to fix that current() call error, but it turns out I uploaded the wrong patch. Please undo that patch and try the new one attached.

Some review comments for patch #8.

1. +++ b/includes/bootstrap.inc
   @@ -3776,7 +3776,7 @@ function _drupal_shutdown_function() {
   +    foreach($callbacks as $key => $callback) {
   

   Coding standards: Add a whitespace between the foreach statement and the open parenthesis '('.

2. +++ b/includes/install.inc
   @@ -779,7 +779,7 @@ function drupal_uninstall_modules($module_list = array(), $uninstall_dependents
   +    foreach($module_list as $module) {
   

   This introduces a bug. The $module variable is expected to contain the module name. Since the $module_list contains an array with module name as key and weight as value, the variable $module contains the weight. This line should be like foreach ($module_list as $module '=> $weight) {

   Coding standards: Add a whitespace between the foreach statement and the open parenthesis '('.

3. +++ b/includes/module.inc
   @@ -404,7 +404,7 @@ function module_enable($module_list, $enable_dependencies = TRUE) {
   +    foreach($module_list as $module) {
   

   Same as above. No proper coding standards and $module contains the weight instead of the module name.

4. +++ b/includes/module.inc
   @@ -540,7 +540,7 @@ function module_disable($module_list, $disable_dependents = TRUE) {
   +    foreach($module_list as $module) {
   

   Same as above. No proper coding standards and $module contains the weight instead of the module name.

Should we also change the use of each() in the book.module module file? Function each() is still used twice in that file.

Makes sense the simpletest module couldn't be enabled at the CI tests.

Did some testing and it seems like we should use next() and key()/current() instead of a foreach loop, since new values are added to the module list array while iterating over it.

For example:

$list = array('1', '2', '3');
while (list($key, $val) = each($list)) {
  echo $val;
  if ($val == '2') {
    $list[] = '4';
  }
}

Will result in 1234

$list = array('1', '2', '3');
foreach ($list as $key => $val) {
  echo $val;
  if ($val == '2') {
    $list[] = '4';
  }
}

Will result in 123

You can test this at 3v4l.org: https://3v4l.org/g77Ae

Did some cleanup and implemented the key() / next() loop.

Btw using iterators is nice but has reset() as requirement sometimes

--- a/includes/bootstrap.inc
+++ b/includes/bootstrap.inc
@@ -3776,7 +3776,7 @@ function _drupal_shutdown_function() {
   chdir(DRUPAL_ROOT);
 
   try {
-    while (list($key, $callback) = each($callbacks)) {
+    foreach ($callbacks as $callback) {
       call_user_func_array($callback['callback'], $callback['arguments']);
     }
   }

We should also use current() and next() here, since the $callbacks array is a drupal_static() variable which be can be altered

Updated patch #16 so callbacks are also looped with current() and next().

Looks still needs reset()

Added resets

Adding back the changes in #20 to fix tests

Looks good to go!

I get the error:
Deprecated function: The each() function is deprecated. This message will be suppressed on further calls a _drupal_shutdown_function() (línia 3779 de /var/local/html/dadesobertes/includes/bootstrap.inc).

Applied the patch #24 and works with PHP 7.2.1

Patch still aplies cleanly:

$ git apply -v deprecated_each_function_drupalcore-2925449-23.patch
Checking patch includes/bootstrap.inc...
Checking patch includes/install.inc...
Checking patch includes/menu.inc...
Checking patch includes/module.inc...
Applied patch includes/bootstrap.inc cleanly.
Applied patch includes/install.inc cleanly.
Applied patch includes/menu.inc cleanly.
Applied patch includes/module.inc cleanly.

I have found three extra occurrences in current code:

\modules\book\book.module:
  773    do {
  774      $prev = $curr;
  775:     list($key, $curr) = each($flat);
  776    } while ($key && $key != $book_link['mlid']);
  777  
  ...
  809    // Assigning the array to $flat resets the array pointer for use with each().
  810    do {
  811:     list($key, $curr) = each($flat);
  812    }
  813    while ($key && $key != $book_link['mlid']);

\modules\locale\locale.test:
 3190          $negotiation = variable_get("language_negotiation_$type", array());
 3191          $equal = count($info['fixed']) == count($negotiation);
 3192:         while ($equal && list($id) = each($negotiation)) {
 3193:           list(, $info_id) = each($info['fixed']);
 3194            $equal = $info_id == $id;
 3195          }

Some review comments:

1. +++ b/modules/book/book.module
   @@ -772,7 +772,8 @@ function book_prev($book_link) {
      $curr = NULL;
      do {
        $prev = $curr;
   ...
   +    $curr = current($flat);
   +    next($flat);
      } while ($key && $key != $book_link['mlid']);
   

   Assigment of $key is missing. Should be $key = key($flat);

2. +++ b/modules/book/book.module
   @@ -808,7 +809,8 @@ function book_next($book_link) {
      // Assigning the array to $flat resets the array pointer for use with each().
      do {
   -    list($key, $curr) = each($flat);
   +    $curr = current($flat);
   +    next($flat);
      }
      while ($key && $key != $book_link['mlid']);
   

   Assigment of $key is missing. Should be $key = key($flat);

Oh! Extra:
The comment should be changed:
// Assigning the array to $flat resets the array pointer for use with each().

And $curr is not used, we really doesn't need the var, only need the key.

Looks like there's still one left:

diff --git a/modules/locale/locale.test b/modules/locale/locale.test
index db87e05548..c54259165f 100644
--- a/modules/locale/locale.test
+++ b/modules/locale/locale.test
@@ -3190,7 +3190,7 @@ class LocaleLanguageNegotiationInfoFunctionalTest extends DrupalWebTestCase {
         $negotiation = variable_get("language_negotiation_$type", array());
         $equal = count($info['fixed']) == count($negotiation);
         while ($equal && list($id) = each($negotiation)) {
-          list(, $info_id) = each($info['fixed']);
+          $info_id = current($info['fixed']);

So the patch fixes one of the each() calls in that block of code, but still leaves the first one behind (while ($equal && list($id) = each($negotiation)) ).

--

In general, and maybe this is just personal preference, I have to say that I find foreach() (with a break statement if necessary) way more readable than next(), current(), etc... anyone else? Not a commit blocker from my point of view, but if it were me I'd try to use foreach() whenever possible.

Not a commit blocker from my point of view, but if it were me I'd try to use foreach() whenever possible.

Well actually, it also looks like the Drupal 8 patch at #2885309: [PHP 7.2] each() function is deprecated did use foreach() for a number of these... seems like we should at least use foreach() whenever that did if possible to do so; there isn't much reason to have the Drupal 7 and 8 fixes diverge unnecessarily.

Thanks for the work on this issue!

+1 for the foreach() preference. I'll try to re-roll the patch to a (hopefully) final one using foreach() whenever possible if the patch at #32 passes.

OK! Reroll! @Ayesh

For fixing test in do/while style do this:

+++ b/modules/locale/locale.test
@@ -3189,9 +3189,11 @@ class LocaleLanguageNegotiationInfoFunctionalTest extends DrupalWebTestCase {
       if (isset($info['fixed'])) {
         $negotiation = variable_get("language_negotiation_$type", array());
         $equal = count($info['fixed']) == count($negotiation);
-        while ($equal && list($id) = each($negotiation)) {
-          list(, $info_id) = each($info['fixed']);
+        while ($equal && $id = key($negotiation)) {
+          $info_id = current($info['fixed']);
           $equal = $info_id == $id;
+          next($negotiation);
+          next($info['fixed']);
         }
         $this->assertTrue($equal, format_string('language negotiation for %type is properly set up', array('%type' => $type)));
       }

but if you want to refactor in a foreach/break style I agree that it's more rediable.

For the locale.test one in particular, I have to say the Drupal 8 fix looks particularly enticing: http://cgit.drupalcode.org/drupal/commit/core/modules/language/tests/src...

It doesn't have to use foreach() or next()/current()/etc .... instead it just drastically simplifies the code :) Pretty sure we could do that here too.

count() will also fail if the variable is not countable. This patch should also address it; based off of #40.

Tested the latest patch #42, the line 1441 in form.inc cause all required fields to fail validation.

$is_empty_multiple = (!is_array($elements['#value']) || !count($elements['#value']));

if it's not an array the first condition return TRUE, thus causing $is_empty_multiple to be TRUE and mark the field in error.
attached updated patch

edit: not all fields fail validation, only non-multiple fields that are #required.

So regarding the core original code it seems that:

$is_empty_multiple = (!count($elements['#value']));

was doing more than just detect empty multiple fields.

edit: the patch in #42 address more fix to count() calls, they need to be reviewed too.

We might have to do more than a one-liner to fix the $is_empty_multiple issue.

Going to silence the error on $is_empty_multiple to see if there is anything else causing issues.

+++ b/modules/book/book.module
@@ -806,9 +807,9 @@ function book_prev($book_link) {
-    list($key, $curr) = each($flat);
+    $key = key($flat);
+    next($flat);

current is lost

$curr is lost in book_next() because is not used, see all function.

function book_next($book_link) {
  $flat = book_get_flat_menu($book_link);
  do {
    $key = key($flat);
    next($flat);
  }
  while ($key && $key != $book_link['mlid']);

  if ($key == $book_link['mlid']) {
    return current($flat);
  }
}

In book_prev() we have $curr because is used:

function book_prev($book_link) {
  // If the parent is zero, we are at the start of a book.
  if ($book_link['plid'] == 0) {
    return NULL;
  }
  $flat = book_get_flat_menu($book_link);
  $curr = NULL;
  do {
    $prev = $curr;
    $curr = current($flat);
    $key = key($flat);
    next($flat);
  } while ($key && $key != $book_link['mlid']);

  if ($key == $book_link['mlid']) {
    // The previous page in the book may be a child of the previous visible link.
    if ($prev['depth'] == $book_link['depth'] && $prev['has_children']) {
      // The subtree will have only one link at the top level - get its data.
      $tree = book_menu_subtree_data($prev);
      $data = array_shift($tree);
      // The link of interest is the last child - iterate to find the deepest one.
      while ($data['below']) {
        $data = end($data['below']);
      }

      return $data['link'];
    }
    else {
      return $prev;
    }
  }
}

In your comment says that we miss current in book_prev but really this is missing in book_next, that it's correct:

I have rerolled a patch based on #40 because this issue is about to each deprecation, not count() issues with PHP 7.2

If we want to solve count() bugs and deprecations we can manage this in another issue.

Sounds good. I'd RTBC this.

For count() I re-purposed issue #2932916: [PHP7] Warning: count(): Parameter must be an array or an object that implements Countable in PHP 7.2

I don't understand why the reset() was added all over. That will reset the internal pointer of the array to the first element, but it's not as though each() was doing that?

In other words, adding the reset() will reset the array to start whereas before this patch, that was not happening.

After reading the docs I'm convinced it's not appropriate to add the resets(). But maybe someone can explain why they are needed?

@bkosborne, please provide a new patch with your recommended changes.

@bkosborne

1. reset works
2. tests pass
3. manual testing also pass
4. without reset doesn't work and tests doesn't pass
5. the reset arrays are only used in each, arrays are not reused later and need to be reset for working in each context
6. there is no performance reasons
7. we use reset in other parts of core
8. I'm using this patch in a PHP 7.2 server without any related problem

Surely we can use foreach structures, and maybe is more redeable.

I'm not interested in working on a new patch when we already have a solution that works, but if you want to provide a new patch using foreach style I will be glad to review it. In the meantime, if nobody wants to work in an alternative patch, I would appreciate that doesn't block the current solution.

Sounds good to me +1 for RTBC. using this solution also all test passes.

It is nice that patch 58 passes, great work, however @DavidRothstein points out in comment #39 that the D8 fix in one case was very simple and yet not one replied to him #2925449-39: [PHP 7] Function each() is deprecated since PHP 7.2 [D7]

And he also had a suggestion in comment #35 to approach this in a similar way that D8 did with foreach #2925449-35: [PHP 7] Function each() is deprecated since PHP 7.2 [D7]

This will make it easier to maintain.

Meanwhile I used the patch #58 to test out #2947772: Fully support PHP 7.2 in Drupal 7
I am not sure why but when I combined this patch with the other one, there is one obscure fail, probably not caused by this patch but if anyone already has 7.2 working in their environment please have a look at 2947772 thanks.

I'll address #65

Ok, here's my changes.

There appeared to be some bugs in the patch.

I replaced iterators with foreach loops where possible (and this is how the D8 version of the patch did it). This also fixed two bugs in the previous patch in the module_enable and module_disable code where the next() iterator was not called properly.

I also added some reset() calls to the book module where they were missing. The D8 version of patch did this as well.

The code in menu.inc seemed wrong too. I removed the two reset() calls since I don't think they belonged there.

Will look at those test failures today

Ok I think the module installer / uninstaller are failing now because when we switched to a foreach(), but it looks like the installer is modifying the array in place, which I don't think behaves the same way with a foreach as it does with an each(). But if that's the case, then I think this may be broken in D8 as well and maybe there is just more coverage in D7. Will continue looking into it.

@bkosborne I have already mentioned this in comments #19 and #20. That is why I used reset() and next() instead of a foreach loop.

Same for module_enable and module_disable. While these methods can enable/disable dependend modules

@sjerdo, Indeed. I went back to the manual iterator that you added. I made one small change, which is putting the next() as the first line in the loop instead of the last. This more closely resembles what each() was doing (it immediately advances the array pointer) and avoids a potential issue where the next() wouldn't be called when the continue; code path is hit, forcing it to stay on the same item.

I also removed one extra reset that doesn't appear necessary.

The interdiff is still diffed against the patch in #55.

Fix that last failing test by keeping the loop on reference for system shutdown function

Great work everyone, I think this can be RTBC on patch #74.

@bkosborne, and for everyone else, I've taken patch 74 and used it for 2947772 there's still one 7.2 fail , I took patch #74 (great work btw) and combined it with the other 7.2 patch , unfortunately there's still one failure but I don't think it's related to this issue.
Please see #2947772-5: Fully support PHP 7.2 in Drupal 7
and the test fail.

Apologies if questions I have, have been answered before in the discussion:

1. +++ b/includes/bootstrap.inc
   @@ -3776,7 +3776,7 @@ function _drupal_shutdown_function() {
   -    while (list($key, $callback) = each($callbacks)) {
   +    foreach ($callbacks as &$callback) {
   

   What is the reason to use a reference here in the loop?

   Is that necessary to keep some BC somewhere?

2. +++ b/includes/install.inc
   @@ -779,7 +779,7 @@ function drupal_uninstall_modules($module_list = array(), $uninstall_dependents
   -    while (list($module) = each($module_list)) {
   +    foreach (array_keys($module_list) as $module) {
   

   I assume we checked that only the current module would be unset in this function, so that we could not run into the situation to visit an already unset $module?

3. +++ b/includes/menu.inc
   @@ -2402,7 +2403,8 @@ function menu_set_active_trail($new_trail = NULL) {
   -      list($key, $curr) = each($tree);
   +      $curr = current($tree);
   +      next($tree);
   
   @@ -2432,7 +2434,8 @@ function menu_set_active_trail($new_trail = NULL) {
   -      list($key, $curr) = each($tree);
   +      $curr = current($tree);
   +      next($tree);
   

   I assume $key was unused here?

4. +++ b/includes/module.inc
   @@ -404,7 +404,11 @@ function module_enable($module_list, $enable_dependencies = TRUE) {
   +    // The array is iterated over manually (instead of using a foreach) because
   +    // modules may be added to the list within the loop and we need to process
   +    // them.
   

   Nice comment.

5. +++ b/modules/book/book.module
   @@ -768,11 +768,13 @@ function book_prev($book_link) {
      do {
        $prev = $curr;
   -    list($key, $curr) = each($flat);
   +    $curr = current($flat);
   +    $key = key($flat);
   

   $curr is unused here and not used below. Is it needed later?

6. +++ b/modules/locale/locale.test
   @@ -3188,11 +3188,7 @@ class LocaleLanguageNegotiationInfoFunctionalTest extends DrupalWebTestCase {
   +        $equal = array_keys($negotiation) === array_values($info['fixed']);
   

   Nice job!

Addressing comments and will add new patch soon..

EDIT: Oops, used a .patch extension on the interdiff. Ignore the obvious test failure that will produce.

1. Good question. This confused me initially so I removed it, but it (thankfully) caused tests to fail (specifically, "testShutdownFunctions" in system.test).

The original code looped over the shutdown functions using a manual iterator, which meant that shutdown functions added to the list by other shutdown functions would still be executed (that's a benefit of using a manual iterator as I've come to learn). But we changed it to a foreach, which operates on a copy of the array, so additional shutdown functions that were registered in the process of executing other shutdown functions were never called.

I guess you can force the foreach loop to NOT create a copy of the array by declaring that it's items should be referenced (like the patch shows). This is how the D8 version of patch did it so I copied it. But, it's not at all clear that this is the reasoning for the reference, so I'll convert this back to a manual iterator and add a comment.

2. I don't understand the scenario you're describing, can you elaborate?

3. That's right.

5. It is used, but in a round-a-bout way. It's used to help figure out what the $prev should be when the loop exits.

---

Attached patch addresses the first comment.

The failures in PHP 7.2 are unrelated to the work in this patch.

Great work, however the simpletest is the true review.
Patch #51 has only 1 failure with php 7.2 ,
however patch #80 has 308 fails with php 7.2

how can this not be related?
why is patch #51 'almost' ready for php 7.2 , but patch #80 has 308 fails?

I notice that the patch #51 has only 1 fail, the same issue I was trying to resolve in #2947772: Fully support PHP 7.2 in Drupal 7
For the 1 fail, follow that up in 2947772

Thanks,

I sent #51 for a re-test on PHP 7.2 to see if the includes/form.inc error is related.

---

Using a reference is a good trick / side effect, however I like the explicit iterator more, so good for changing.

I think this can be changed back to RTBC and then committed - provided that #51 has the same number of test failures.

Hmm, nope, it had just 1 failure as before. Not sure what to make of that massive difference. Will try and look at this again tonight.

Suppose there are differences between #51 and #80 because patch #51 contain also fix to 'count' function...

yes, patch #80 should be RTBC provided test results match those of patch #55,
#87 is right the count issue was split to another issue, #80 can be compared with patch #55 , (I just queued it for php 7.2, expect the same 308 fail result)
also see comments #51, #52, #53, #54

For combining the 7.2 compatibility fixes, see:
#2947772: Fully support PHP 7.2 in Drupal 7

The previous patches were written against 7.56
appears that they no longer apply cleanly to 7.57.
patch 80 looks like it is correct. The 308 php 7.2 fails are out of scope.

If PHP 5.3 passes in #80 I'd RTBC this.

This needs a re-roll ...

Reroll of patch #80

testbot glitch, was choking on the interdiff added in comment #80, because of the .patch extension instead of .txt

hiding the interdiff.patch , this time the patches should apply cleanly

I will requeue patch #80 for php 5.3

Patch #80 is good, the fails are due to some glitch in the testbot.

For testrunner issue, followup here:
#2916566: d7 contrib Testrunner Builds not working properly with stable

Patch #80 is fine for me as well, including it in our composer file by default.

Retested all of the failing patches.

Thanks Mixologic!
Passes all but 7.2 which will be fixed in #2947772: Fully support PHP 7.2 in Drupal 7

I am happy with the patch now :).

Just not at a computer to commit it.

Im getting the issue

Deprecated function: The each() function is deprecated. This message will be suppressed on further calls in _menu_load_objects() (line 579 of C:\Apache24\htdocs\drupal\includes\menu.inc).

I saw the above patches, but as I am new to drupal, I do not hv idea on to apply these patches. Pls help me here.

Hi,

A simple Google query with 'Drupal how to apply patches" and you would have find the solution quicker than posting the message :-)

Anyway, here's the link that will help you: https://www.drupal.org/patch/apply

Thanks a lot Pol. It helped.
But can figure out why I m getting this. Completely new o drupla. :)

C:\Apache24\htdocs\drupal\includes>git apply -v deprecated_.patch

includes/deprecated_each_2925449-80.patch:10: trailing whitespace.
// Manually iterate over the array instead of using a foreach loop.
includes/deprecated_each_2925449-80.patch:11: trailing whitespace.
// A foreach operates on a copy of the array, so any shutdown functions that

includes/deprecated_each_2925449-80.patch:12: trailing whitespace.
// were added from other shutdown functions would never be called.
includes/deprecated_each_2925449-80.patch:13: trailing whitespace.
while ($callback = current($callbacks)) {
includes/deprecated_each_2925449-80.patch:15: trailing whitespace.
next($callbacks);
Skipped patch 'modules/book/book.module'.
Skipped patch 'modules/locale/locale.test'.
Checking patch includes/bootstrap.inc...
error: while searching for:
chdir(DRUPAL_ROOT);?
?
try {?
while (list($key, $callback) = each($callbacks)) {?
call_user_func_array($callback['callback'], $callback['arguments']);?
}?
}?
catch (Exception $exception) {?

error: patch failed: includes/bootstrap.inc:3776
error: includes/bootstrap.inc: patch does not apply
Checking patch includes/install.inc...

Reroll post-7.58 release.
@devu_divya: Try this new patch against the latest 7.x-dev version. Patches in issue queues are always made against the dev version of core or the contrib project.

Crack open a bottle of bubbly! php 7.2 now passing all tests.
Great work everyone!
See:
#2947772: Fully support PHP 7.2 in Drupal 7

ready for commit

Awesome work on getting the patch to pass!

Not so fast ....

I had to roll back to PHP7.1, as I was getting fatal errors on Drupal 7 sites with 7.2

An AJAX HTTP error occurred. HTTP Result Code: 500 Debugging information follows. Path: /batch?render=overlay&id=809&op=do StatusText: error ResponseText:"

Following "Continue to the error page", I have this: "Fatal error trying to download."

Drush updating not a problem, though.

@silor #109
That is a different issue. Apply the patch in #2946045: Unable to update modules due to Archive_Tar incompatibility with PHP 7.2

@sjerdo
Sorry, and thanks. Saw loads of these errors too, but yes; different issue.

Hello all, would appreciate any help you could provide. I installed Drupal 7 on my Win server running PHP 7 and MySQL 5. My search for the errors I received after the installation stated it completed lead me to this forum. I am currently trying to follow the wonderful guide here https://www.drupal.org/node/620014 to install this patch.

So far my question is, where should I have the patch file located in my website directory? The guide states within the module folder but these errors and patch if I am understanding correctly needs to be applied to the Drupal core files. This is a totally new endeavor for me. I worked in web hosting years ago and all these wonderful CMS systems were either 1 click installs or already installed. Now that I am doing this from scratch on my own development server I am having to relearn so much. Thank you again for any assistance you could provide.

@ljndit
I will answer your question, but the issue queue is not really the place for such questions, and even especially not in a "bug report".
You should either:
1) Ask on the forums, see https://www.drupal.org/forum
2) On the Slack #support channel, see https://www.drupal.org/slack
3) In a new issue of your own creation under the category "support request" (not very likely to get an answer, the other two options are preferred)

Anyway about your question, you apply module patches from within the corresponding module folder (as you mentioned already), patches to Drupal itself are performed from the root web directory, i.e. the folder with Drupal's index.php file.

Committed and pushed to 7.x. Thanks all!

I would like to apply this patch to my existing D7 site, since I am also affected by PHP 7.2.

I've noticed that the fix has been pushed to 7.x, but 7.59 is still the latest, so this thing isn't out yet.

Can someone kindly tell me how to apply a patch to a site, and which patch to use?

Thank you!

@magmatic

here is a link to a useful tutorial on applying patchs
https://drupalize.me/videos/applying-and-creating-patches-git?p=1173

@magmatic , if you do not know how to apply patches or prefer not to, you can safely use the 7.x-dev branch of D7 which has all these php 7.2.x fixes.
https://www.drupal.org/project/drupal/releases/7.x-dev

7.60 rolled out and.... not committed.

Patching all sites... Again.

Security releases do not contain bug fixes, so none of these PHP 7.2 fixes were included in the 7.60 release. Hopefully maintainers will finally merge and release a 7.61 with these changes.

This has been reported almost a year ago and you guys still rolling out updates with deprecated code for what reason exactly? And same was told about 7.60 that it will hopefully finally merge. So saying maybe it will merge with 7.61 means nothing, it's literally been almost a year.

I know it was released almost a year ago, and in fact, I contributed patches on this issue and other 7.2-fix issues too.

Security releases don't contain any bug fixes, and they have to be coordinated with other branches (7.x, 8.6, etc), and these security fix patches are a PITA when commits are diverged.

For 7.x branch in particular, I believe we have new maintainers, who may need to catch up the speed. We should also keep in mind that we all are here as volunteers. That said, I am looking forward to get these finally released into a proper release as well, to keep our post-update-patch workflow minimal.

Hi,

We are preparing the patches and commits for the upcoming release.

I'm doing all my best to have this one committed as well, it should be straightforward :-)

Sorry for the delay guys.

Update today to Drupal 7.60 and apparently the "Function each() is deprecated since PHP 7.2 [D7]" is still an issue in menu.inc file.
I had to apply the patch manually again.

As a reminder, you can always use the -dev snapshot of core until the next stable version is released.

I think I've been too hasty in my previous comment.

The patch in this issue has been released in 7.60, why do you still have to apply it manually?

Am I missing something?

@Pol: If you check the git log, 7.60 was the same as 7.59 with the new security fix added, it didn't include everything else committed on the 7.x branch, the last time there was a feature release of D7 it was 7.55.

to fix the d7 dev branch that was short circuited by 7.60 you could simply cherry-pick all the commits they skipped in the 7.60 release
git clone d7core
cd d7core
git checkout 7.x-dev
git cherry-pick HASHKEY

repeat for all of these commits:
https://cgit.drupalcode.org/drupal/commit/?h=7.x&id=083a4eca4a2ebc5276ee...
https://cgit.drupalcode.org/drupal/commit/?h=7.x&id=b014c196e1eab417e070...
https://cgit.drupalcode.org/drupal/commit/?h=7.x&id=76a4dc7098771a14e7d6...
https://cgit.drupalcode.org/drupal/commit/?h=7.x&id=73e12f0ddf1ed60c1333...
https://cgit.drupalcode.org/drupal/commit/?h=7.x&id=28de6772813387bf02a4...
https://cgit.drupalcode.org/drupal/commit/?h=7.x&id=127ffe91fffb5813c324...
https://cgit.drupalcode.org/drupal/commit/?h=7.x&id=dcf8c646fbab4f9fbb0d...
https://cgit.drupalcode.org/drupal/commit/?h=7.x&id=ee301cf5ebff3534b59f...

this should bring all these commits back into head of 7.x-dev branch
maybe there's a better way, but would get us back on track. Currently all 7.x-dev testing is going to fail on php 7.2.x until this is fixed.

@joseph.olstad
The release of 7.60 without all those commits did not affect 7.x branch at all. 7.x still has all those commits, plus the security fix. Testing will go on as before.
(There is no "7.x-dev" branch of core.)
DamienMcKenna is correct. Everyone using a dev snapshot will have the fix in this issue.

ok thanks for the clarification.
so 7.x branch has the fixes. I stand corrected, this is good thanks!

How to check/test patches? like
7.x: PHP 7 & MySQL 5.5 2,041 pass PHP 5.3 & MySQL 5.5 2,036 pass, 4 fail

These patches are already in 7.67 release the main drupal core qa testing runs automatically all those versions of php are currently passing all tests. Go to the drupal core project page and click on the rightmost tab after version control and releases