Add a rebuild script

Haven't addressed above issues, but updated the patch to replace drupal_hmac_base64() with Crypt::hmacBase64().

How about something like this?

Sorry, forgot to add the .htaccess change to the actual patch.

I think we should add this but we needed the same thing for Drupal 7 and survived OK with drush rr in the end, so don't think it counts as critical.

3: 2097189-3.patch queued for re-testing.

Just settings as needs review, for...reviews. The bot is being funny atm, and we have no tests for this anyway.

Patch seems fine to me but I think we should add a rebuild link here

+++ b/index.php
@@ -9,4 +9,10 @@
+  print 'If you have just changed code (for example deployed a new module or moved an existing one) read http://drupal.org/documentation/rebuild';

Is it possible to write a test for this script?

1. +++ b/core/rebuild.php
   @@ -0,0 +1,43 @@
   +if (settings()->get('rebuild_access', FALSE) ||
   

   Should we add a few lines to settings.php about this new setting and maybe a warning on status page if it is enabled?

2. +++ b/core/rebuild.php
   @@ -0,0 +1,43 @@
   +  $GLOBALS['conf']['system.performance']['cache.page.use_internal'] = FALSE;
   

   Is the purpose of this line to disable page cache? If so, should we use drupal_page_is_cacheable(FALSE)?

3. +++ b/core/scripts/rebuild_token_calculator.sh
   @@ -0,0 +1,24 @@
   +if (PHP_SAPI !== 'cli') {
   

   Why not drupal_is_cli()?

Thanks both for the reviews.

#9, I am not against adding text to the exception message, I would personally prefer to just update the content on http://drupal.org/documentation/rebuild with these instructions, as having these on this page, or even a link to rebuild.php here would be strange IMO.

#10, good point, The last patch was a bit rough :) (I just fixed up the initial patch). Added the setting to default.settings.php with some docs, added a check to system_requirements, and others.

I think #12 makes sense form DX pov.

Yes, that seems like a good compromise. Doing that...

Sorry about the delay, this one went off the radar. How about something like this?

ok, let's try that again (Thanks chx!). chx also suggested just making them links, makes sense to me. Interdiff showing new changes.

How can I trigger this condition in order to test that the script works?

Could we move the main functionality of the rebuild.php script into a drupal_rebuild() function in bootstrap.inc, so that e.g. Drush can re-use the facility without having to duplicate it?

Here's what I mean -- no functional changes.

I'm not sure why a function like that should live in bootstrap.inc?

Ok, as there has been radio silence, how about we compromise here and add this to utility.inc instead? It is a utility function, and it's not in bootstrap then.

Works for me. As long as there is a helper function that can be invoked through other mechanisms, I'm happy.

Same patch as the previous, but sans the extraneous use statements in bootstrap.inc that are obsolete now.

23: 2097189-22.patch queued for re-testing.

Back to RTBC then.

Yep, +1 on #23 removing the use statements I forgot to remove. An interdiff would have been nice though... :)

Committed/pushed to 8.x, thanks! Could use a change notice for the new feature.

Added https://drupal.org/node/2153725

Oh, and realised we need #2153741: Add executable permission to rebuild_token_calculator.sh script

Thanks! I revised the change notice a bit to clarify there are two methods of running the rebuild script.

We can remove the php from the instructions for running the script once #2153741: Add executable permission to rebuild_token_calculator.sh script is taken care of.

Great work. Thanks everyone.

This needs a follow-up so that when Drupal doesn't bootstrap, it points you to this rebuild script instead of that stupid link in index.php to drupal.org (which will really not help if you're for instance on an airplain).

  $message = 'If you have just changed code (for example deployed a new module or moved an existing one) read <a href="http://drupal.org/documentation/rebuild">http://drupal.org/documentation/rebuild</a>';
  if (settings()->get('rebuild_access', FALSE)) {
    $rebuild_path = $GLOBALS['base_url'] . '/rebuild.php';
    $message .= " or run the <a href=\"$rebuild_path\">rebuild script</a>";
  }
 

This doesn't help if you didn't already have the rebuild access set to TRUE. It should tell you to set it to TRUE, like it does if you try to run update.php.

And I also noticed if I tried to run the rebuild script without it being set to TRUE it failed with no explanation, if the site was hosed in the first place -- I just got back to the unhelpful index.php message.

So I don't think this is really finished. I'd like to reopen but a follow-up would be OK too.

The change notice is also not showing up on
https://drupal.org/list-changes/drupal/

Why is that????? Oh, the Project wasn't set. I think it needs to be a required field. I'll file a separate issue.

#34, The purpose of this issue is finished: "Add a rebuild script" - added.

Feel free to open a followup and propose whatever changes there. I've said before, we seriously need to stop repurposing issues.

And I also noticed if I tried to run the rebuild script without it being set to TRUE it failed with no explanation, if the site was hosed in the first place -- I just got back to the unhelpful index.php message.

What else do you propose happens here? If your site is not functional, there is not much we can do here. That exception message is IT. We intentionally just redirect this script back to the site. Adding to the exception message is certainly an option.

Also, update.php has a full page with markup (basically). So might not be quite the same, as update is using drupal to render that. Some of which may not be possible if the dumped container is borked.

Yes, I know I know but the patch that was committed had bugs in it... Anyway, I opened a separate issue:
#2156287: Followup to new rebuild script - index.php messaging is incorrect/incomplete
as requested.

Not sure how that's a bug, but ok. most of what you're talking about I'd based on opinion.

Also, we just added to the message. We didn't add the d.o link.

Follow-up - Might be worth adding a settings.php array of PHPStorage bins that should be cleared. That way a site could clear more than twig and service_container. The script currently privileges those systems.

Possibly, Not sure most implementations using any php storage will be able to cripple a site like an out of date service container though?

The rebuild script failed to rebuild for me today and ended in a fatal error instead; follow-up issue:

#2159459: Rebuild script triggers errors in error handler, fails to rebuild container

https://drupal.org/documentation/rebuild needs to be updated

I think we could backport this to Drupal 7.

drush rr is fine, but not everyone has that.

Related, the Drush maintainers have said "no" to adding the Registry Rebuild functionality into the main Drush system: https://github.com/drush-ops/drush/issues/1744