Module name check prevents panels custom content panes export

subscribe

I haven't tested yet, but this looks like a better approach than the one I posted in #1068528: Custom content panes Integration?

Just for reference, my patch doesn't eliminate the name check, but adds the appropriate arrays to ctools_features_api() to allow those components to be exported.

FYI this applies cleanly to Features v7.x-1.x too. Going to give it a spin.

subscribe

Marked #1064058: Exporting Custom Content Panes into Features as a duplicate of this issue.

After doing a little research, I found out where these lines of code came from. Not sure if they're still worthwhile or not. http://drupalcode.org/sandbox/Haugen/1186794.git/commit/1b11e7a

subscribing thanks!

This patch fixes the issue for me as well.

The question is whether the name should be changed.

            'name' => isset($modules[$api_module]->info['name']) ? $modules[$api_module]->info['name'] : $api_module,

Currently it's "chaos tools" for "custom content", but that's somehow unflexible.
What about using $schema['export']['api']['owner'] and $schema['export']['api']['api']

Make status updated.

Yes, that would be fine. :-)
had to search a little bit before I could find this option for adding content panes in "Chaos Tools".

+1 for this one

Thanks!

http://drupalcode.org/project/features.git/commit/ff5100c

We have 6.x issues marked as duplicate of this. Was it fixed in 6.x aswell ?

Unfortunately, this patch (on the D7 Beta4 release of Features) causes an error with certain PHP versions (less than 5.2.17). On Acquia hosting running PHP 5.2.4, this patch causes the errors:

array_merge_recursive(): recursion detected in module_invoke_all() (line 821 of /mnt/www/html/aljturkeystg/docroot/includes/module.inc).

when the cache is cleared (such as with "drush cc all").

Mark things needs work when they need work. Is this also buggy in 7.x?

Seeing the same issues on another Acquia Hosting client with a D7 site. If this breaks PHP <5.2.17, this should be fixed or require a minimum PHP version (obviously the first being the right option).

Here is a variation of the patch in #3 that works with PHP <5.2.17. It creates a static cache of routines already returned by ctools and doesn't return the same data again. Prevents the recursion in module.inc but still deals with the original issue with the module name. So I think this solves both issues. Needs review and testing.

Here is an update that uses drupal_cache so somebody else can clear the cache if needed. Note this is for D7.

Unfortunately patch #24 doesn't work either. It actually breaks Features completely and causes changes to ctools features (like views) to not be detected as overridden.

Still need a solution for this then that doesn't cause the error on PHP 5.2.

A very slight reroll of #23, only to make it play nice with older versions of drush make that require the older -p0 style patches.

...with the file.

Patch #28 doesn't apply from main Features directory.

But also it has the same issue as #24 and breaks Views/Features so changes to a view are not marked as overridden. So we need to find another approach to fix this.

OK, here is a somewhat kludgy alternative.

This patch restores the original module_name check to prevent the PHP merge_array_recursive error. To handle the ctools content issue, I just added a check for a module_name of 'ctools'. This allows the custom content to show up, but still prevents the recursive error.

It's very possible that this will still cause the recursive error when you are using custom content panes *and* you have the wrong version of PHP. In that rare case I suggest just trying to upgrade your PHP.

I applied the patch from #30 and updated my Acquia hosting environment. I was able to revert features and didn't get the recursion error caused by that version of PHP.

I have rolled a version of the patch in #30 that works with Drush Make <2.3. It's only for those of us stuck in an older version of drush_make atm; otherwise use the patch in #30.

Unfortunately, I found that the patch in #30 is subject to the same array_merge_recursive() errors when used in combination with php 5.2.4. I've rolled a patch that synthesizes the approaches in #24 and #30. I've tested the following with 5.2.4 and 5.3.6:

• created a custom content pane and successfully exported it.
• modified an exported view, the view showed as overridden, and reverting worked successfully.
• cleared cache without receiving array_merge_recursive errors

thanks much,

Adrian

Sorry, but the patches in #1, #2, #3 up to the commit in #17 totally don't make sense to me.
We get a parameter, then discard it. How can this be the intended behavior?

