Convert actions variable(s) to CMI - add upgrade path

My first work with new CMI system.

Yay, thanks! :)

+++ b/core/modules/action/config/action.max_stack.yml
@@ -0,0 +1 @@
+max_stack: '35'

1) The config file/object should be named 'action.settings'.

2) 'max_stack' doesn't sound very self-descriptive to me. It's about stack overflow, and infinite recursion, ... I'm fairly sure that there is a better and more common name for such a parameter in the tech industry.

1) The config file/object should be named 'action.settings'.

fixed.

2) 'max_stack' doesn't sound very self-descriptive....

As far as I know, the similar thing is the recursion limit in python language:

import sys
print(sys.getrecursionlimit())

Output:
1000

The function in python can not call itself more than 1000 times.

So I replaced "max_stack" with "stack_limit".

As far as I know, the similar thing is the recursion limit in python language

Why don't we simply name it recursion_limit then? :) That sounds most self-descriptive, at least to me.

+1 to recursion_limit - self descriptive

+++ b/core/modules/action/action.moduleundefined
@@ -141,7 +141,8 @@ function actions_do($action_ids, $object = NULL, $context = NULL, $a1 = NULL, $a
+  $config = config('action.settings');
+  if ($stack > $config->get('stack_limit')) {
     watchdog('action', 'Stack overflow: too many calls to actions_do(). Aborting to prevent infinite recursion.', array(), WATCHDOG_ERROR);

+++ b/core/modules/action/lib/Drupal/action/Tests/LoopTest.phpundefined
@@ -61,7 +63,8 @@ class LoopTest extends WebTestBase {
+    $config = config('action.settings');
+    for ($i = 1; $i <= $config->get('stack_limit'); $i++) {
       $expected[] = "Test log #$i";
     }
     $expected[] = 'Stack overflow: too many calls to actions_do(). Aborting to prevent infinite recursion.';

Use a variable here like

$recursion_limit = config('action.settings')->get('recursion_limit');

Also I suggest to change a watchdog() message to contain a variable to describe a limit.
A kind of "Stack overflow: recursive calls to actions_do() is limited by %limit... ", array('%limit' => $recursion_limit

+++ b/core/modules/action/lib/Drupal/action/Tests/LoopTest.phpundefined
@@ -48,7 +48,9 @@ class LoopTest extends WebTestBase {
+    $config = config('action.settings');
+    $config->set('action.stack_limit', 7);
+    $config->save();

Use chain call without variable:

config('action.settings')
  ->set('action.stack_limit', 7)
  ->save();

Thank you all for your help!

Here is reworked patch.

Thanks! :)

Thanks a lot!

Committed and pushed to 8.x.

The yml file seems to be missing from the directory.

Yep, we need this in. I have added it to the patch in #1828410: Provide a bulk_form element for actions and fixes the issue.

Committed and pushed to 8.x. Thanks!

Did we forget the upgrade path here?

Here is patch with update path.

I guess we should also add an upgrade path test for that.

#17: 1788084-convert_actions_variables_to_CMI_add_upgrade.patch queued for re-testing.

@dawehner I see no reason to add upgrade test - this issue about 1 variable not a table

Working on that

In contrast to the patch in #17 i moved this into system.install because previously the functionality of actions.module happened in includes/actions.inc

Committed to 8.x. Thanks.