In the drupal_bootstrap_full() there is a totally incorrect use of drupal_static()

In addition, all the require_ince should be converted to require according to Rasmus.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Damien Tournoud’s picture

Title: remove drupal_static and convert require_once to require in bootstrap.inc » Remove drupal_static

Rasmus or not, no way we convert those require_once to require. The difference is minimum and the first is *way* more reliable.

pwolanin’s picture

Title: Remove drupal_static » Remove drupal_static and require_once in _drupal_bootstrap_full()
FileSize
1.97 KB

Still makes sense to just use require to me.

pwolanin’s picture

Status: Active » Needs review
Damien Tournoud’s picture

Title: Remove drupal_static and require_once in _drupal_bootstrap_full() » Remove drupal_static() in _drupal_bootstrap_full()

Changing those require_once() to require() doesn't make any real difference in terms of performance, and would make the whole bootstrap process less robust.

For example, if a warning is triggered during bootstrap process, for example, error.inc will be loaded, and the require() call will explode, a module could want to load unicode.inc during a hook_boot, etc.

I'm not sure about removing the static, it seems to make sense to me.

Status: Needs review » Needs work

The last submitted patch, 892864-bootstap-rasmus-2.patch, failed testing.

pwolanin’s picture

Status: Needs work » Needs review
FileSize
664 bytes

minimal patch

Status: Needs review » Needs work

The last submitted patch, 892864-bootstap-rasmus-6.patch, failed testing.

pounard’s picture

Why do you want to replace require_once using require ? require_once is better and good for you. OPCode caches all optimize this method, and it may avoid potential crashes if somes modules attempt to require some of these files by themselves (using a light bootstrap or any other context like hook_boot() or such).

Here is a patch attached, that also fixes the wrong drupal_static() usage made in fix_gpc_magic(). If a miracle happens and this bootstrap method is run more than once, after the drupal_static() registry has been cleared, doing twice fix_gpc_magic() would totally break superglobals content.

pounard’s picture

Status: Needs work » Needs review
Dries’s picture

Status: Needs review » Reviewed & tested by the community

This looks RTBC to me, but I want to give it bit more time for additional reviews.

pounard’s picture

Nice, thanks.

Damien Tournoud’s picture

Looks good to me too.

pounard’s picture

If I had one thing to add, it would be a good comment about why those are not drupal_static() calls. It may worth it since a lot of developers could read this code and ask themselves why.

Dries’s picture

Status: Reviewed & tested by the community » Fixed

Committed to CVS HEAD. Thanks.

Status: Fixed » Closed (fixed)

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