Here's the install profile I am using: https://github.com/mikecrittenden/panopoly-hates-me

Note that *that* works, but if you rename it to something that starts with an "e" then it fails. Interestingly, it seems to fail every time if the install profile starts with the letter "e" but works if it starts with another letter (tested with "what" or "fetsy" worked, but "etsy" and "etowah" fail, among others).

Steps to reproduce:

This depends on the panopoly_base_distribution_starter_kit drush extension to provide the drush psk command.

mkdir issue-2183937
cd issue-2183937
drush psk "Evil Panopoly"
cd evilpanopoly
drush make --no-core --contrib-destination=. drupal-org.make
cd ..
drush dl drupal-7.32
cd drupal-7.32
ln -s ../../evilpanopoly profiles/
# Replace DB information with a blank user
drush si evilpanopoly --db-url=mysql://db_user:db_pass@localhost/db_name --account-name=admin --account-pass=admin

Exact error:

Starting Drupal installation. This takes a few seconds ...                                                                                                     
WD php: PDOException: SQLSTATE[23000]: Integrity constraint violation: 1048 Column 'module' cannot be null: INSERT INTO {role_permission} (rid, permission, mod
(:db_insert_placeholder_0, :db_insert_placeholder_1, :db_insert_placeholder_2); Array
(
    [:db_insert_placeholder_0] => 4
    [:db_insert_placeholder_1] => administer panelizer taxonomy_term panopoly_categories breadcrumbs
    [:db_insert_placeholder_2] => 
)
 in user_role_grant_permissions() (line 3112 of /srv/http/d71/modules/user/user.module).
Cannot modify header information - headers already sent by (output started at /usr/lib/drush/includes/output.inc:38) bootstrap.inc:1217                        
PDOException: SQLSTATE[23000]: Integrity constraint violation: 1048 Column 'module' cannot be null: INSERT INTO {role_permission} (rid, permission, mb_insert_placeholder_1, :db_insert_placeholder_2); Array
(
    [:db_insert_placeholder_0] => 4
    [:db_insert_placeholder_1] => administer panelizer taxonomy_term panopoly_categories breadcrumbs
    [:db_insert_placeholder_2] => 
)
 in user_role_grant_permissions() (line 3112 of /srv/http/d71/modules/user/user.module).
Drush command terminated abnormally due to an unrecoverable error.        
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

dsnopek’s picture

Title: Error on install of Panopoly-based install profile: "Column 'module' cannot be null" » Error on install of Panopoly-based install profile: "Column 'module' cannot be null" when profile starts with "e"

I have tested this and can reproduce the problem! I also tested with a profile starting with "d" and it failed as well. I suspect that it's hook order changing depending on the name of the profile and that any name starting with "a", "b" or "c" would also fail (however, I've only tested "e" and "d").

Looking at the modules used by Panopoly, the most important "f" module is "features"!

However, I haven't dug deep enough to confirm that this is the root cause.

tom friedhof’s picture

Component: Code » Admin
FileSize
808 bytes

I tracked this problem down to this patch to panopoly_core https://drupal.org/files/panopoly_core-panelizer_default_permissions-183...

I was getting this issue, along with other issues after I upgraded to Panopoly 1.1. By reversing the patch I pasted above, my stuff works again.

mcrittenden’s picture

Original issue for the patch mentioned in #2 which caused this: #1837312: Troubles Implementing Default Permissions for Panelizer

mcrittenden’s picture

Status: Active » Needs review

I suppose Needs Review is technically correct for this since there is a patch, but it shouldn't be committed as is for obvious reasons.

dsnopek’s picture

Wow, thanks so much for tracking this down! :-)

It's still going to be interesting solving this, but knowing the change and issue will make it 100x easier.

lsolesen’s picture

Status: Needs review » Needs work

Should probably be tagged as Needs Work then :)

tom friedhof’s picture

It appears more than just this installation bug was introduced with Panopoly 1.1. The navbar seems to have stopped working too, the module can't find underscore or backbone any more. I think I'm going to stick with Panopoly version 59ba1c6 for now.

Panopoly installs with a bunch of features overridden as well, and they won't revert.

mrfelton’s picture

@tom friedhof have you updated any modules outside of the scope of a panopoly distribution upgrade?

tom friedhof’s picture

I initially did update other modules, but this issue seems isolated to panopoly. Once I put the panopoly revision back to 59ba1c6 in my make file, and rebuilt Drupal, the issues I was experiencing went away.

The patch I posted above only fixed one particular issue I was facing. Reverting back to the revision fixed all my issues.

tom friedhof’s picture

