Problem/Motivation

Right now components running order cannot be configured. This lead to the situation when some components are reverting before other component can provide them with fresh data in database. For example if a revert brings a new field type, then the module providing that field type should be installed first. In other words, the 'dependencies' component should be reverted before 'field_base' component.

Proposed resolution

Implement weight on components so that components are loaded ordered by weight. Set 'dependencies' weight to -10.

Remaining tasks

Research if other components need prioritization and open separate issues, if case.

User interface changes

None.

API changes

The component defined in hook_featutes_api() can define a weight value. If omitted, it defaults to 0.

Data model changes

None.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

humansky’s picture

I ran into the same issue....I agree that features should enable the dependent modules first, then add fields, permissions, etc. The obvious workaround, is to add the dependency to the feature first, deploy, then add the field and redeploy. Just seems like a kludge to me, but that works.

hefox’s picture

Category: bug » feature

Patches welcome to autodetect whatever dependency, though I thouht it already did that.

mpotter’s picture

Status: Active » Postponed (maintainer needs more info)

Features should already be scanning your Field export and adding dependencies for modules that it needs. Sounds like maybe there is some obscure type of field being added by a module that is not doing this?

For example, when I add a "number" field, Features properly added "number" as a dependency. When I export a entityreference, it added "entityreference" module. So we'd need a step by step procedure with more detail for whatever specific module you had a problem with. I can't think of anything Features can do that it's not already doing.

joachim’s picture

It was a while since I filed this, but my wording suggests that the module that provided the field type was included as a dependency of the Feature. The problem was that it was *added* as a dependency, and on deploying the code, Features was trying to create the new field before the extra module had been enabled.

mpotter’s picture

Unfortunately that becomes a core issue. If the dependency is listed in the .info file for the feature module and Drupal is not enabling that module before your Feature module, then there isn't much Features can do.

joachim’s picture

I think you're misunderstanding the problem I reported.

The sequence of events is this:

- on dev server, create a feature
- deploy it to production. Everything fine.
- back on dev, add and enable a module that provides field type foo
- still on dev, add components to the feature, including a field of type foo
- deploy the feature

So at the point where the crash happens, the feature is already enabled, but a dependency and a field have just been added to its code.

mpotter’s picture

OK, Right, I understand now. But how is Features supposed to deal with this? Your Feature Module A now depends upon Module B. You need to disable Module A, enable Module B, then re-enable Module A. Features has no way to detect that this needs to be done. It's just a normal job of deployment. Part of deployment is to run update hooks, revert features, etc. You can't just push new code and expect everything to magically work.

For example, when I work on a site and add a new module dependency, I write an update hook that enables the new module on production, and part of the production deployment runs the update.php (drush updb) to handle that.

hefox’s picture

Features does enable newly added dependencies during features_rebuild to my memory, though I never rely on it and likely caches are not cleared between enabling the module and rebuilding the rest of the feature

Matters’s picture

I ran into the same problem. The solution for me was to perform in "mysite.com/devel/php" the function "module_enable(array('fivestar'), TRUE);"

RogerB’s picture

Thanks Matters for the solution. My site was completly locked up. Lifesaver.

VladSavitsky’s picture

I've solved the same issue by using field_cache_clear() in hook_update() after module_enable() and before field creation.
See https://api.drupal.org/api/drupal/modules!field!field.module/function/fi...

joco_sp’s picture

Issue summary: View changes

#9 worked for me. Lifesaver for me too. Thank you

claudiu.cristea’s picture

Category: Feature request » Bug report
Status: Postponed (maintainer needs more info) » Active

I'm getting the same error. On dev, to an existing feature, I'm adding a new field having a type provided by a new installed module. I'm recreating the feature and the new module, the field and the field instance are added. When I'm reverting on the other side with "revert all", I'm getting the error. A workaround is to manually revert first dependencies and then the field instance. But this is bug.

claudiu.cristea’s picture

Status: Active » Needs review
FileSize
1.26 KB

This is fixing the issue. It might be less elegant but having the modules enabled before anything else seems to me more than critical. I believe that dependencies must be treated with highest priority.

ovidenov’s picture

The patch from #14 seems to work for me.

claudiu.cristea’s picture

@ovidenov, great! Then why not setting the issue to RTBC? :)

ovidenov’s picture

Status: Needs review » Reviewed & tested by the community
hefox’s picture

