The taxonomies set up by my feature modules are no longer created after I upgraded to features 7.x-1.0-rc2. Downgrading back to rc1 makes the taxonomies be set up again as they should.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

gaas’s picture

Priority: Critical » Normal

'git bisect' tells me that this commit introduced the problem:

commit fdbd18f6abb573d259145562af9a893a49c1d587
Author: Mike Potter <mpotter@phase2technology.com>
Date:   Fri Apr 13 11:57:02 2012 -0400

    Issue #1493274 by neochief, hefox: Fixed Feature installed before Strongarm blocks all other variable imports.
steinmb’s picture

Priority: Normal » Critical

Changing to critical then this problem is rather critical.

hefox’s picture

  • Do you mean the taxonomy vocabularies?
  • Is this during installing a site? Enabling on a preexisting site?
  • Is it all features with vocabularies? If you make a new feature with a vocab, does and enable however you were enabling, does it have same problems?
  • Remove features_include(TRUE); from features_modules_enabled. Vocab support is provided by features; the commit you reference is mostly about ctools intergration with that sole line being the only one unrelated to ctools. The line triggers features_include which goes through and includes any default files.
gaas’s picture

Yes, it's the taxonomy vocabularies that do not get created (3 of them, split between 2 different feature modules). The node types and other stuff seems to get created as they should.

The missing vocabularies is what I observe after 'drush site-install' with a profile that depends on my feature modules. After installing the site the features that define these vocabularies think they are up-to-date even if the vocabularies did not show up (in the database).

I can do some more experiments with this tomorrow (euro time). I don't have that code or site available where I'm now. If you think there are other things I can try, or information to gather please tell me.

hefox’s picture

