Since the patch for #553944: Define hook_menu_get_item_alter() as a reliable hook that runs before the page is doomed was committed, menu_get_item() is being called before hook_init(). This means, for example, that strongarmed variables are not available during hook_node_load() when viewing a node.

Putting the call to strongarm_set_conf() in hook_custom_theme() works, but that seems silly. Any ideas?

-Scott

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Dripman’s picture

I think that I have a problem like yours.

If I try to create features - module that contains variable 'theme_default' = 'my_theme';
And when I do "Revert" in features -> this variable deletes from database, and does not want to get from feature - module correctly.

So we have global $theme_key = 'batrik', but global $conf['theme_default'] = 'my_theme';
And instead of my theme works 'batrik'.

But if I go to /admin/appearance it seems like my theme is current default theme, but actually works 'batrik'. And if I set another theme, and then my theme -> my theme works and 'theme_default' variable appears in database again.

Can anybody explain how to resolve this problem?

P.S. strongarm : 7.x-2.0-beta2; features : 7.x-1.x-dev

Dripman’s picture

Hey? True coders? Where are you? Problem still not resolved!!!

webflo’s picture

Hi,

i run in this issue too. I analyzed the Drupal bootstrap and found something interesting. hook_stream_wrappers is invoked very early. (common.inc line 4912) even before hook_custom_theme (common.inc line 4935) and hook_init (common.inc line 4937). This resolves the site_frontpage issue and (maybe) some others e.g. #1046204: access denied for translatable content and #1122244: Features/Strongarm compatibility.

webflo’s picture

Status: Active » Needs review

change status.

googletorp’s picture

Status: Needs review » Reviewed & tested by the community

It looks good as far as I can tell. Fixes #973906: Document _GET['q'] override in hook_init for Drupal 7.

Boobaa’s picture

I got a strange issue: enabling strongarm-7.x-2.0-beta2 rendered my i18n-backed site almost unusable, since all the links and form actions did have the language prefix, but i18n was not able to act upon them, so I got 404 all over the place. Applying the patch in #3 solved my issue as well. (Internal ref. no #949.)

derhasi’s picture

Works fine for me too. It's time to get in a dev release ;)

webflo’s picture

loganfsmyth’s picture

This worked well for me too.
I was running into the node_load issues along with problems in a custom module that uses variables as part of the menu access callback, so it was getting access denied at all times.