I've installed the latest version of Panopoly (4b70ce3), and now everything seems to be working ok. I'm not experiencing this issue, or the other issues I was experiencing when I chimed into this issue. Bazaar!

caschbre’s picture

I ran several tests against the latest panopoly dev and I'm not seeing the issue. And just so I'm clear, the issue is if the profile name starts with 'd', 'e', and possibly 'a', 'b', or 'c'?

Shameless Plug
I used https://www.drupal.org/project/panopoly_base_distribution_starter_kit to create several distributions with install profile names starting with various letters. I haven't hit this issue yet.

dsnopek’s picture

Yes, you are understanding correctly! I'm glad to hear that this bug isn't happening anymore. I guess it's still a mystery exactly what was causing it...

caschbre’s picture

Status: Needs work » Closed (cannot reproduce)

Marking this closed (unable to produce) at this time based on the latest -dev release of Panopoly. If anyone can provide more details on how to reproduce it, we can take another look.

lslinnet’s picture

Status: Closed (cannot reproduce) » Needs work

This still happens, if you install using drush with a distribution based of panopoly.

If you use the platform in https://github.com/drupaldanmark/drupalhagen (which uses the profile of https://github.com/drupaldanmark/dh_core) it still happens.

mglaman’s picture

Status: Needs work » Closed (cannot reproduce)

@lslinnet

I'm re-closing this as it doesn't look like that platform is using Panopoly? Might be an issue to open on another contrib project which could be causing the similar problem. (Like adapt_core.)

dsnopek’s picture

Issue summary: View changes
Status: Closed (cannot reproduce) » Needs review

So, I'm still able to reproduce this, even though @caschbre is unable to. I've updated the issue summary with the exact steps I take to reproduce. I haven't had a chance to try the patch posted here yet, though, so I'm marking this "Needs Review".

dsnopek’s picture

Status: Needs review » Needs work

I tried applying the patch and it didn't make any difference - I still got the error. Marking to "Needs works".

caschbre’s picture

mkdir issue-2183937
cd issue-2183937
drush psk "Evil Panopoly"
cd evilpanopoly
drush make --no-core --contrib-destination=. drupal-org.make
cd ..
drush dl drupal-7.32
cd drupal-7.32
ln -s ../../evilpanopoly profiles/
# Replace DB information with a blank user
drush si evilpanopoly --db-url=mysql://db_user:db_pass@localhost/db_name --account-name=admin --account-pass=admin

Is there a reason you drush make without core? Wouldn't a fresh install do a drush make on the build-**.make file?

dsnopek’s picture

Is there a reason you drush make without core? Wouldn't a fresh install do a drush make on the build-**.make file?

So, the build-*.make file won't work without either (1) uploading my 'evilpanopoly' to Git somewhere or (2) modifying build-*.make to work locally with make_local. This is basically the same as this issue for panopoly_base_distribution_starter_kit: #2338743: Include a script to build distro from drupal-org.make?

Doing the extra steps with downloading Drupal and setting up the symlink (only 3 extra commands) just seemed easier!

In any case, if you can reproduce it using fewer commands, feel free to update the issue summary. :-)

fastangel’s picture

Status: Needs work » Needs review
FileSize
612 bytes

I found the bug is so stupid but a lot painful to find.

What is the problem?
The problem is the order that panopoly enable the modules. Panopoly core enable first panelizer and after this enable taxonomy. Then what happen?. Panelizer module has a condition:

/**
 * @file
 * Definition of the taxonomy term plugin.
 */

if (module_exists('taxonomy')) {
  $plugin = array(
    'handler' => 'PanelizerEntityTaxonomyTerm',
    'entity path' => 'taxonomy/term/%taxonomy_term',
    'uses page manager' => TRUE,
    'hooks' => array(
      'menu' => TRUE,
      'admin_paths' => TRUE,
      'permission' => TRUE,
      'panelizer_defaults' => TRUE,
      'default_page_manager_handlers' => TRUE,
      'form_alter' => TRUE,
      'views_data_alter' => TRUE,
    ),
  );
}
~   

That check if module taxonomy was enabled. On this case isn't enabled and then doesn't add the plugin and we don't have the permissions. I thought some solutions for this problem (clear module list, add again the plugins for panelizer,....) but I found the more easy solution. If we change the order on the depend modules (.info file) and we add first taxonomy and after taxonomy panelizer work fine.

mglaman’s picture

The order you place dependencies in a module's .info actually has an impact to the way they're enabled? That is nuts and I had no idea. If so, looks like patch is the homerun.

fastangel’s picture

yes has impact :)

dsnopek’s picture

Thanks for digging into this! I'm looking forward to testing it. :-)