The function ctools_component_features_api() is designed to return all component info for a given module.
It did what it was designed for - until the commit in #17.

Yet I see no explanation how the ctools_component_features_api() $module_name check causes the bug that was reported by the issue starter. Removing the check seems to fix the bug, but noone has yet cared to investigate or explain why :(

Ok, I think I know what is broken.

Looking at function features_include(), in features.module

<?php
function features_include($reset = FALSE) {
  static $once;
  if (!isset($once) || $reset) {
    $once = TRUE;

    // Check for implementing modules and make necessary inclusions.
    foreach (module_implements('features_api') as $module) {
      $info = module_invoke($module, 'features_api');
      foreach ($info as $component) {
        if (isset($component['file'])) {
          require_once DRUPAL_ROOT . '/' . $component['file'];
        }
      }
    }

    // Features provides integration on behalf of these modules.
    // The features include provides handling for the feature dependencies.
    // Note that ctools is placed last because it implements hooks "dynamically" for other modules.
    $modules = array('features', 'block', 'context', 'field', 'filter', 'image', 'menu', 'node', 'taxonomy', 'user', 'views', 'ctools');');

    foreach (array_filter($modules, 'module_exists') as $module) {
      if (!module_hook($module, 'features_api')) {
        module_load_include('inc', 'features', "includes/features.$module");
      }
    }
?>

This can only work if ctools is really included the last of all.
However, some modules (like ds) include the "includes/features.ctools.inc" before features_include() does so itself.
Too bad.

Patch going to follow.

And here is the patch.
Moving the eval() hack into features_include().

Testing and code improvements up to you :)

+1 to the patch in #34 for both avoiding the array_merge_recursive error and for also allowing export of custom content panes.

I need to spend more time looking at #37 and testing it before making any recommendation about it, although part of me dislikes bringing so much ctools-specific stuff into the core features module. I'd rather keep exportables cleanly in their separate files as much as possible.

I'd rather keep exportables cleanly in their separate files as much as possible.

There is nothing clean about that ugly eval() hack, no matter where it is sitting.
And the features.module file is not at all agnostic about those plugins: It knows each of them by name, and wants them in a specific order. Which then fails.

I need to spend more time looking at #37 and testing it

Yep.

One thing we should have a second thought about: What if any of those modules wants to implement the hook by itself, some day in the future?
In general, this should be np.
But for ctools? Would ctools also add this eval() stuff? Or just its own implementation?
It is likely it would only do its own implementation.
The features.ctools.inc inclusion would not and must not fire in this case, so the ctools_component_* functions would all be unavailable.

Conclusion:
If ctools implements the hook itself, then either
- we totally skip the eval() stuff, assuming ctools is totally taking care of this by itself, or
- we assume that ctools also defines the ctools_component_* stuff, or
- we implement this stuff elsewhere. Either in the features.module file, or a separate include, that does not clash with ctools.

Either way, would require some modification to the patch in #37.
(but would also require some modification to the existing code, unless we choose the first option)

Conclusion:
If ctools implements the hook itself, then either
- we totally skip the eval() stuff, assuming ctools is totally taking care of this by itself, or
- we assume that ctools also defines the ctools_component_* stuff, or
- we implement this stuff elsewhere. Either in the features.module file, or a separate include, that does not clash with ctools.

Except that, ctools cannot "take care of this by itself", because its own hook_features_api() in this case is no longer guaranteed to run last, and there is no other chance where it could do the eval() stuff.

So, what I suggest we do:
1) We put the ctools_component_* stuff elsewhere, maybe even with a new name, clearly making it part of the features namespace, so we don't clash with any future ctools stuff.
2) We don't care, until merlinofchaos knocks the door :)

Ah, nice, I see the current patch #37 already behaves like the first option.
The _features_eval_ctools_functions() will not do anything, if the features.ctools.inc was not included before. At least this is no regression.

OK, I've got a modified version of your patch. It leaves the ctools function that does the eval in the ctools.inc file, but still calls it from the features_includes as you suggested. I like this approach a bit better.

But in general, your patch does seem to work. It prevents the array_merge_recursive issue, and yet also allows custom content panes to be exported to Features.

