Install.php does not correctly check the database settings [#299661]

the following docs from default.settings.php don't seem to match the behaviour of the installer:

 * For a single database configuration, the following is sufficient:
 *
 * $databases = array(
 *   'driver' => 'mysql',
 *   'database' => 'databasename',
 *   'username' => 'username',
 *   'password' => 'password',
 *   'host' => 'localhost',
 * );
 *
 * That is equivalent to:
 *
 * $databases['default']['default'] = array(
 *   'driver' => 'mysql',
 *   'database' => 'databasename',
 *   'username' => 'username',
 *   'password' => 'password',
 *   'host' => 'localhost',
 * );

when i use the following in settings.php for a fresh install of HEAD:

$databases = array(
  'driver' => 'mysql',
  'database' => 'databasename',
  'username' => 'username',
  'password' => 'password',
  'host' => 'localhost',
);

i get the following error:

In your ./sites/default/settings.php file you have configured Drupal to use a  server, however your PHP installation currently does not support this database type.

so, is the code wrong or is the doco wrong?

Comment	File	Size	Author
#11	databases.patch	3.88 KB	smk-ka
#11
#6	install.php_.patch	748 bytes	oadaeh
#6

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

Anonymous (not verified) CreditAttribution: Anonymous commented 25 August 2008 at 13:22

Status:

Active

» Closed (duplicate)

closing this, as it seems to be a duplicate of Select MySQL as default database driver, and fix a E_ALL warning during install.

sorry for the noise.

Comment #2

Damien Tournoud CreditAttribution: Damien Tournoud commented 25 August 2008 at 13:42

Status:

Closed (duplicate)

» Active

This has nothing to do with the other one.

Comment #3

Crell CreditAttribution: Crell commented 28 August 2008 at 09:44

Status:

Active

» Postponed (maintainer needs more info)

That looks like a bug in the code, because that fallback logic is supposed to work.

I have actually been pondering removing the fancy-schmancy fallback nesting. It just burns cycles on bootstrap, and with the installer 95% of users will never write their own settings.php file anyway.

Would anyone object greatly if we just got rid of the crazy fallback stuff? I don't think it serves any purpose now that we have an automated installer.

Comment #4

Damien Tournoud CreditAttribution: Damien Tournoud commented 28 August 2008 at 09:48

Crell: the fallback logic does not work in the installer, there is a:

$database = $databases['default']['default'];

in install_verify_settings()...

Comment #5

Anonymous (not verified) CreditAttribution: Anonymous commented 5 September 2008 at 12:15

i had a look at this, its not a trivial fix. i'm totally fine with removing/simplifying the fallback code.

EDIT: and i'd be happy to supply a doc fix patch if that's the way we decide to go.

Comment #6

oadaeh CreditAttribution: oadaeh commented 15 September 2008 at 15:18

Title:	docs in default.settings.php out of sync with code	» Install.php does not correctly check the database settings
Assigned:		» Unassigned
Status:	Postponed (maintainer needs more info)	» Needs review

File	Size
install.php_.patch	748 bytes

I think this problem can be fixed with a simple if/then conditional.

(I'm tempted to mark this as critical, because I wasted over an hour on it when attempting to do a "quick" 15 minute patch review and update.)

Comment #7

oadaeh CreditAttribution: oadaeh commented 15 September 2008 at 15:29

Assigned:	Unassigned	» oadaeh
Status:	Needs review	» Needs work

Okay, that wasn't enough. I'll see if I can find where else this needs to be corrected and upload a new patch.

Comment #8

oadaeh CreditAttribution: oadaeh commented 15 September 2008 at 16:41

The patch I provided (in #6) fixes the install problem, but doesn't fix the bootstrap problem that happens after Drupal is successfully installed. I've spent quite a bit of time following threads and have not yet been able to locate where it strays. I will have to come back to this later, as I need to leave for out appointments, unless the consensus is to just remove it.