Installer on PHP <5.3 results in parse error instead of warning about old php

Sounds good. That would need a comment in the PHPDoc, though, explaining why we use define here, when we use const everywhere else.

+++ b/core/install.php
@@ -15,8 +15,9 @@ define('DRUPAL_ROOT', getcwd());
+ * Use define() so PHP <5.3 can parse this file and display requirements.

There needs to be a blank line before this. Also I think the comment could be more clear. Maybe: "Use define() so that on PHP versions before 5.3 the installer can display a proper warning instead of a blank page because of a fatal error."
That's actually too verbose, I think, so if you could come up with a shorter version, that would be even better. :)

0 days to next Drupal core point release.

To clarify, the intent here is to prevent the installer from crashing before it displays the requirements information.

The patch looks good, but I'd suggest one change:

+++ b/core/install.phpundefined
@@ -16,7 +16,10 @@ define('DRUPAL_ROOT', getcwd());
+// Use define() instead of const so PHP versions older than 5.3 can display the
+// proper PHP requirements.
+define('MAINTENANCE_MODE', 'install');

The two lines of inline comments here should be moved to the docblock (after a blank line). I.e.:

/**
 * Global flag to indicate that site is in installation mode.
 *
 * Use define() instead of const so PHP versions older than 5.3 can display the
 * proper PHP requirements.
 */

Oops, xpost.

Ah, that's exactly why I thought it should be in the docblock. :) We have additional paragraphs in constant doxygen all over the place, but an inline comment after the docblock isn't a pattern I've seen.

#10 looks good to me.

I think I understand Kjartan's reasoning here; this documentation is definitely going to look a little odd at http://api.drupal.org/api/drupal/core--install.php/constant/MAINTENANCE_... the way it is now.

If this were a function, the new sentence would definitely be an inline code comment rather than part of the function's API documentation. However, if we don't do that anywhere else there's probably a good chance that api.drupal.org wouldn't parse it right, so maybe it's not worth experimenting.

To make it a little more clear on a page like that, the beginning of the sentence could be changed to something like "This constant is defined using..." That way someone who comes across it in the API docs will not be confused that the "Use define()..." is some kind of instructions for how they are supposed to use the constant in their own code. It might be worth a quick reroll, but I don't care quite enough to un-RTBC it :)

If the version check goes at the top of the file, can we still have const instead of define?

As usual, David's suggestion makes way too much sense. :) Here's a reroll with that.

Tentatively leaving at RTBC 'cause, yeah. If you think it shouldn't be, please set it back. ;)

What about moving it to core/includes/install.core.inc ?

Moving it to install.core.inc would limit the ability of outside code to use the functions in that file. Granted, most of those functions aren't too useful if you're not in the middle of an installation anyway, but I think it would still be better not to limit it.

Thanks, committed/pushed to 8.x.

Filed follow-up #2906787: Move definition of MAINTENANCE_MODE = install to install_drupal