Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 UTC on 18 March 2024, to get $100 off your ticket.
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.
Comment | File | Size | Author |
---|---|---|---|
#8 | common.inc-fix_static_usage_bootstrap.patch | 1.17 KB | pounard |
#6 | 892864-bootstap-rasmus-6.patch | 664 bytes | pwolanin |
#2 | 892864-bootstap-rasmus-2.patch | 1.97 KB | pwolanin |
Comments
Comment #1
Damien Tournoud CreditAttribution: Damien Tournoud commentedRasmus or not, no way we convert those require_once to require. The difference is minimum and the first is *way* more reliable.
Comment #2
pwolanin CreditAttribution: pwolanin commentedStill makes sense to just use require to me.
Comment #3
pwolanin CreditAttribution: pwolanin commentedComment #4
Damien Tournoud CreditAttribution: Damien Tournoud commentedChanging 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.
Comment #6
pwolanin CreditAttribution: pwolanin commentedminimal patch
Comment #8
pounardWhy do you want to replace
require_once
usingrequire
?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 likehook_boot()
or such).Here is a patch attached, that also fixes the wrong
drupal_static()
usage made infix_gpc_magic()
. If a miracle happens and this bootstrap method is run more than once, after thedrupal_static()
registry has been cleared, doing twicefix_gpc_magic()
would totally break superglobals content.Comment #9
pounardComment #10
Dries CreditAttribution: Dries commentedThis looks RTBC to me, but I want to give it bit more time for additional reviews.
Comment #11
pounardNice, thanks.
Comment #12
Damien Tournoud CreditAttribution: Damien Tournoud commentedLooks good to me too.
Comment #13
pounardIf 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.
Comment #14
Dries CreditAttribution: Dries commentedCommitted to CVS HEAD. Thanks.