The .install file fails when using an installation profile with a brand new site. This is because it's trying to call _block_rehash. I believe it's failing because drupal is not bootstrapped at this point.

Either a check for this function should be done, or this call should be removed. I'm wondering why it's even there? (I'm admittedly a newb.)

CommentFileSizeAuthor
#7 drupal_screen.png28.61 KBcha0s
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

moshe weitzman’s picture

moshe weitzman CreditAttribution: moshe weitzman commented

It is there so that you don't have to manually enable the block. So, does anyone know of a better way to do that?

cha0s’s picture

cha0s CreditAttribution: cha0s commented

Let the user enable it instead? ;)

moshe weitzman’s picture

moshe weitzman CreditAttribution: moshe weitzman commented

Yes, if we get no better ideas then thats what we will do.

salvis’s picture

salvis
he/his
CreditAttribution: salvis commented

Having it auto-enabled is nice, because the module doesn't do anything without the block.

We could check for _block_rehash() before calling it, then it wouldn't fail in an installation profile, but it wouldn't be working either, so there'd be no point in putting it into the installation profile.

If the user has to manually enable the block, then he may as well manually enable the module, or am I missing something?

cha0s’s picture

cha0s CreditAttribution: cha0s commented

Yeah, you definitely have a point. Although I think at the very least it should be able to be installed in an installation profile without borking everything else as it currently does, however maybe there should just be a warning thrown if the _block_rehash function isn't usable.

Take DHTML menu for instance, when you install it in an installation profile, it gives you a link afterward to actually enable the menus, maybe that concept could be reused for devel_node_access to enable the block?

salvis’s picture

salvis
he/his
CreditAttribution: salvis commented

I looked at DHTML Menu. It's a one-shot message from hook_enable(). Does that really work in an installation profile? How does it scale? What if all modules in the profile do this?

cha0s’s picture

cha0s CreditAttribution: cha0s commented
FileSize
drupal_screen.png28.61 KB

It seems that the messages will "stack" on the finished installation screen. I have attached a screenshot from Drupal 5 (which in this case, devel_node_access has been removed for now...)

The installation profile I'm using can be found here: http://therealcha0s.net/ubercart/ubercart_test.profile

If those messages are being displayed using hook_enable, then I guess that's the proper place to put _block_rehash(). :)

salvis’s picture

salvis
he/his
CreditAttribution: salvis commented

Thanks for the screenshot!

Let's take a step back:

@moshe:

It is there so that you don't have to manually enable the block.

Is it really needed? My testing on D5 shows that the DNA block shows up automatically, even if _block_rehash() isn't called explicitely.

Could this be a left-over of D47?

moshe weitzman’s picture

moshe weitzman CreditAttribution: moshe weitzman commented

I guess it could be. I'm OK with removing this from hook_install()

salvis’s picture

salvis
he/his
CreditAttribution: salvis commented
Status: Active » Fixed

Ok, committed to all three versions.

Sorry about taking the long road, cha0s...

cha0s’s picture

cha0s CreditAttribution: cha0s commented

No problem, thanks for committing it.

Anonymous’s picture

Anonymous (not verified) CreditAttribution: Anonymous commented
Status: Fixed » Closed (fixed)

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