Make sure the [feature].features.taxonomy_vocabulary.inc (think that's the file) is included at during install process
See what the diff is saying (install diff module).

JvE’s picture

I've just encountered this issue as well.

It only occurs when installing through Drush (both 4.5 and 5.1) with an install profile that uses a feature that has vocabularies.

Manually installing works fine.
Using drush to enable the feature once the site is installed works fine.

When features_include(TRUE); from features_modules_enabled is commented out the install works fine.
I suspect that somewhere along that chain a bit too much is being re-set, but I'm no features/ctools/drush expert.

gaas’s picture

Priority: Normal » Critical

I also verified that commenting out the features_include(TRUE) call avoids the problem.

What's interesting is that changing it to features_include(FALSE) doesn't make a difference. The vocabularies still don't show.

So it looks like it is the the fact that feature_include() is triggered by this code before it would normally be included that is the problem. If I apply this patch then my 'site-install' works as expected. With luck the original fix isn't harmed by this either :-)

diff --git a/sites/all/modules/features/features.module b/sites/all/modules/features/features.module
index 52704e7..8e3b214 100644
--- a/sites/all/modules/features/features.module
+++ b/sites/all/modules/features/features.module
@@ -306,6 +306,8 @@ function features_modules_enabled($modules) {
  */
 function features_include($reset = FALSE) {
   static $once;
+  if (!isset($once) && $reset)
+    return;
   if (!isset($once) || $reset) {
     $once = TRUE;
 
hefox’s picture

I suspect features_include is a red herring; including it triggered the bug, but my guess is another function is bugged.

I'd be really curious what happens during the first features_rebuild after the module is enabled
inside _features_restore (think that's the name)

if ( currently reverting that feature module) {
.. see if taxonomy_vocabulary file has been included, etc.
}

Also, I'd be curious if moving the features_include to the top of the features_modules_enabled helps.

JvE’s picture

Status: Active » Needs review
FileSize
370 bytes

I think the problem is that previously features_include() was called for the first time after all the dependencies (like taxonomy) of the feature were installed and enabled.

With the features_include call in the features_modules_enabled function the first call of features_include() moves up to the point where the features module is installed.

This causes features_get_components() to statically cache the available components which at that point is limited to only the installed modules so far.

My solution was to extend the $reset parameter to the features_get_components() call:

foreach (features_get_components(NULL, 'file', $reset) as $file) {

I'm not sure if this is a 100% solution, so I'm attaching a patch and marking for review.

hefox’s picture

Hm, fix makes sense. However, will slow things down a bit, but likely can clean that up #1530386: Avoid unnecessary cache rebuilds and improve installation performance

hefox’s picture

hefox’s picture

Status: Needs review » Reviewed & tested by the community

Based on comments here and in #1265168: Rebuild the file list properly when a feature is enabled or disabled, patch should address the issue + works in the base case that clears that cache when features_include(TRUE) is called + makes sense + simple patch = RTBC

hefox’s picture

Fix + tests.

My was it painful to reproduce the error.

hefox’s picture

Status: Reviewed & tested by the community » Needs review
FileSize
6.8 KB

Missing addition of new file (added vocabulary to export)

JvE’s picture

Status: Needs review » Reviewed & tested by the community

My was it painful to reproduce the error.

I can imagine. I got a headache just trying to think of where to start writing a test for this.
Yours is quite elegant and appears to work as it should (if we ignore the sneaking in of unrelated coding standard fixes ;)
Note that it can crash on taxonomy_features_api() not existing when combined with related bugs.

bojanz’s picture

Confirming RTBC.

mrfelton’s picture

With the patch from #14 applied to RC2, I get the following errors, with or with out the patch from #1530386: Avoid unnecessary cache rebuilds and improve installation performance (commet 6) also applied.

WD registry: PDOException: SQLSTATE[23000]: Integrity constraint violation: 1062 Duplicate entry 'FeaturesEnableTestCase-class' for key 'PRIMARY': INSERT INTO      [error]
{registry} (name, type, filename, module, weight) VALUES (:db_insert_placeholder_0, :db_insert_placeholder_1, :db_insert_placeholder_2, :db_insert_placeholder_3,
:db_insert_placeholder_4), (:db_insert_placeholder_5, :db_insert_placeholder_6, :db_insert_placeholder_7, :db_insert_placeholder_8, :db_insert_placeholder_9),
(:db_insert_placeholder_10, :db_insert_placeholder_11, :db_insert_placeholder_12, :db_insert_placeholder_13, :db_insert_placeholder_14), (:db_insert_placeholder_15,
:db_insert_placeholder_16, :db_insert_placeholder_17, :db_insert_placeholder_18, :db_insert_placeholder_19); Array
(
    [:db_insert_placeholder_0] => FeaturesUserTestCase
    [:db_insert_placeholder_1] => class
    [:db_insert_placeholder_2] => profiles/concern/modules/contrib/features/tests/features.test
    [:db_insert_placeholder_3] => features
    [:db_insert_placeholder_4] => 0
    [:db_insert_placeholder_5] => FeaturesEnableTestCase
    [:db_insert_placeholder_6] => class
    [:db_insert_placeholder_7] => profiles/concern/modules/contrib/features/tests/features.test
    [:db_insert_placeholder_8] => features
    [:db_insert_placeholder_9] => 0
    [:db_insert_placeholder_10] => FeaturesCtoolsIntegrationTest
    [:db_insert_placeholder_11] => class
    [:db_insert_placeholder_12] => profiles/concern/modules/contrib/features/tests/features.test
    [:db_insert_placeholder_13] => features
    [:db_insert_placeholder_14] => 0
    [:db_insert_placeholder_15] => FeaturesEnableTestCase
    [:db_insert_placeholder_16] => class
    [:db_insert_placeholder_17] => profiles/concern/modules/contrib/features/tests/features.test
    [:db_insert_placeholder_18] => features
    [:db_insert_placeholder_19] => 0
)
 in _registry_parse_file() (line 179 of /Users/tom/workspace/concern7/includes/registry.inc).

I actually get this printed 3 times in a row.

Damien Tournoud’s picture

@mrfelton: you probably applied the patch twice and ended up with two (or more) FeaturesEnableTestCase classes in tests/features.test. Just remove the unwelcome duplicates.

gaas’s picture

I've now verified that rc2 + features_1537838_features_get_components_14.patch avoids the install problem in my setting as well. Thanks!

steinmb’s picture

Bump, any chance to this committed?

Best reg.
Stein

mpotter’s picture

Status: Reviewed & tested by the community » Fixed

Commited to a2e030d.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.