(btw, in the future you might want to read about the patch naming convention here: http://drupal.org/patch/submit)

Marking this as needed for the D7 release.

Thanks.
I'm ok with the changes.

(off-topic)

btw, in the future you might want to read about the patch naming convention here

Btw, how can you know the comment number before the comment is created? You can only guess, can you?
(/off-topic)

donquixote: I've been confused by that too, it should probably be documented better. You use the ordinal number of the comment within the thread, rather than the comment id, which is indeed unknowable before posting a comment. e.g. the patch in comment #42 above is named ctools_custom_content-1079440-42.patch

donquixote: I've been confused by that too, it should probably be documented better. You use the ordinal number of the comment within the thread, rather than the comment id, which is indeed unknowable before posting a comment. e.g. the patch in comment #42 above is named ctools_custom_content-1079440-42.patch

True, but even that can be thwarted by cross-posting, if you take some time to write a decent explanation for your patch.

True, but even that can be thwarted by cross-posting, if you take some time to write a decent explanation for your patch.

The only fail-safe solution: write and post your comment, then edit it to attach a patch.

*edit*: bah, I thought that was possible, but it turns out I'm wrong. Best way is to write your comment in an external editor, refresh thread, and post... Still could get happen, but less likely...

Tested the patch in #42 on Acquia hosting (PHP 5.2.4), and worked (no recursion errors).

Thanks everyone, especially for finding the real problem and not the symptom. Committed to 7.x.

http://drupalcode.org/project/features.git/commit/a1b2f3d

Needed for backport?

This caused fatals by calling a ctools function without checking first.
Taking care of it here #1382156-5: PHP Fatal error: Call to undefined function ctools_features_declare_functions()

Hm. Patch #37 had a check
if (function_exists('_ctools_features_get_info'))
The idea was, if ctools is disabled, then the file would not be included, and the function would not exist.
(not sure if that would have worked)

Patch #42 has the same check, but it _in_ the included file.
So, when we hit this check, it is already too late, the file has been included.

Probably the new check from #1382156-5 does what we need, so, ok with it.
Just wondering, will the file include, if ctools is disabled? And would it have been with patch #37 ?

I am using 7.x-1.x-dev which includes the commit in #42, though the component does not appear when creating or re-creating a feature. By my testing it appears the test at line 8 is passing - function_exists('_ctools_features_get_info') - but the components still aren't available in the UI. Spoke with acrollet in IRC and he says he's running into the same problem.

(CTools/Custom content panes both at 7.x-1.0-rc1)

As with @bradjones1 and his buddy on IRC, I'm also not seeing these components as exportable options in the Features UI.

As with comment #55, custom content panes are not present within Features UI (or drush commands for that matter). It doesn't seem to matter where the pane is created. Using either Page manager or the 'Custom content panes' admin UI makes no difference.

Using 7.x-1.0-rc3 on Drupal 7.14

Everything seems to export just fine when created within a Panels page.

After updating from Features 7.x-1.0-beta4 to 7.x-1.0-rc3 ctools_custom_content panes are not possible to export with features.
After a little digging around I found that the possibility to export ctools_custom_content was gone already in the update from 7.x-1.0-beta4 to beta5. The commit in #49 is the one removing ctools_custom_panes from the dropdown in features. I have attached a patch based on #33 and everything works again.

I am not at all sure this is the correct way but it works and I'm sure someone else will be able to look through any flaws and correct them.

Alternatively this patch to ctools will also make everything work, http://drupal.org/node/1504180
I am not sure if this should actually be fixed in ctools instead.

No, this goes back to the state we had in comment #30. Please re-read this entire thread. There are more complicated issues involved here.

In #42 we confirmed that we were able to export custom content panes. Are you sure you've updated ctools as required for this?

I am running latest version of both features and ctools also tried doing a clean d7 install with only features and ctools enabled. Even tried with dev versions of both modules. The issue is still there, reuseable custom content panes are not exportable through features.

When I create a custom content pane through /admin/structure/ctools-content/add, that is then available to insert in any panel. I am not able to export that custom content pane through features, here I mean by selecting it in the edit components dropdown that you get on /admin/structure/features/create.

custom content panes export just fine as long as they are not marked as reusable. Then they are just a part of the page manager page export.
The issue is only with reuseable custom content panes, those are as of features version 7.x-1.0-beta5 (when above mentioned patch was committed) no longer selectable when creating a feature. At least not according to the testing I have been doing (hope to god I am right).

Sorry for not clarifying that the issue is with reusable content panes, think that might be valuable information.

I am not deep into features and ctools code, basically just started to dig in today, I just discovered it not working and took a dive to figure out how it could work :)

