I'm trying to install drupal 7 (alpha 6 and 13 july dev). Before starting installation, i exports french transation archive from localize.drupal.org for drupal 7 and untar file in / site.

Then, i can't go after language selection screen anymore, wich return :
Fatal error: require_once(): Failed opening required 'drupal7/includes/database/translations/install.inc' (include_path='.:/usr/lib/php5') in drupal7/includes/database/database.inc on line 2122

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Stevel’s picture

Status: Active » Postponed (maintainer needs more info)

I used the generated export from http://ftp.drupal.org/files/translations/7.x/drupal/ and that worked fine (given I renamed it to languagecode.po). The file should be in the /profiles/profile/translations folder for the profile you use.

Could you provide the translation archive you used?

Stevel’s picture

Title: Error during installation un other language » includes/database/translations folder mistaken for database driver
Status: Postponed (maintainer needs more info) » Needs review
FileSize
1.45 KB

The translations folder matches the regex for database drivers, and thus is added to the list. The installer tries to load some classes from these database drivers causing the fatal error.

This patch verifies that the necessary files exists in the database driver before adding them to the list.

andypost’s picture

Stevel, just make Export from ldo. Generated file contains all strings from core so preferable way is import only strings for modules that you really using.

Related issues:

This one already bout this trouble #484442: Make Drupal 6 export format contrib friendly
#801028: One string in includes/database/schema.inc not in general.nl.po (Drupal 7.0-alpha4)

tstoeckler’s picture

Why do we not simply exclude the 'translations' folder from the regex?

Damien Tournoud’s picture

Status: Needs review » Needs work

Actually, we should search for 'database.inc' here. That would be more consistent with how modules, themes and profiles are detected.

andypost’s picture

Damien Tournoud, this function is related to install so better to add check for install.inc and change comment.

Actually this error thrown from db_autoload() on

function drupal_detect_database_types() {
.....
$class = 'DatabaseTasks_' . $driver;
$installer = new $class(); 
andypost’s picture

Status: Needs work » Needs review
FileSize
1.52 KB

Something like this

Status: Needs review » Needs work

The last submitted patch, 853748-detect-db-d7.patch, failed testing.

Stevel’s picture

Status: Needs work » Needs review
FileSize
17.78 KB

A simpler version which checks just for database.inc

Stevel’s picture

FileSize
808 bytes

Last patch contained a bit too much, reroll

andypost’s picture

@Stevel this bug caused by non-existent install.inc

Stevel’s picture

@andypost: The including of .../install.inc is what is displayed, yes. However this happens because there is no database driver at all, so there should not be any call to install.inc in the first place. (it qualifies more as a symptom than as a cause I think).

I don't really care what file(s) is/are checked for though...

@tstoeckler: That would solve this bug for this specific instance only, but the bug would reappear when eg. a folder includes/database/tests/ is added. So checking that there is indeed a db driver present is a more comprehensive solution.

chx’s picture

Status: Needs review » Reviewed & tested by the community

I did a lot of experimental work with DBNTG drivers (and not so experimental w mongodb dbtng) and this is definitely needed. It's debatable whether you want to check for database.inc or install.inc but as a matter of fact, database.inc should be enough. As the driver() function in connection will tell you what the postfix for classnames are used there is very little (if anything) that you can do without it so any sane driver will contain database.inc. And even if some wretched mind comes up with a driver that one does not have database.inc, we have enough zero-byte modules to prove that's not a problem, just add a zero byte database.inc (although even I could not come up with a driver which would make sense doing that...)

andypost’s picture

Status: Reviewed & tested by the community » Needs work

I think there's no debates because drupal_detect_database_types() *explicitly* require name() method in install.inc

  foreach ($drivers as $driver => $file) {
    $class = 'DatabaseTasks_' . $driver;
    $installer = new $class();
    if ($installer->installable()) {
      $databases[$driver] = $installer->name();
    }
  }

So while we are on it, let's make it fixed!

Stevel’s picture

Status: Needs work » Needs review
FileSize
807 bytes

If you insist...

chx’s picture

Status: Needs review » Closed (fixed)

While both work fine, still #10 is the RTBC patch because what andypost says only fires on install time AFAIK but

    $driver_class = 'DatabaseConnection_' . $driver;
    require_once DRUPAL_ROOT . '/includes/database/' . $driver . '/database.inc';

is in openConnection itself. I can imagine a driver that can only be added after install. I have a very vivid and wild imagination of what's possible with the DB drivers. But a driver you can't connect with, that's not working.

chx’s picture

Status: Closed (fixed) » Reviewed & tested by the community

Hm, i meant RTBC.

andypost’s picture

@chx this function has primary usage for install_settings_form() so it should return "installable" drivers.

So RTBC #15

EDIT: because this one lives in install.inc it can't be used in other places in core.

andypost’s picture

Status: Reviewed & tested by the community » Needs work

Doc block should be changed because this function detects only Drupal-aware not PHP drivers

Stevel’s picture

Status: Needs work » Needs review
FileSize
1.54 KB

Alright, following patch gives us a function that can detect both installable and usable but not installable database drivers, after long discussion on irc

Stevel’s picture

Noted a small error in variable naming. I desperately need a php refactoring plugin for vim :)

Stevel’s picture

Title: includes/database/translations folder mistaken for database driver » Folders in includes/database that do not contain an (installable) driver are used anyway.

Remains critical because it gives a PHP fatal error because of this when the "translations" folder is present (which is when installing Drupal in a different language).

andypost’s picture

We need DB name only for installer form.

system_requirements() does not need load installer class because only check for availability. Also this hunk a WTF because system_requirements() is called within already installed site so if no drivers defined this function could not be called.

Stevel’s picture

Dropping the driver without installer idea, each DBTNG driver just needs to make sure that there is an install.inc along the database.inc and possibly others.

We don't check that the database is enabled when we display the status report, because we simply won't get that far if we have no database.

andypost’s picture

Also we should add check for availability each driver as PDO when we are not in install phase

Stevel’s picture

This version checks for
- install.inc
- database.inc
- schema.inc

These files need to extend abstract classes, without which the driver cannot work.

Stevel’s picture

So schema.inc is not required when you are extending an existing driver, leaving it out of the patch.

andypost’s picture

I think $phase != 'runtime' should be $phase == 'install'

This more readable and tells exactly about phase this check related to

Stevel’s picture

Well, there's also $phase == 'update' where the check is important as well, as the user wasn't using PDO before upgrading, we need to check for it.

We won't to exclude the check from runtime, because then you need the database to be able to get to this part of the code, we do want this check in install, update and whatnot other maintenance mode.

tstoeckler’s picture

if ($phase == 'install' || $phase == 'update')) {}
Everyone happy now? :)

andypost’s picture

Status: Needs review » Reviewed & tested by the community

Exactly

webchick’s picture

Status: Reviewed & tested by the community » Fixed

Great work! Committed to HEAD.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.