Status: Reviewed & tested by the community » Needs review

A few more reviews and testbot are needed. One review isn't really "the community" :( Thanks though!

I haven't been paying attention to this, but from a brief look at what the patch does, I prefer a patch that changes the order that components are reverted -- there's an issue about that (revert order) but so far no good patches IIRC

jorgegc’s picture

Patch in #14 no longer applies. Uploading rerolled patch.

jorgegc’s picture

FYI this patch does not work when rebuilding the registry. We need to come up with something that takes that into consideration too.

jorgegc’s picture

Version: 7.x-1.x-dev » 7.x-2.x-dev
FileSize
1.37 KB

Uploading a patch that covers both scenarios. Also, changing the version of the module this bug affects. Please review.

Status: Needs review » Needs work

The last submitted patch, 21: fieldexception-1814122-20.patch, failed testing.

jorgegc’s picture

Status: Needs work » Needs review
FileSize
1.44 KB

I have run simpletests locally and they all passed. Uploading new patch!

shahmeerarshad’s picture

feature name field exception handeled

Status: Needs review » Needs work

The last submitted patch, 24: fieldexception_1814122_23.patch, failed testing.

jorgegc’s picture

Status: Needs work » Needs review

What has just happened? Spam?!

shahmeerarshad’s picture

sorry about the previous post
i am a newbie at drupal open source community

Status: Needs review » Needs work

The last submitted patch, 27: fieldException_1814122_27.patch, failed testing.

claudiu.cristea’s picture

Issue tags: +Needs tests
FileSize
1.94 KB

@jorgegc, we cannot go in that direction because in #18 the module maintainer disagreed with this approach. And I have to agree with him even it was my idea to implement hook_features_pre_restore().

The solution should be something like this but we need tests and I don't know how to write a test for this case.

claudiu.cristea’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 29: feat.patch, failed testing.

claudiu.cristea’s picture

Status: Needs work » Needs review
FileSize
1.35 KB
2.64 KB

There was a circular call in features_get_info():

features_get_info() > features_get_components() => ... => _ctools_features_get_info() => features_get_info()

I used system_rebuild_module_data() instead of features_get_info() in _ctools_features_get_info() because the only thing needed there is the module name and this is faster because the module list is statically cached.

@hefox, @mpotter, I still have no clue how we can test this. Can you take a look to this patch?

claudiu.cristea’s picture

FileSize
3.12 KB
495 bytes

Added docs.

claudiu.cristea’s picture

FileSize
3.12 KB
659 bytes

Minor doc typo fix.

hefox’s picture

hm, test that components are ordered correctly in feature info?

what really want to test is that dependencies are always enabled first, but off top of head, not sure how that'd be tested.

claudiu.cristea’s picture

FileSize
3.73 KB
623 bytes
623 bytes

This test alone should fail. It tests if 'dependencies' is on top.

claudiu.cristea’s picture

Issue tags: -Needs tests

Status: Needs review » Needs work

The last submitted patch, 36: fieldexception_-1814122-36-test-only.patch, failed testing.

claudiu.cristea’s picture

Title: FieldException: Attempt to create a field of unknown type in field_create_field » Allow components to be ordered by weight
Issue summary: View changes
Status: Needs work » Needs review
FileSize
462 bytes
4.18 KB

Yes. Perfect!

Changing the issue title and summary. Added a CHANGELOG entry.

claudiu.cristea’s picture

claudiu.cristea’s picture

@hefox, Unfortunately this patch assures that components are ordered when loading a feature with features_load_feature() but I'm not so sure that on revert/rebuild the components are loaded in this way... :(

Maico de Jong’s picture

Nice work!

#23 worked for me
#39 didn't

I was using drush (drush fr) to revert the components, permissions we're still reverted before dependencies causing an error. Maybe drush is using a different way to loop through components?

gnucifer’s picture

@claudiu.cristea One place to reorder components could perhaps be in a hypothetical alter hook for each modules components right after hook_feature_pre_restore is invoked, or to sort them by weight there. But I could have missed something.

gnucifer’s picture

Here is a patch with my suggested solution.

gnucifer’s picture

@claudiu.cristea I looked through your patch again, and I really should have included your changes as well. I think that if you include the small fix in my patch (sorting before components are rebuilt/reverted) that would hopefully be a working solution for this problem.