Unfortunately, the order of dependencies is automatically alphabetized by Features. So, if this is the solution, ideally we should up with a way (maybe an alter hook somewhere?) that would prevent Features from constantly undoing this, everytime we run drush fu -y panopoly_core.

mglaman’s picture

Can't jam on this right now, but looks like we might want

    // Invoke hook_system_info_alter() to give installed modules a chance to
    // modify the data in the .info files if necessary.
    $type = 'module';
    drupal_alter('system_info', $modules[$key]->info, $modules[$key], $type);

Problem is.. hook_system_info runs on installed modules. This is fine if we're altering Panopoly profile, but Core isn't installed yet so I'm guessing it would run.

What if we just get crazy with it and array_unshift() it to the top of the dependencies here? Note: never actually played with the func below to know what it specifically does, aside from sort dependencies around.

/**
 * Task handler to load our install profile and enhance the dependency information
 */
function panopoly_core_install_load_profile(&$install_state) {

  // Loading the install profile normally
  install_load_profile($install_state);

  // Include any dependencies that we might have missed...
  $dependencies = $install_state['profile_info']['dependencies'];
  foreach ($dependencies as $module) {
    $module_info = drupal_parse_info_file(drupal_get_path('module', $module) . '/' . $module . '.info');
    if (!empty($module_info['dependencies'])) {
      foreach ($module_info['dependencies'] as $dependency) {
        $parts = explode(' (', $dependency, 2);
        $dependencies[] = array_shift($parts);
      }
    }
  }
  $install_state['profile_info']['dependencies'] = array_unique($dependencies);
}
dsnopek’s picture

