Currently the VERSION constant is defined in system.module. This has an unfortunate effect that code (for example in a alternate cache include file) cannot determine the version of Drupal this is running. The version constant is not defined until the final phase of bootstrap, DRUPAL_BOOTSTRAP_FULL.

We should move this define to bootstrap.inc instead, so that the version of Drupal is always available from the start of the bootstrap process.

CommentFileSizeAuthor
#13 bootstrap_version-618938-d6-13.patch1.94 KBalbert volkman
#12 bootstrap_version-618938-11-d6.patch1.94 KBalbert volkman
#1 bootstrap-version-618938-1.patch1.73 KBpwolanin

Comments

pwolanin’s picture

pwolanin commented
Status: Active » Needs review
StatusFileSize
new bootstrap-version-618938-1.patch1.73 KB

Seems like we should move the minimum version info, etc as well.

simple patch - just cut from system.module and paste into bootstrap.inc

horncologne’s picture

horncologne commented
Status: Needs review » Reviewed & tested by the community

Looking good! Great idea, btw.

chx’s picture

chx commented

That's OK. The performance hit resulting from this code movement is likely to be negligible small. (However, APC has a speedup for defines and there is a hidef extension, too. But I think we still do not have enough defines for a hit.)

bjaspan’s picture

bjaspan commented

I certainly approve of the idea. Is it bikeshedding to suggest that the minimum support version of individual database engines does not belong at the same level in bootstrap?

pwolanin’s picture

pwolanin commented
Status: Reviewed & tested by the community » Needs review

Well, it seemed reasonable to me that we'd want to have that information available in DRUPAL_BOOTSTRAP_DATABASE. I guess you could make the case for moving it to database.inc instead, but keeping it together seems more sensible to me in terms of finding it in the codebase.

pwolanin’s picture

pwolanin commented
Status: Needs review » Reviewed & tested by the community
dries’s picture

dries commented
Status: Reviewed & tested by the community » Fixed

Committed.

pwolanin’s picture

pwolanin commented
Version: 7.x-dev » 6.x-dev
Status: Fixed » Patch (to be ported)
pwolanin’s picture

pwolanin commented
Version: 6.x-dev » 7.x-dev
Status: Patch (to be ported) » Reviewed & tested by the community

oops - looks like there were extra changes in the commit: http://drupal.org/cvs?commit=281932

dries’s picture

dries commented
Status: Reviewed & tested by the community » Fixed

Yep, that was by accident. I already updated the other issue.

pwolanin’s picture

pwolanin commented
Version: 7.x-dev » 6.x-dev
Status: Fixed » Patch (to be ported)

seems like it would make sense to do the same for 6 and 5?

albert volkman’s picture

albert volkman commented
Status: Patch (to be ported) » Needs review
StatusFileSize
new bootstrap_version-618938-11-d6.patch1.94 KB

Not sure if there's still interest in this after 2+ years, but here's a D6 backport.

albert volkman’s picture

albert volkman commented
StatusFileSize
new bootstrap_version-618938-d6-13.patch1.94 KB

Renaming patch.

Status: Needs review » Closed (outdated)

Automatically closed because Drupal 6 is no longer supported. If the issue verifiably applies to later versions, please reopen with details and update the version.