hook_stream_wrappers is ugly but effective :(

jhedstrom’s picture

Priority: Normal » Major

This fixes the issue for me (I was having trouble with spaces_dashboard, and the spaces_features variable not being set before the access callback was fired).

marcvangend’s picture

The patch from #3 worked for me to get language path prefixes working again. It's ugly, but very needed!

Ravi.J’s picture

I had quite a few problems with XML SItemap not working when exported into features #1132958: Access denied on xmlsitemap/rebuild. Patch from #3 works for me.
I am however not sure if setting if hook_stream_wrappers() is the right place to set variables cache.
I am going to keep an eye on this issue. Looks like permanent solution to the problem might need hook_init to be called much sooner in bootstrap process.

jhedstrom’s picture

Since calling hook_init() earlier in the bootstrap process is a core issue, this most likely won't ever happen in 7.x, and probably not in 8.x since it could negatively impact performance. While hook_stream_wrappers() feels hacky, it would appear to be the only available hook that is run early enough for strongarm to work for most variables.

marcvangend’s picture

If I'm not mistaken, hook_boot is called even earlier. Would that be an option instead of hook_stream_wrappers? (The downside would be a possible performance impact, because hook_boot is also invoked on cached pages.)

jhedstrom’s picture

Strongarm 1.x used hook_boot. On the first commit to 2.x, this was removed without explanation. Using hook_boot would definitely be less of a hack, as that is the exact place that the bootstrap process initializes the $conf variable anyway. With cached strongarms, I wouldn't think it would impact performance too much, but it might be worth benchmarking at some point.

marcvangend’s picture

Status: Reviewed & tested by the community » Needs work

@jhedstrom: interesting. The inline docs of version 1.x called it "agressive but necessary" to use hook_boot. Sounds to me like we should revert to hook_boot - marking as 'needs work'.

jhedstrom’s picture

Another potential downside of hook_boot, aside from perhaps performance, is that running the module will tell people that it may be incompatible with aggressive caching, even though it shouldn't be.

webflo’s picture

Has anyone tried to call strongarm_set_conf() in strongarm_boot(). I think its not that easy ...

Fabianx’s picture

Subscribe.

The downside of hook_boot is that strongarm uses other modules as far as I can see, which would need to be loaded, too.

I think strongarm would also benefit from a variable.inc mechanism to be able to handle variable handling in an efficient way.

Best Wishes,

Fabian

Fabianx’s picture

Update:

Okay, here is why hook_boot does not work.

In hook_boot only those modules are loaded that have hook_boot defined, so a module_implements('features_strongarm') or whatever won't work.

On the other hand loading all modules here is a severe performance hit.

Solution for this:

* Using cached variables in hook_boot is working really fine. (Using this in my faster_variables sandbox project)

* The cache needs to be rebuilt immediately after a cache_clear_all.

* This means only hook_exit() or other hooks running afterwards are possible for building the cache.

There is one approach by catch to build the core variable cache on hook_exit, too.
(While I am not 100% behind this approach and working on an alternative, it shows the feasibility: #987768: [PP-1] Optimize variable caching)

Problems with this approach:

* Cache going away (memcached dies, DB table was re-dumped)

Another idea would be to set the conf in hook_init, but load it in hook_boot and reload the page via 302 request in hook_init.

The problem here is:

* What happens if no cache is available? (endless loop)

* Should we store this additionally in the DB as fallback? And if we store this in the DB, when do we rebuild this?

Best Wishes,

Fabian

Robin Millette’s picture

subbing

DamienMcKenna’s picture

Subscribe.

Cyberwolf’s picture

This caused me some serious headaches. We're using Features/Strongarm for deployment of content types and more, and saw strange things happening on a production site.

On the "Manage display" tab of certain content types, the Language field which is added by the i18n_node module was (at first sight randomly) appearing and disappearing again. After a cache clear the field appears again. So apparantly the field info cache got filled with different data now and then. i18n_node checks with core's locale_multilingual_node_type() if the specific content type is multilingual, that method on its turn checks the 'language_content_type_x' variables. Something went wrong with those variables, as they were sometimes set (i18n_node adds the Language field), and other times not set (i18n_node does not add the Language field).

I eventually caught the node/[nid] page calls where the field info cache gets filled before strongarm injects the exported variables, and ended up here.

Adding the hook_stream_wrappers implementation to strongarm seems to solve the issue in my case. However, I don't feel comfortable about it as this hook is not really intended for such purposes. It's not clear to me though if there is a better hook for this purposes.

Simon Georges’s picture

Subscribe.

Messenjer’s picture

Subscribe

James Andres’s picture

Here is a quick re-roll of #3 (no changes) since it did not apply cleanly to a fresh copy of Git tag 7.x-2.0-beta2.

das-peter’s picture

I'd recommend to check this ticket #1094598: Performance issues from strongarm_init() - its goal is to totally get rid of this performance killer.
There are working patches for the 6.x and 7.x version.
Would be nice to have some more attention & feedback there ;)

Nico Heulsen’s picture

Subscribe

acbramley’s picture

sub

acbramley’s picture

#26 worked well for me. I was having problems with install profiles not setting variables in the database and strongarm wasn't setting them soon enough!

Simon Georges’s picture

Re-roll, on 2.x-dev, without the trailing space.

checker’s picture

Just asking. Fixing this issue does not make variable translation (i18n) with strongarm possile, does it?

Messenjer’s picture

The solution of Simon Georges fix the access denied on url node/%node/translate , so the variable of language_content_type_%node_type are loaded sooner.

http://drupal.org/files/issues/strongarm_1062452_2.x-dev.patch

drupal_was_my_past’s picture

#31 applied cleanly for me and fixed my issues (similar to #1) where the wrong theme and admin theme were being used after a features revert when theme_default was exported via features + strongarm.

fabsor’s picture

Status: Needs work » Needs review

This actually works, even though it is a hack of enormous proportions. It is an easy win to get this in, and it would make things work for a lot of users while we work on a better more permanent solution. On that basis, I'm marking this as needs review.

jhedstrom’s picture

Priority: Major » Critical
Status: Needs review » Reviewed & tested by the community

I think this fix is RTBC it had been extensively tested prior to the discussion about using hook_boot (which doesn't work properly). fabsor has an idea for a long term solution of simply populating the variable table on a cache rebuild, but that should be a feature request--this patch here fixes a critical bug.

jief’s picture

This patch does not work with simpletest.
ResponseText: PDOException: SQLSTATE[HY000]: General error: 1 no such table: simpletest745242.languages: SELECT * FROM {languages} ORDER BY weight ASC, name ASC; Array ( ) in language_list() (line 2324 of includes/bootstrap.inc).

sylus’s picture

Subscribe

hctom’s picture

subscribing

Just a small update: This is really critical if you rely on setting values in menu item access callbacks. When menu item access is checked the variables are nit initialized already and so every access is blocked, even when your feature (using strongarm) allows it by the corresponding setting.

febbraro’s picture

Am I correct in my assessment that if we remove the need for hook_init() (basically something needing to happen on every page load) that this is no longer an issue?

As das-peter said in #27, lets work on #1094598: Performance issues from strongarm_init() for 6 and 7 and make this issue a non-issue.

febbraro’s picture

Ok does the patch in #32 of #1094598: Performance issues from strongarm_init() fix this issue as well? We removed the timing aspect of it all.

febbraro’s picture

ping

fabsor’s picture

#1094598: Performance issues from strongarm_init() Fixes all cases where this would have been relevant as far as I can see, so we don't need to do anything more here, not at least as far as I am concerned. The newest version of worked with all cases where we had trouble before.

webflo’s picture

Status: Reviewed & tested by the community » Fixed

fabsor is right. variables are always stored in db and loaded by drupal core. we dont need this override any more.

gmania’s picture

This workaround is sadly not available for 6.x, since hook_stream_wrappers doesn't exist in Drupal 6.

I opened a separate ticket for Drupal 6:

http://drupal.org/node/1298236

Status: Fixed » Closed (fixed)

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