Symptoms:
latest code from 8.x fails to install, throwing fatal PHP error Cannot use object of type stdClass as array in .../core/includes/install.inc on line 257
Steps to reproduce:
- Get the latest 8.x source code
- Copy sites/default/default.settings.php to sites/default/settings.php
- Launch installer
- Choose English. Press Save and Continue
- Choose Standard profile. Press Save and Continue
- Proceed to database connection details screen. Press Save and Continue
- Bingo. Error 500
Analysis
Error comes from line 257 in includes/install.inc:
if (isset($current[$index])) {
$current is a part of $settings array coming to drupal_rewrite_settings() from install_settings_form_submit():
(install.core.inc, lines 1109-1112)
$settings['databases'] = (object) array(
'value' => array('default' => array('default' => $form_state['storage']['database'])),
'required' => TRUE,
);
$current (part of $settings parameter) is an object, while drupal_rewrite_settings() expects array
I set this to critical, because this leaves installation in unusable state.
Update
Clean install without any settings.php goes just fine
Dropping existing database, and trying to re-install with automatically generated settings.php fails.
Follow-up Issues
Comment | File | Size | Author |
---|---|---|---|
#66 | already_installed.png | 49.58 KB | xjm |
#62 | 60-62-interdiff.txt | 1.5 KB | alexpott |
#62 | 1951068.install-error-reporting.62.patch | 6.29 KB | alexpott |
#60 | 57-60-interdiff.txt | 1.44 KB | alexpott |
#60 | 1951068.install-error-reporting.60.patch | 6.27 KB | alexpott |
Comments
Comment #0.0
valthebaldUpdated issue summary.
Comment #1
valthebaldShorter title
Comment #2
swentel CreditAttribution: swentel commentedI've tried every possible combination and all is fine here ..
Comment #3
valthebaldCan it be PHP-version specific? mine is 5.4.4
Comment #4
mgiffordI'm marking this issue I just posted a duplicate #1951866: Can't seem to install D8 this week
I'm using:
PHP 5.3.15 with Suhosin-Patch (cli) (built: Aug 24 2012 17:45:44) - on Mac.
As I mentioned in the other post, I'm also getting this with
drush si
using Drush 6:Figured leaving a stack trace from my error log couldn't hurt either.
Comment #5
valthebaldI see this as a side-effect of #1921818: Modify drupal_rewrite_settings() to allow writing $settings values. $settings was converted from array to stdClass, causing PHP Fatal error in drupal_rewrite_settings().
I wonder what would be the best way to solve this
Comment #6
swentel CreditAttribution: swentel commentedWait, is this both with Drush ? If so, then this is a non issue, drush isn't always up to date ... (and even then, it works fine here)
Comment #7
valthebaldNo, for me this happens both with drush and web-based install
Comment #8
valthebald@swentel: To fully reproduce this in existing installation, I also deleted contents of sites/default/files/php and sites/default/config*
Without deleting sites/default/files/*, dropping site database leaves it in WSOD state (which, I believe, is also an issue, although not as critical as this one)
Comment #9
catchThere's no support for head to head upgrades yet. #1921822: Take account of drupal_hash_salt during migration path from 7.x will handle this for 7-8 upgrades, so I don't think there's anything to do here.
Comment #10
valthebaldHEAD generates settings.php that cannot be used by itself more than once, so it's not exactly upgrade
Comment #11
chx CreditAttribution: chx commentedWhile I can't reproduce the fail. Nonetheless, reinstalling is impossible because if you empty your database and your files directory and you load any installer page then:
The whole requirements for config directories is a farce because Drupal won't get far if the config directory doesn't exist. It most definitely won't get so far as to be able to display a page...
Comment #12
valthebaldPatch from #11 fixed the problem! It seems that part of it rolls back changes announced in #1921822: Take account of drupal_hash_salt during migration path from 7.x, I wonder if that's fine
Comment #13
chx CreditAttribution: chx commentedNot sure what do you mean, that issue is not yet in and has 0% common with my patch.
Comment #14
valthebald#13: sorry, I meant #1921818: Modify drupal_rewrite_settings() to allow writing $settings values, which is in core already
Comment #15
chx CreditAttribution: chx commentedAs for that, I wrote it and I am confident I am not turning anything back, quite to the contrary, that hunk is a sort of followup so that instead of overwriting the whole of $database we override only $database['default']]['default']. What do you see as rolling back?
Comment #16
valthebaldThis move from array() to (object) array() and back confused me.
I saw that you are the author of mentioned patch, so if you're confident, so am I :)
Comment #17
webchickIt'd be great to get alexpott's sign-off on those changes to system.install, since he deliberately added them in #1741804-9: Implement a config import/staging directory.
Comment #18
mgiffordI've been struggling with this. Realized only now that if I just delete the settings.php file & recopy everything over from default.settings.php it might work. It did.
Anyways, would love this to be more straight forward. I was definitely running into object problems.
Thanks @valthebald & @chx for moving this ahead.
Comment #19
alexpottI have followed the instructions in the issue summary and I just can not get the bug to occur.
The following occurs with or without this patch...
If I...
Scenario 1
Fatal error: Uncaught exception 'PDOException' with message 'SQLSTATE[42S02]: Base table or view not found: 1146 Table 'drupal8alt.cache_config' doesn't exist' in /Users/alex/dev/sites/drupal8alt.dev/core/lib/Drupal/Core/Database/Connection.php on line 554
Scenario 2
SQLSTATE[42S02]: Base table or view not found: 1146 Table 'drupal8alt.users' doesn't exist: SELECT u.*, s.* FROM {users} u INNER JOIN {sessions} s ON u.uid = s.uid WHERE s.sid = :sid; Array ( [:sid] => sXjNqBZGb-PMaixy57hmZiBU3xTUJIoYrrRtMJ0soU8 )
Scenario 3
SQLSTATE[42S02]: Base table or view not found: 1146 Table 'drupal8alt.users' doesn't exist: SELECT u.*, s.* FROM {users} u INNER JOIN {sessions} s ON u.uid = s.uid WHERE s.sid = :sid; Array ( [:sid] => sXjNqBZGb-PMaixy57hmZiBU3xTUJIoYrrRtMJ0soU8 )
Scenario 4
Conclusions
EDIT: Improve the scenario instructions
Comment #20
alexpottFound another interesting thing...
Scenario 5
Comment #21
chx CreditAttribution: chx commentedNeither of these are addressed, or, I am afraid, going to be addressed.
As long as there is a config directory and settings.php points to it, you can't reinstall. Either delete the config dir or remove the config directories variables from settings.php.
Edit: what this patch fixes is, if I remember correctly, is the one where you empty out the database and the config directory but leave settings.php untouched.
Comment #22
valthebaldIf not addressing these issues is a (sad) given fact, it may be a good idea to document it somewhere (default.settings.php? )
Also, lowering level to major, since it affects only part of installations, and doesn't happen during "normal" install process
Comment #23
Berdir#11: 1951068_11.patch queued for re-testing.
Comment #25
chx CreditAttribution: chx commentedComment #26
valthebaldYeah, I was thinking of better title :)
Comment #27
alexpottWow this was fun...
Ok we have a problem with error reporting because that has a dependency of config so the real error gets hidden is config broken. We need to ensure that we don't call config before the system is ready or if we do then we catch the error.
Additionally if you go to core/install.php after a successful installation we expect a valid $user object in
user_template_preprocess_default_variables_alter()
Also we are not checking that the active configuration directory is empty of config before attempting installation... all sorts of oddities would are possible here...
This does not fix:
scenario 1,2 or 3 (which was a duplicate of 2) - deleting the database is not enough... you have to remove the active config too... the error that's reported is correct ie. "missing cache_config table". In order to make these scenarios display the drupal is already installed page is going to be very tricky as essentially at this point Drupal is broken... config + no db is madness.
This does fix:
The real scenario 5 - which is actually a bug as currently it is impossible to view the Drupal is already installed page!
Plus for drush users see #1954898: site-install fails to install over an existing site because the format of $config_directories in settings.php has changed
Comment #29
alexpottHo hum broke the interactive installer :(
Comment #30
chx CreditAttribution: chx commentedThat's good but I still think that we should remove that config directories requirements check -- in another issue, of course.
We all owe you a beer for making error reporting in install work again!
Comment #31
valthebaldWith patch from #29, I dropped all tables, but left database itself
Result: WSOD for the frontpage, core/install.php displays PHP exception.
Which is, of course, big improvement - there is clear indication that problem with installation lays in DB.
Yet, there is still no indication that to reinstall, I have to delete sites/default/files/config_* folder - install_already_done_error() is not called at this stage.
Comment #32
alexpott@valthebald... if you drop all the tables and don't delete your active config you've only done half the job... if you just drop cache_config you'll have the same issue. As stated in #27 I think this is the correct behaviour.
Comment #33
alexpottIn my opinion only dropping the database... is like in Drupal 7 terms... removing half of the tables in the database and expecting Drupal to still work...
Comment #34
valthebald@alexpott: that may be obvious for those knowing internals of D8, and differences between D7 and D8, but not, for example, regular developer finding his dev DB corrupted
Comment #35
chx CreditAttribution: chx commentedThis should go in and then we can try to do more following up. Recognizing the DB gone but the config directory is there situation is very tricky -- we need to recognize that while a connection can be made, there are no tables... and do this on-the-fly somehow because we most definitely won't want to connect to the DB just for this. Right now we detect that the databases global is not filled in at all and that's practically free. Also, if we do #1167144: Make cache backends responsible for their own storage which we totally should then the system will happily self correct the missing cache tables and then good luck figuring out where to figure out that this "DB dropped" situation happened.
Comment #36
valthebaldI don't suggest to handle every possible failure in code, but to provide site builders with complete information on how to react. This can reside in default.settings.php and/or d.o. documentation
Comment #37
chx CreditAttribution: chx commentedI retitled the issue to be more close to what gets fixed. It will help reinstall attempts and installer development by not needing a debugger any more with a breakpoint in drupal_exception_handler to figure out wtf broken.
To document the reinstall process, we can add to README/INSTALL/default.settings.php but any developer used to D7 will not read them. That's definitely a followup -- again, a very helpful and not too hard followup but the usefulness, alas, will be limited.
Comment #38
mgiffordI am having this problem, so was very eager to try the patch in #29.
I deleted the database, removed a wack of files that have given me grief in the past
sudo rm -fr c sites/default/files/*
and then went through the /core/install.php process and got:Repeated the whole process and got that again.
It was a nicer message though. Way better than the critical error I was getting before.
Comment #39
chx CreditAttribution: chx commentedThis needs to be unassigned because I do not think it's proper for Alex to commit his own patch (no matter how much I'd love to see this in).
Comment #40
BTMash CreditAttribution: BTMash commentedRan into the issue myself while trying to install D8 for the first time in a while. Here is a reroll of the patch since other changes made it in since March 27.
Comment #41
valthebaldPatch from #40 did not help.
After deleting DB tables - WSOD
After deleting DB tables and sites/default/files/* - installation starts, but WSOD after DB connection screen
Comment #42
BTMash CreditAttribution: BTMash commentedHmm, does error.log provide any details on the reason for WSOD? chx explains the only deleting db tables portion in post 35 but the WSOD after db connection screen makes less sense since that is where I was having the problem myself and the patch fixed the issue for me.
Comment #43
valthebaldMy bad, I didn't apply *any* patch after the last git pull (facepalm)
After applying, patch from #40 behaves much like #29 - WSOD after dropping tables, the following line in error.log:
After deleting files in sites/default/files, Exception text in browser screen:
Comment #44
BTMash CreditAttribution: BTMash commented@valthebald, Can you try dropping all tables from the db and removing files in sites/default/files? It *seems* like some tables were created between first dropping the tables and later removing the files.
Comment #45
valthebald@BTMash: just checked, no tables in DB
Comment #46
valthebaldI was able to track the problem down until theme.maintenance.inc, line 53:
$custom_theme = variable_get('maintenance_theme', config('system.theme')->get('default')) ?: 'bartik';
Before that, there is normally handled database exception. Since error is fatal and drupal cannot be started, maintenance theme is initialized by _drupal_maintenance_theme().
If we get back to line 53, it's a key to Pandora box:
config() calls drupal_container(), which returns current DC. Since failure is early, DC is empty, drupal_container() returns NULL, and config() fails with fatal PHP error "Call to a member function get() on a non-object"
I dare say it's a bug in drupal_container() (it's return value declared to be Drupal\Core\Config\Config, not mixed Drupal\Core\Config\Config or NULL). If it's so, it's possible to handle the case of missing DC and return some dumb config object. Does that make sense?
Comment #47
chx CreditAttribution: chx commentedBurdening config() with checking like that for the sake of this extreme case edge case, I do not think so.
Comment #49
BTMash CreditAttribution: BTMash commented#40: 1951068.drupal-reinstall.40.patch queued for re-testing.
Comment #50
BTMash CreditAttribution: BTMash commented#47: 1951068_47.patch queued for re-testing.
Comment #51
BTMash CreditAttribution: BTMash commentedI reviewed the patch and code/comments look good (the creation of
_drupal_get_error_level
is a nice touch - I'm assuming it is named the way it is so it could be used elsewhere?). I tested it locally and get the errors in appropriate places (tried without db but with config dir and after full purge) and so far, works as expected. I ran the simpletests again as the patch failing seemed off and turns out that was the case. This is a +1 towards RTBC from me.Comment #52
chx CreditAttribution: chx commented_drupal_get_error_level is a function because it's code used twice not because I'd want anyone else to call it.
Comment #53
mgifford@chx's patch still isn't letting me install D8. It's been an annoying problem that's really slowed down what I can review. It's even worse now that SimplyTest.me doesn't work for D8.
Comment #54
chx CreditAttribution: chx commentedI do not know what to say / do . The error reporting patch is correct, it helps tremendously. I am sorry for those who can't reinstall but this is a first step towards that -- and that is not supported AFAIK anyways.
So, can we get this in so that I don't need to apply every time I want to develop anything?
Comment #55
chx CreditAttribution: chx commentedNote that I reinstall with
mysql -u root -e 'drop database systemdrop;create database systemdrop' ; cp sites/default/default.settings.php sites/default/settings.php; sudo rm -r sites/default/files/*
.Comment #56
mgiffordI could install the code when I just copied the default.setting.php over the settings.php
Hadn't occurred to me to try that along with removing the other bits that D8 is adding and needs to be removed for a re-install.
Comment #57
alexpottPatch attached improves the doc block for
_drupal_get_error_level()
function, swaps outconfig()
forDrupal::config()
and does a minor optimisation in_drupal_maintenance_theme()
.I've raised the priority of this patch to major because actually debugging changes to the installer or pre DRUPAL_BOOTSTRAP_KERNEL is really difficult without it.
Comment #58
alexpottNow for the patch :) referred to in #57
Comment #59
BerdirIf you currently try to access core/install.php and the site is already installed then it explodes with a fatal error about not being able to call __clone().
Applying this patch fixes that and displays the previous and slightly adjusted error messages.
It also fixes a number of other issues, e.g. the error display configuration (which leads to another, misleading error when an error happens, see also https://twitter.com/beejeebus/status/315017933531279360 ;)) ) and the fatal error in theme.maintaince.inc, for which I've seen multiple bug reports already.
This might not be perfect yet, it for example doesn't yet track the situation where you have an existing settings.php that points to an empty database (see #55/56) but we can look into that in a follow-up issue. The only thing that we could maybe do is also mention it in the error message that we display. Basically put the commands in #55 in words. But with or without that, I think this ready to get in and important to do so asap.
Comment #60
alexpottOn @berdir's suggestion in #59 and IRC improved the already installed message to cover steps outlined in #55
Comment #60.0
alexpottUpdated issue summary.
Comment #61
xjmWe should be consistent in how we format
settings.php
(looks like it's in an<em>
tag in the next line).Comment #62
alexpottGood spot @xjm...
Comment #64
YesCT CreditAttribution: YesCT commentedterm test fail seems unrelated.
Comment #65
YesCT CreditAttribution: YesCT commented#62: 1951068.install-error-reporting.62.patch queued for re-testing.
Comment #66
xjmLooks good if the retest is good.
Comment #67
valthebaldPatch looks great!
I would only add another item in the list of suggestions (between "To start over..." and "To install to a different...":
Comment #68
xjmFor #67, instead of adding a whole new bullet, I'd add the bit about deleting active configuration and removing active configuration variables to the second point. If we put too much overwhelming information on this screen, users will not read it, and it's already up there.
Comment #69
valthebald@xjm: amending second bullet instead of adding the new one sounds good, as far as it is clear that using existing settings.php is, in fact, possible.
Comment #70
xjm#69 also isn't really in scope here, so let's address it in a followup issue. @valthebald, will you file it?
Comment #71
valthebald@xjm: opened #1963760: Make sure install.php gives accurate information if Drupal is already installed
Comment #72
xjmThanks @valthebald!
Comment #73
webchickWell that definitely seems far preferable to spewing blood. :)
Committed and pushed to 8.x. Thanks!
Comment #74.0
(not verified) CreditAttribution: commentedadded follow-up