Thanks for the clarification. I'll try to reproduce this issue again with reusable content panes. I'm pretty sure I just tried normal content panes previously.

Unfortunately, because of the issue mentioned in #34 I still cannot commit your patch. You are probably running on a later version of PHP and don't get the array_merge_recursive issue that is related to this. We need to make sure we get this tested with people running PHP 5.2.4.

So I'm still looking for guidance on how we can solve this complex issue here.

I can confirm that errors come rolling in when running the #57 patch on a PHP 5.2.x server.

I did some more testing and figured it had to be something with ctools_features_declare_functions so I added a little extra snippet.

Attached patch works with both PHP 5.2.17 and PHP 5.3.6 which are the versions I have at hand for testing. No errors, just Chaos tools: ctools_custom_content available for export when creating a feature through UI.

Hmm, that's very interesting. I'm wondering why the first line uses $info['module'] when the rest of the lines all use $component instead. Let's get a few more eyes on this, but I'm wondering if that first line with $info['module'] is just wrong.

Hi,

i think in normal cases the line $info['module'] is not wrong but by specify the owner key (example: in the schema of ctools_custom_content.install) on other value as the module name we can now search / build the hook_feature_api on a other module ok but in this case the owner key is set to "ctools" which ends up in the function name "ctools_features_api" but this function already exists in features.ctools.inc.

i have no idea for a solution... any ideas here ? did we need really the owner set to "ctools" ? We need some response von merlinofchaos here... #1504180: Custom pane export into features

Hi,

ok i have added a patch it based upon #63 from zorp but additional we need in ctools_component_features_api the same check to ensure the processing of the component.

Now it works fine for me.

Edit:
this also solves the export problem for other ctools export plugins like ctools_access_ruleset

Can somebody post test results of the patch in #66 on a version of PHP earlier than 5.2.17? (the version that fixed the recursive array merge bug in PHP)? Since this kind of patch has been problematic in the past, it really needs more testing before I can commit it.

I can give it a go on PHP 5.1.6 when I get a little time at hand one of the next days.

i have tested #66 on:

- a new machine (Debian Lenny) With PHP 5.2.6 without no errors.
- 5.2.17 on mac with mamp pro
- 5.3.6 on mac with mamp pro

in all cases no notices / warnings or errors. The result from feature seems to be good.

Commited the patch in #66 to a2d5bdf. Please encourage people to test the latest -dev version to ensure there are no other side-effects of this change since this is such a long-standing and tricky issue. Thanks for all the work on this!!

I've re-rolled the patch in #28 (similar to #24) against 2.0-beta1 and made sure it was from the main features directory and not the includes directory as mentioned in #29. I recognize there are other issues that were found with this patch but it worked for my specific purpose so I've made it available in case anyone else needs it.

I updated the issue summary to document the commits that were done here.

I am quite confused by the third commit e894439, the one from #70.

Hmm, that's very interesting. I'm wondering why the first line uses $info['module'] when the rest of the lines all use $component instead. Let's get a few more eyes on this, but I'm wondering if that first line with $info['module'] is just wrong.

The reason is that hook_features_api() is a module hook, whereas the other hooks are component hooks.
The latest version of features 7.x-2.x has improved documentation in features.api.php, which hopefully clarify the difference.

Consequently, the "{$component}_features_api()" does not make sense.
It only makes sense if $component happens to also be a module name.
This happens to be the case here (or at least in the old version of ctools that was used) but this is a coincidence we should not rely on.

but in this case the owner key is set to "ctools"

This is no longer the case since #1504180: Custom pane export into features was committed in ctools.

If we do have ctools components with owner = 'ctools', then these could be declared directly in ctools_features_api().