Reduce index.php for easier upgrading

Th patch now contains both core/includes/index.inc and core/index.inc.

#7: drupal-1831998-7.patch queued for re-testing.

Do we really want to add one more include statement into the critical path? If it's that important that we have all the core PHP code in /core, is there a way we can do this with the .htaccess?

#14 Regarding .htaccess both nginx and IIS don't have such a thing.

This is an important issue, but I don't believe it's a major task (see http://drupal.org/node/45111 and http://drupal.org/node/1181250). Specifically, this was marked "major" to indicate that several long-time Drupal contributors agree it's a good idea, but that's not really related to issue priority... I mean, you could probably also get them to agree that fixing spelling mistakes in Drupal's code comments is a good idea (which it is!) but that's not the same thing as saying that spelling mistakes in code comments have a significant effect on Drupal's functionality :)

If you disagree and feel this must still be marked major, please provide a very specific justification and keep in mind that every major issue prevents other features that people are working hard on from getting into Drupal core, due to the issue queue threshold policy (until/unless #1810428: [Policy, no patch] Adjust major and critical task thresholds during code thaw is addressed, at any rate). Note that I am not singling out this issue for particular attention but rather (slowly) trying to go through the issue queue and do this in many places so that other features still have a chance to get into Drupal core soon without the thresholds getting in the way. I am trying my best to be impartial, and of course, just because an issue isn't marked "major" certainly doesn't mean it's unimportant to work on. You can follow my progress via the tag I've just added to this issue :)

I think this might almost qualify as major on security grounds, but I'm not sure, so using the "Security improvements" tag instead. If the plan is that once 8.0 is released, that the way a site updates to 8.1 is by just copying the "core" directory, then us not being able to fix bugs in index.php after 8.0 is released could conceivably end up creating security problems. Though the same is as true (or maybe even more true) with .htaccess and web.config.

Agreed with effulgentsia that this could be an upgrade-WTF that would leave a lot of sites vulnerable. Whenever we have "special note: X" in security advisories there are a large number of people who don't notice that special note.

Granted it is uncommon to need to update index.php for any reason, but this would help reduce it.

.htaccess and web.config (and sites.php, settings.php) are already somewhat special cases since folks often customize them meaning that they purposefully don't replace them during an upgrade.

I think tags with forward slashes break the taxonomy autocomplete. Fun.

I think tags with forward slashes break the taxonomy autocomplete

Fixed in D8 and D7. Backport to D6 not yet committed: #93854: Allow autocompletion requests to include slashes.

I think the basic idea of moving more code into core/ makes sense, but having an included file automatically run code seems counter to the way Drupal normally does things (and also makes it a little harder to follow).

What about doing this more like Drupal 7, reducing the code as much as possible but not removing it? So for example (ignoring the code comments) index.php could look like:

  define('DRUPAL_ROOT', getcwd());
  require_once DRUPAL_ROOT . '/core/includes/bootstrap.inc';
  drupal_bootstrap(DRUPAL_BOOTSTRAP_CODE);
  drupal_handle_request();

where drupal_handle_request() is a new function that lives in boostrap.inc and contains all the other things.

This might also help make it easier for someone trying to write their own script which bootstraps Drupal to understand how to do it using index.php as an example.

Since this does leave a few lines of code in index.php, in theory it could mean there's a Drupal 8 point release that requires changing it, but given what that code looks like I would say the chances are pretty much zero that it would ever happen.

#23 sounds good to me. The only part of it that seems potentially volatile is the DRUPAL_BOOTSTRAP_CODE constant, but #1784312: Stop doing so much pre-kernel bootstrapping is working on changing that to DRUPAL_BOOTSTRAP_CONFIGURATION, and my hope is that once page cache and session handling have moved to the Symfony architecture, that DRUPAL_BOOTSTRAP_CONFIGURATION is the only bootstrap phase left, at which point, we can drop the constant entirely.

I also wouldn't be opposed to removing the drupal_bootstrap(WHATEVER_CONSTANT) from index.php entirely, and moving it into drupal_handle_request().

Looks good to me. Small nit:

+++ b/core/includes/bootstrap.inc
@@ -2180,6 +2181,21 @@ function drupal_bootstrap($phase = NULL, $new_phase = TRUE) {
+ * Handle an incoming request. The routines here dispatch control to the
+ * appropriate handler, which then prints the appropriate page.
+ */
+function drupal_handle_request() {

Per http://drupal.org/coding-standards/docs#functions, I think the comment could be shortened to just "Handles an incoming request." The second sentence is a leftover from D7, but isn't all that meaningful for the new Symfony-based code. Possibly, some Symfony-oriented docs/links would be appropriate here, but that can be done in another issue.

#31: drupal-1831998-31.patch queued for re-testing.

I renamed drupal_handle_request() to drupal_handle_entire_php_request(),
moved drupal_bootstrap(DRUPAL_BOOTSTRAP_CONFIGURATION) into it, added more documentation to it, and updated http.php and https.php to use it. I'm willing to call this RTBC if c4rl approves of my changes.

I think it helps indicate that it's taking over the entirety of PHP's responsibility for handling the request, as opposed to Kernel::handle(), which has other stuff before and after it.

I think this is a great improvement, and it's even more clear than D7's index.php

"Entire" and "PHP" might be somewhat redundant, but I wanted to convey two things:

1) That we don't mean request in the Symfony use of the word (i.e., handling a $request object), but rather in the PHP sense of the word (the thing that caused PHP to be running a script).

2) That we handle everything, from bootstrapping at the beginning to exiting at the end.

Really? This seems like it comes from the redundant department of redundancy to me. :) Am I missing something?

Why would I assume it were handling a JavaScript or Node.js request? I wouldn't. So why is "php" necessary?
Why would I assume it wasn't the entire request? It's the only function in that file. It must therefore be handling the entire request.

It's true we're using Symfony Request objects as part of the overall PHP request, but that's a detail that doesn't become apparent until you're already inside the function, which is a lovely place to note (as the PHPDoc of that function currently does in #35) that you're talking about the entire PHP request, and not Symfony's. There isn't even mention of Symfony at all anymore in index.php (namespaces, etc.), so I find the extra details in the function name there just confusing.

So I'd rather commit #31, with the PHPDoc of #35. What say you?

Yep, #39 helps make the case for the shorter name.

+++ b/core/modules/system/tests/http.php
@@ -21,21 +18,7 @@
+drupal_handle_entire_php_request(TRUE);

Missed this one. This fixes it. Wait for green before commit, but otherwise, this is RTBC.

Looks good to me. Committed/pushed to 8.x.

Sorry for the noise; I'm told that slashes in tags can break the autocomplete on drupal.org.