That could be a possibility! I was thinking more along the lines of altering the output from Features, so that it writes the .info file the way we want it. panopoly_core will definitely be installed when running drush fu -y panopoly_core (because otherwise it wouldn't work). Then we don't have to worry about messing around with the install process because the .info file is just right.

dsnopek’s picture

Component: Admin » Core
Assigned: Unassigned » dsnopek

Sorry for taking so long to get around to this! I'm able to verify that this patch does fix the problem in my testing. :-)

However, like I mentioned above, since the dependency list will always be sorted by features, this change will get undone everytime we run drush fu panopoly_core. So, we'll need an alternate solution to accomplish the same thing!

I looked through Features - unfortunately, it's sorting the dependencies after the last hook (hook_features_export_alter()) is fired, so we can't change it's output.

So, I'm experimenting a bit with some of the things that @mglaman suggested in #24.

dsnopek’s picture

Assigned: dsnopek » Unassigned

I have proven conclusively that messing with panopoly_core_install_load_profile() doesn't help... The dependencies get re-ordered again after that function adds them, so it makes no different. However, for the record, is a patch of some of the things I tried to no avail:

--- a/panopoly_core.profile.inc
+++ b/panopoly_core.profile.inc
@@ -21,15 +21,40 @@ function panopoly_core_install_load_profile(&$install_state) {
   install_load_profile($install_state);
 
   // Include any dependencies that we might have missed...
-  $dependencies = $install_state['profile_info']['dependencies'];
-  foreach ($dependencies as $module) {
+  $dependency_tree = array();
+  foreach ($install_state['profile_info']['dependencies'] as $module) {
     $module_info = drupal_parse_info_file(drupal_get_path('module', $module) . '/' . $module . '.info');
     if (!empty($module_info['dependencies'])) {
       foreach ($module_info['dependencies'] as $dependency) {
         $parts = explode(' (', $dependency, 2);
-        $dependencies[] = array_shift($parts);
+        $dependency_tree[$module][] = array_shift($parts);
       }
     }
   }
-  $install_state['profile_info']['dependencies'] = array_unique($dependencies);
+
+  $profile_dependencies = array();
+  foreach ($dependency_tree as $module => $module_dependencies) {
+    $profile_dependencies = array_merge($profile_dependencies, $module_dependencies);
+    $profile_dependencies[] = $module;
+  }
+  $profile_dependencies = array_values(array_unique($profile_dependencies));
+  
+  // Issue #2183937: To avoid issues with 'drush si' and installing Taxonomy
+  // after Panelizer, we need to make sure it's installed first!
+  /*
+  if (($taxonomy_index = array_search('taxonomy', $profile_dependencies)) !== FALSE) {
+    if (($panelizer_index = array_search('panelizer', $profile_dependencies)) !== FALSE && $taxonomy_index > $panelizer_index) {
+      unset($profile_dependencies[$taxonomy_index]);
+      array_splice($profile_dependencies, $panelizer_index, 0, 'taxonomy');
+    }
+  }
+  */
+
+  // Go a simpler way?
+  if (($taxonomy_index = array_search('taxonomy', $profile_dependencies)) !== FALSE) {
+    unset($profile_dependencies[$taxonomy_index]);
+    array_unshift($profile_dependencies, 'taxonomy');
+  }
+
+  $install_state['profile_info']['dependencies'] = $profile_dependencies;
 }

And, we can't use hook_system_info_alter() in panopoly_core for the reasons that @mglaman gives. We could make it so everyone needs to put a magic snippet into their profiles .profile, but that's less than ideal. Some people might just try Panopoly and give up before they find the docs for that.

So, I think I'm back to using @fastangel's patch, but adding something to our testing infrastructure that will catch this. So, we ever do a drush fu panopoly_core down the line, Travis-CI will complain after it's been committed (but before it's been released!).

rlmumford’s picture

I tried the dependency reordering patch and the error still happened, although my profile did start with an "o" so it might be a different issue entirely

rlmumford’s picture

Status: Needs review » Needs work

For the record, neither patch on this issue did anything to make the error go away, although the error is being thrown by exactly the same place in code.

rlmumford’s picture

For the record, neither patch on this issue did anything to make the error go away, although the error is being thrown by exactly the same place in code.

fastangel’s picture

@rlmumford can you provide more info because I tried with a new profile starting with o and I can't reproduce. Are you sure that you applied the patch fine and do a fresh install?

dsnopek’s picture

@rlmumford: It's also possible that you're getting the same error, but for a completely different reason! This error is usually caused by Features trying to grant a permission to a role, but the permission hasn't been declared yet. There's lots of reasons this could happen, and actually, Panopoly works around it in a couple places (see panopoly_pages_modules_enabled()).

This issue is about how that error occurs only for profiles that start with a specific letters, but doesn't for identical profiles that start with a different letters.

If you're encountering this error for another reason, it should be a new issue!

rlmumford’s picture

@dsnopek: I don't really know whether it's a different reason or not. The specific call to user_role_grant_permissions() that triggers the error is the same as in this issue.

Some more information:
The profile concerned is OpenCRM Kickstart (it's on drupal.org) and the error seems to occur when installing with aegir, haven't tested much elsewhere yet.

dsnopek’s picture

@rlmumford: Ok, thanks for the extra info! Can you open a new issue for that? I don't want it to stall progress on fixing the bug outlined here, since this does fix a problem for users and we are sooo close to actually getting this merged. :-) It just needs tests and we're done! For your problem we have to start investigating from scratch.

dsnopek’s picture

Issue tags: +sprint
aubjr_drupal’s picture

With Webspark, we ran into this issue with the exact same taxonomy perms error.

Once it was because of a newly built feature that we added to our sub-distro and had a panopoly_* module as a dependency in its .info file. This would bump the panopoly_* feature/module way up in the install order, before another module it needed was installed (in this case, the taxonomy module).

Another time I saw this behavior was with panelizer and a panopoly_ feature being dependencies for a Webspark feature. I removed the panelizer and panopoly_* dependencies, and the issue went away.

I think that the dependency ordering code in https://api.drupal.org/api/drupal/includes!install.core.inc/function/ins... is the real source of the problem. I don't see any way to hook into that function and manually change the order (around line 1426 in install.core.inc) without hacking core. Or is line 1407 the problem, where either the distro or subdistro name is not being pushed to the end of the install list?

mglaman’s picture

Following up to comments in relation to #20: features_populate() invokes the hook_features_export_alter() hook. However it then runs ksort() right afterwards on dependencies, so you can't change the output ordering for dependencies.

dsnopek’s picture

dsnopek’s picture

Priority: Normal » Major
mglaman’s picture

Status: Needs work » Needs review
FileSize
1.45 KB

Here is a test which checks if the dependencies ever get changed, ensuring taxonomy is #0!

dsnopek’s picture

Hiding the old patch so that I can test this on Travis.

EDIT: Here is the Travis build: https://travis-ci.org/panopoly/panopoly/builds/70727095

  • dsnopek committed 991f70b on 7.x-1.x
    Update Panopoly Core and Test for Issue #2183937 by mglaman, tom...
dsnopek’s picture

Status: Needs review » Fixed

Passed on Travis! In my local testing, the test failed without the panopoly_core patch and succeeded with it! So, I think this is FINALLY ready to go. :-)

While this test is totally sufficient to prevent regressing this exact issue (which is fine by me, in order to get this committed), we really should be doing much more general automated testing of child distros. I've created a follow-up issue for that: #2532178: Install a couple child distros as part of the Travis-CI tests

Thanks to everyone who helped track this one down and fix it over the past 1.5 years!

Status: Fixed » Closed (fixed)

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