Installer for Drupal Core

Looks like at least three issues crammed into one.

"-Ankur, Jeremy, and myself reviewed the environment issues that have arisen with the CivicSpace installer and the integration tasks related to integrating CiviCRM with Drupal."

Why CiviCRM? Drupal core doesn't care about it.

"-PHP Safe mode, PHP Register Globals, PHP version check, PHP memory size, PHP mail compiled in"

Drupal should work with safe mode. Memory is highly dependant on what modules you have.

"-Apache mod_rewrite, gzip, allow override for .htacces"

.htaccess is actually optional too. So is Apache.

"-Drupal core should have one .install file for installing core."

Right now it is database/updates.inc.

"-Drupal core module should have their own independent .install file for updates checked"

Thats a large directory structure change. Have fun with that.

"-the install hook should have a help function that can render help themed as the DB connections errors are presented today."

Wuh? You mean script, not hook, right?

"-the install hook should be in a top level file which will be called from install.php and the contents will be included in a file install.inc kept with update.inc"

Put the one line of code in a function in install.inc if you want.

Here's a patch for install.inc to offer code toward this goal. Essentially this is a collection of helper functions for the installer to do things like verify that php.ini is properly configured. The goal is to define core requirements, but to also allow modules to define their own requirements (as well as "install profiles").

The first installer helper function is used to auto-generate $base_url using PHP predefined variables (aka superglobals). This allows the installer to pre-fill $base_url for the user (who could then change it if necessary).

The second collection of helper functions is used to verify php.ini settings. Refer to drupal_verify_php_requirements() to see how the helper functions are used to verfiy the php version, safe_mode, register_globals and memory_limit. Modules would define additional requirements in their .install files, as necessary.

The third helper function is used to detect all available database types (those that are compiled into PHP and are recognized by Drupal).

Please offer feedback. If the general feeling is that this is a good approach, I will continue down this path to verify file permissions, and will add install.apache.inc and install.mysql.inc to offer application specific tests as well.

If you'd like to test this on your server, patch install.inc and then add the following two lines to index.php after DRUPAL_BOOTSTRAP_FULL:

require_once './includes/install.inc';
drupal_verify_php_requirements();

(This is just for testing. In actuality, this call will be made from install.php, not index.php... You can then play with changing DRUPAL_MINIMUM_PHP and DRUPAL_MINIMUM_MEMORY at the top of install.inc to see what happens...)

I'm going to close an issue I opened in January related to this.
http://drupal.org/node/45166

You could also check for required PHP extensions.

Here's some code that can be used to check for SQL engine versions:

function get_mysql_version() {
  if (function_exists('mysql_get_server_info') && ($version = @mysql_get_server_info()) && !empty($version)) {
    return $version;
  }
  if ($row = db_fetch_object(db_query('SELECT VERSION() AS server_version'))) {
    return $row->server_version;
  }
  return FALSE;
}

function get_pgsql_version() {
  global $active_db;
  if (function_exists('pg_version') && ($version = @pg_version($active_db)) && isset($version['server_version'])) {
    return $version['server_version'];
  }
  if ($row = db_fetch_object(db_query('SELECT VERSION() AS server_version'))) {
    return $row->server_version;
  }
  return FALSE;
}

It is code extracted from something I wrote sometime ago, here adapted to Drupal DBAL. I have tested the MySQL check, but I haven't had the oportunity to test if the pg version works.

At the moment I'm trying to define an InstallAPI that is modeled after the FormAPI, moving away from hard coded checks. The goal is to allow module's to use this InstallAPI in their .install files. (How we actually determine the various versions and settings is handled underneath the API. MySQL specific stuff would be handled in install.mysql.inc, PostgreSQL specific stuff would be handled in install.pgsql.inc, Apache specific stuff would be handled in install.apache.inc, etc... )

At this time, the API I have defined is a fairly limited proof of concept and only works for checking PHP info. In any case, to illustrate what I'm trying to do here is a snapshot of code that checks for core Drupal PHP requirements:

<?php
  $settings['version'] = array(
    'required_version' => DRUPAL_MINIMUM_PHP,
    'message'        => t('Drupal requires PHP version %required_version or greater.  You are using PHP version %installed_version so Drupal will not work properly on this installation.'),
    'message_type'   => 'error'
  );

  // verify safe_mode
  $settings['safe_mode'] = array(
    'required_value' => 0,
    'message'        => t('PHP safe_mode is currently enabled.  You must disable safe_mode for Drupal to properly function.'),
    'message_type'   => 'error'
  );

  // verify register_globals
  $settings['register_globals'] = array(
    'required_value' => 0,
    'message'        => t('PHP register_globals is currently enabled.  As of Drupal 4.2.0 it is advised that you disable register_globals.')
  );

  // verify PHP memory limits
  if (function_exists(memory_get_usage)) {
    // memory_get_usage() will only be defined if PHP is compiled with 
    // the --enable-memory-limit configuration option.
    $settings['memory_limit'] = array(
          'required_value' => DRUPAL_MINIMUM_MEMORY,
          'operator'       => '>=',
          'message'        => t('Your PHP installation limits Drupal to using only %current_value of RAM.  It is suggested that you modify the memory_limit directive to allow at least %required_value of RAM.'),
          'shorthand'      => TRUE
    );
  }
?>

The next step is to expand this API to support things other than php. What I envision for the expanded API would turn the first example above into something like this (a two-dimensional array):

<?php
  $install['php']['version'] = array(
    'required_version' => DRUPAL_MINIMUM_PHP,
    'message'        => t('Drupal requires PHP version %required_version or greater.  You are using PHP version %installed_version so Drupal will not work properly on this installation.'),
    'message_type'   => 'error'
  );
?>

At that point the same API would be used to verify version and configuration information for other applications. For example, install.apache.inc and install.mysql.inc would expand this API so the following would verify the minimum version of Apache and MySQL:

<?php
  $install['apache']['version'] = array(
    'required_version' => DRUPAL_MINIMUM_APACHE,
    'message'        => t('If using Apache with Drupal, you must use version %required_version or greater.  You are using Apache version %installed_version so Drupal will not work properly on this installation.'),
    'message_type'   => 'error'
  );
  $install['mysql']['version'] = array(
    'required_version' => DRUPAL_MINIMUM_MYSQL,
    'message'        => t('If using MySQL with Drupal, you must use version %required_version or greater.  You are using MySQL version %installed_version so Drupal will not work properly on this installation.'),
    'message_type'   => 'error'
  );
?>

Finally, this entire API would be available to all modules. Thus, each module would be able to define minimum requirements / configuration settings in their .install files.

The attached patch provides the environment verification API described earlier in this issue. It also begins to define an installation bootstrap process. A quick file-by-file description follows:

install.php:
- new file, launches the install bootstrap logic

install.inc:
- drupal_install() is a bootstrap process similar to drupal_bootstrap(). It first verifies the install environment (defined by system.install and/or profiles), then settings.php and finally the database. From there it allows the selection of a profile (specific themes/modules/etc) and completes the installation. At this time only the environment checks are implemented.
- drupal_verify_install() calls the appropriate verification function, including files as needed
- drupal_verify_install_php() verifies PHP minimum requirements
- drupal_verify_install_file() verifies file permissions
- drupal_verify_install_function() verifies that required functions exist

install.apache.inc
- drupal_verify_install_apache() verifies apache requirements/configuration

install.mysql.inc
- drupal_verify_install_apache() verifies MySQL requirements/configuration

theme.inc
- adds install_page, very similar to maintenance_page (does not require database)

system.install
- defines Drupal core requirements using new API similar to the formsapi. Soon third-party modules will be able to put their own requirements in their own .install files. Additionally, profile requirements will be able to be defined in .profile files.

This is a work in progress, however reviews and comments would be very much appreciated.

Looks very nice.

One comment:

I believe apache_* functions are available only when PHP is running as an Apache module.
http://www.php.net/manual/en/ref.apache.php

Not sure, but the _using_apache() function may return true? $_SERVER['SERVER_SOFTWARE'] may still contain the word "apache"?

I believe apache_* functions are available only when PHP is running as an Apache module.

Yes, that is correct. Both install.apache.inc and install.mysql.inc need to be greatly improved -- up until now I've been primarily focused on designing the framework. I'd greatly appreciate patches/suggestions with code for more accurate and/or additional checks within the framework.

Thanks for reviewing. :)

Here is a zipped copy of today's HEAD and this patch already applied, for those for whom the word 'patch' strikes indelible fear. ;)

This should hopefully make it easier to get a broader range of people tesitng.

Jeremey, awesome work!!! Testing out now on Windows XP with Apache 2.0.55, PHP 5.1.2, MySQL 4.1.7 I'm running into trouble.

The following errors must be resolved before you can continue the installation process:

    * The cron.php file provides automation for Drupal functionality. Most installations will want this file properly installed. The file should be readable, and should not be writeable (as that would be a security risk).
    * File "./index.php" is writeable but should not be.
    * The index.php file is Drupal's main interface with the world. This file must be readable, and must not be writeable (as that would be a security risk).
    * File "./xmlrpc.php" is writeable but should not be.

Obviously, on Windows permissions are handled entirely differently. Now what to do about it, should there be an OS check and if Windows, by pass these checks and possibly inform the user that they need to properly secure their server a different way? Or, should there be a way to bypass these checks with some sort of checkbox or something?

On another note, is the settings.php generated from this script?

I see one usability problem, users are going to drop Drupal into a directory and try and visit the site, by doing this they will likely get an error 'Unable to connect to the database server' ... perhaps that warning should be updated with a link to run install.php? ... however, this could be a security risk, what if Drupal.org database server went down and that link was present? Any means to stop this?

Obviously, on Windows permissions are handled entirely differently. Now what to do about it, should there be an OS check and if Windows, by pass these checks and possibly inform the user that they need to properly secure their server a different way? Or, should there be a way to bypass these checks with some sort of checkbox or something?

http://php.net/is_readable and http://php.net/is_writable should work on windows, although the file would have to be marked "read only" instead of 755/555

On another note, is the settings.php generated from this script?

That would be, IMHO, a required feature. Also, the install can check to see if install.php already contains values to prevent itself from running.

I see one usability problem, users are going to drop Drupal into a directory and try and visit the site, by doing this they will likely get an error 'Unable to connect to the database server' ... perhaps that warning should be updated with a link to run install.php? ... however, this could be a security risk, what if Drupal.org database server went down and that link was present? Any means to stop this?

Drupal can check to see if values are written in settings.php, if none are, then it can directly switch to the installer, like Mam *cough* bo does.

Get work goes on here, keep it up!

Robin

Obviously, on Windows permissions are handled entirely differently.

Those error messages say that is_writeable() is returning true for index.php, cron.php and xmlrpc.php. Try opening them in windows explorer and look at the file permissions -- is it read only? Or read/write?

As the web server process has write permissions to those files, it should be able to auto-remove the write permissions. That will make it less confusing for Windows users that are perhaps less used to dealing with file permissions. I'll test that out.

On another note, is the settings.php generated from this script?

Not yet. The first phase of the install bootstrap is checking the environment, that's basically all this script does yet. The second phase is validating/configuring settings.php. I'm working on that at the moment, and will upload a new patch when it's completed. The third phase will be getting connected to the database.

Also, the install can check to see if install.php already contains values to prevent itself from running.

At the end of the installation process, I intend the installer to set a variable in settings.php which indicates an install succesfully completed, and then to make settings.php read only...

This will be a nice addition, but only after 4.7 is released. It is simply too late to add this now.

I only did a quick read through of the code.

I'd like to see theme_maintenance_page used instead of making a new function.

New strings should have one space between sentances. This is the best thing to do for variable width fonts and browsers collapse the whitespace anyway.

"This will be a nice addition, but only after 4.7 is released. It is simply too late to add this now."

You're probably right, but Dries indicated he'd still consider an installer for Drupal 4.7 at Vancouver so I'm trying to complete this as quickly as possible. If it's too late, it's still worth getting working for consideration after we open up for new features...

I'd like to see theme_maintenance_page used instead of making a new function.

The maintenance_page theme doesn't quite work, as errors and warnings are just dumped onto the screen. I'm hoping to have a much prettier install_page before this patch is finished, with an external install.css. I'd like the installer to look nice, for things to be clear to an inexperienced user, and for Drupal distributers to brand their own installers, etc.

New strings should have one space between sentences. This is the best thing to do for variable width fonts and browsers collapse the whitespace anyway.

Okay. 24 years of teaching myself to press space twice after a '.' is difficult to forget. I can just do a replace in vi after the patch is finished if the double spaces are really that problematic. Thanks for looking the patch over!

Here's an updated version of the installer patch. In addition to verifying the environment, it also configures settings.php. (the function that rewrites settings.php will be made more generic so that it can be used for other settings too, but for now I just want to be sure this method works under multiple installation environments)

Other changes include detecting the approriate settings.php (rather than hard coding default/settings.php), and trying to fix file permissions that are wrong.

Note that the settings.php form that is displayed to the user does not use the formsapi -- I tried to do this for quite a while, but gave up as the formsapi requires the database. (I was able to add some if/else statements to force it to work without the databaes, but it got quite ugly and really isn't worth it for just one page) All other form pages generated by the installer should be able to use the formsapi, as the next bootstrap step is initializing the database.

Please test -- try configuring settings.php with the installer patch applied and see if it works for you on your OS. (I'm offline now for two days, I'll reply to any followups on Friday or Saturday)

Ack, brown paper bag release. That patch was bad, please use this one for testing the settings phase.

Just looking at the code, at the top of install.inc there is:

foreach (module_list() as $module) {

...that's before anything else? ...but module_list() needs access to the DB. Is that a problem or missed something?

Another comment:
define('DRUPAL_MINIMUM_MYSQL', '3.23'); // if using MySQL

I'm not sure MySQL 3 supports some of the queries used here and there. I might be wrong here as well.

...but module_list() needs access to the DB

Grab the next version of the patch. I was "cleaning up" and removed a global I shouldn't have. During the initial bootstrap we're unable to call module_list() as you've noted.

I'm not sure MySQL 3 supports some of the queries used here and there.

As far as I know, we still support MySQL 3.23. According to this page we support 3.23.17 or later. I've not heard an official decision to require MySQL 4 yet.

[pardon the null message, i just wanted to get subscribed to this issue]

I tried a test run and install.php crashes on an error trying to do an include_once() of ./modules/system.install (somewhere in the code install.inc). I think it should really be an include_once() of ./system.install.

Also, it when I fixed the above and loaded install.php, I got a warning in red about a missing "./includes/install.apache.inc", but I'm guessing that is something that'll come later.

-Ankur

This sorta traffic is why I don't see this being ready for 4.7.

It seems like we should add features to drupal_set_message() and make those availiable for theme_page() and theme_maintenance_page(). Distributions are free to retheme those as needed (or at least should be). Otherwise, you may consider that drupal_set_message() isn't the right thing to be using. I'd guess including a distribution-specific theme file and doing something in settings.php would make that work without a database and without modifying core files.

http://delocalizedham.com/node/80

To simplify the testing of this patch as it evolves, I've created a directory on my webserver in which I'm placing a pre-patched version of Drupal CVS HEAD.

install.php crashes on an error trying to do an include_once() of ./modules/system.install

I'm wondering if maybe there was a problem applying the patch? system.install lives in the modules subdirectory, so there shouldn't be a problem trying to include it from there. Please try getting a prepatched Drupal tarball here and let me know if the problem goes away.

a missing "./includes/install.apache.inc"

My bad. :( The script I quickly wrote to generate the patch had a typo. I'm attaching another fixed patch.

Okay, I'll really be offline

The patch as it is now is a big monster... there is no way that it is going into 4.7 like this.

I agree with Neil that it should be split up.

My biggest concern is that it adds a bunch of requirements testing without doing anything with the checks we already have in place. For example, all the database.??sql files already check for the existance of the relevant foo_connect() function. Unicode.inc checks the version of the PCRE library. PgSQL checks for the database character set encoding. Admin/settings checks for permissions of directories and does that HTTP GET check to see if Clean URLs work.

Many hosts have a nasty tendency to do unannounced updates, which result in sites suddenly 'not working anymore' (don't you love those kind of forum posts?). So we need to verify these conditions not just on install, but during the site's lifetime as well.

IMO the install requirements checks should be merged with the run-time checks, and presented on a separate status page in the admin. This patch can be done orthogonal to the installer itself. The conditions that can be verified on install are merely a subset of the total checks.

Code comments:

• PHP does not have a function type, use a quoted string when using function_exists().
• The MySQL/PGSQL version checking mechanism feels very convoluted and wholly inconsistent. For example, install.inc contains explicit checks for the different foo_connect() functions (making it db-specific). But then you go through a huge amount of trouble to specify the db version requirements in system_requirements_pre() and check them in install.foosql.inc. It seems it would be cleaner to just hardcode the list of db types in install.inc and do all checking in install.foosql.inc.
• All the requirements are put in the $settings arrays. But in function naming, settings only refers to the settings.php variables (db_url, base_url, ...), not to requirements. This is very confusing.
• Typography: the double-space after a period, capitalization of comments and formatting of placeholders is very inconsistent. AFAIK the preferred way is to use <code> for filenames and <em> for others.
• Geekyness: 'RAM', referring to file permissions with no extra info,
• Curly braces for string offset indices have been deprecated.
• It is faster to use PHP functions than regexps, if possible. For example, for trimming slashes at the beginning or end, use ltrim / rtrim.
• "$/$file$" is a particularly nasty regexp. First, using $ as delimiters is horrible because there is possible confusion both with the PHP $variable prefix and with the regexp 'end-of-string' operator. Secondly, the $file variable is not escaped, so if it contains "install.php", it would also match "installXphp". Thirdly, there is no reason here not to use str_replace.
• Defining both FILE_WRITEABLE and FILE_WRITABLE is silly. Silent e's at the end are dropped when appending suffixes.
• Documentation! theme_install_page() seems to differ only from theme_maintenance_page() in the way status messages are displayed. This was only clear after doing a side-by-side comparison.
• system_requirements_post() does not return anything.

Finally after grokking the code, I'm still not convinced about the whole $settings array. It seems that mostly, it is shoehorning very simple checks into a very convoluted structure (with install.mysql.inc being the archetypical example). I get the impression it would be much simpler to let the install.foo.inc files provide singular API functions that can be used to verify a single condition.

Essentially, the Forms-API-like array structure provides a couple of programmatic advantages (e.g. being able to get a list of requirements, being able to merge identical requirements, being able to modify requirements), but none of these advantages are being exploited at all. For example, there is no human-readable description of a particular requirement (only of the error message if it's not present) and all requirements are simply checked module by module, with no resolving or post processing.

I don't see any of those things being needed, which means that using the $settings array just means it's a lot of unnecessary code.

IUnable to sleep, I checked my email -- thanks for taking the time to do a lengthy review, Steven.

I agree with Neil that it should be split up.

Sure, I'll split it into bite sized pieces once the full functionality is there. It's got a ways to go, though.

it adds a bunch of requirements testing without doing anything with the checks we already have in place.

Okay, when I've got something I think is merge-ready, I'll look at making the API available to all of core, not just the installer, and then to replacing existing core checks with calls into the API. That seems a logical first step to getting this functionality merged.

use a quoted string when using function_exists()

Fixed.

It seems it would be cleaner to just hardcode the list of db types in install.inc and do all checking in install.foosql.inc.

Fixed.

All the requirements are put in the $settings arrays.

Fixed. Requirements are now put in an array called $requirements to avoid confusion with settings.

double-space after a period, capitalization of comments

Fixed. I've removed double-spaces and capitalized the first word of comments.

formatting of placeholders is very inconsistent. AFAIK the preferred way is to use <code> for filenames and <em> for others.

I'm not sure what you're referring to here.

Geekyness: 'RAM', referring to file permissions with no extra info

Yes, the error messages need to be rewritten for clarity and usefulness. This is on my TODO list.

Curly braces for string offset indices have been deprecated.

What are they replaced with? I thought it was just the use of array-brackets that was deprecated?

It is faster to use PHP functions than regexps, if possible. For example, for trimming slashes at the beginning or end, use ltrim / rtrim.

Fixed.

there is no reason here not to use str_replace.

Okay, the regex was replaced with str_replace().

Defining both FILE_WRITEABLE and FILE_WRITABLE is silly. Silent e's at the end are dropped when appending suffixes.

PHP does the same thing, that's where I got the idea... It's such a common typo.

Documentation! theme_install_page() seems to differ only from theme_maintenance_page() in the way status messages are displayed. This was only clear after doing a side-by-side comparison.

Yes, at this time that's the only difference. Improving the installer theme is on my TODO list. If it turns out to be possible to get rid of the new install_page in favor of using the existing maintenance_page I will do so. I try to improve comments as I go, some are still missing or are vague and will be improved.

system_requirements_post() does not return anything.

Thanks, fixed.

It seems that mostly, it is shoehorning very simple checks into a very convoluted structure (with install.mysql.inc being the archetypical example)

install.mysql.inc really doesn't do anything at the moment. Fleshing it out is the next step, as bootstrap phase three is where we make our connection to the database.

I don't see any of those things being needed, which means that using the $settings array just means it's a lot of unnecessary code.

When I get to the profiles phase, that's when I'm planning to fully take advantage of the $settings (now $requirements) array. At the moment this is just a bootstrap which by its nature has to be somewhat simplistic.

The latest patch is attached. Or if you prefer you can download Drupal CVS HEAD with the patch pre-applied here.

+1 for smaller patches! :)

Step #1 could be the requirement checking. Useful (i) on installation, (ii) on upgrade and (iii) on 'post installation' (after recompiling PHP for example).

Ok. I've tested this patch on two environments I have access to with the Version 0.4 tar.gz from http://kerneltrap.org/jeremy/drupal/installer/. I ran out of time to test this on IIS, sorry. :( I also have access to shared hosting elsewhere and a Mac box and will try to get time in the next couple days to set those environments up.

Environment #1: Local XAMPP Lite install
Windows XP Pro SP 2, PHP 5.1.1, Apache 2.2.0, MySQL 5.0.18

Initial screen came up fine. Received an error in big red text below the settings fieldset:

"Your settings.php file is configured to use the default username and password. This is not allowed for security reasons."

This was kind of confusing, because of course the settings.php had the default username and password... I hadn't run the installer yet! ;)

Down below that, it tells me:

* Automatically fixed the permissions of file './cron.php'.
* Automatically fixed the permissions of file './index.php'.
* Automatically fixed the permissions of file './xmlrpc.php'.
* Directory "./files" does not exist.
* The "files" subdirectory can be created in your installation directory to store files such as custom logos, user avatars, and other media associated with your new site. The sub-directory requires "read and write" permission by the Drupal server process if you wish to provide this functionality.
* For improved Drupal performance and reduced bandwidth consumption you can enable the PHP zlib extension. If enabled, Drupal will compress cached pages and serve these pre-compressed pages to web browsers that support gzip.
* Your Base URL was automatically changed from the example 'http://www.example.com' to the detected URL 'http://localhost/drupal-installer'. (nice touch pre-filling the base URL box, btw)

I click "Save configuration" and am told Drupal has installed successfully. Still prompted about the files folder and the zlib thing. The settings.php file is written correctly. Yay!

Environment #2: Shared hosting (DreamHost)
Debian Linux (sarge), PHP 4.4.2, Apache 1.3.33, MySQL 5.0.16 phpinfo()

Result:
Fatal error: Call to undefined function: apache_get_modules() in /includes/install.apache.inc on line 65

From http://ca.php.net/apache_get_modules looks like this might be because DH is still on Apache 1.3.

So as a result...

Patch Review

1. Need to check for function_exists('apache_get_modules') .. possibly some other ones.
2. A check should be put in somehow to see if the installer has run first before displaying an error about default settings.php
3. Error messages should be displayed 'above the fold' -- I missed all the messages the first time around because my browser window was reduced size. Status messages are probably okay below the form.
4. If the files directory doesn't exist, it should probably attempt to create it, with the proper permissions. Or does this run into a whole issue where when PHP is running under 'php' then user 'me' can't access the stuff in there?

So I downloaded and tried it on an IIS6 system and got 'Unsupported install environment' with a link to the Drupal requirements page which lists IIS. Also some errors were listed.

"The following errors must be resolved before you can continue the installation process:"

* The cron.php file provides automation for Drupal functionality. Most installations will want this file properly installed. The file should be readable not writeable (The file must not be writeable as that would be a security risk).
* File "./index.php" is writable but should not be.
* The index.php file is Drupal's main interface with the world. This file must be readable and not writeable. (The file must not be writeable as that would be a security risk).
* File "./xmlrpc.php" is writable but should not be.

IIS doesn't use cron, it uses the system scheduler. The files it indicates as world writable are not to the best of my knowledge. I would be willing to run any test and provide any additional information requested.

Here are a few places where checks are repeated in core:

PHP safe mode checks:
- cron.php, lines 12-15
- includes/locale.inc, lines 147-150

File writability:
- file.inc, lines 97-107

File existance:
- bootstrap.inc, lines 124-127, functions drupal_get_filename, line 150
- file.inc, lines 173-175;250-253;263-274;279;functions file_create_filename (342), file_save_upload (383), file_save_data (432), file_download (485)
- module.inc, line 59
And that's just a start on those style calls.

GZIP/gzencode checks:
- bootstrap.inc on lines 483-488
- common.inc, lines 1322-1333

DB version check:
- updates.inc on line 1492
- database.mysql.inc, 84

PHP version check:
- common.inc, 1125

Robin

My suggested CSS changes...

Not sure where these would go, but I stuck them in misc/maintenance.css while working on it. I didn't have that much time to work on this, so, I know it's not great, but there are some space improvements that I liked.

::: BEGIN CSS :::

body { font-family: Lucida Grande, Myriad, Andale Sans, Luxi Sans, Bitstream Vera Sans, Tahoma, Toga Sans, Helvetica, Arial, sans-serif; }

input, select { margin: 5px 0; }
input { padding: 3px; }
input:hover { border: 2px solid #93C5E4; }
input:focus { background: #E6F6FF; color: #0073BA; }

input.form-submit { padding: 0; margin: 0; }
html>body input.form-submit { padding: 3px; margin: 0; }

label { font-weight: normal; font-size: 0.8em; }
legend { font-size: 1.4em; }

p { color: #0073BA; }

div.messages {
border: 1px solid red;
padding: 10px;
}

::: END CSS :::

Feel free to let me know what you think.

Trae

Trae brings up a good point.

If we are to theme this installer, we need to think about the other DB maintenance and 'Drupal down for maintenace' type pages. Those all need to be themed and consisently. I don't think the Drupal installer needs to be any different from those, but rather, all of those the same.

As such, here is hunk (or split) #2: theme the installer, DB maintenance, and Drupal-site down packages ... that can go in as a seperate patch.

Here's an updated patch to address the issues raised so far, as well as general code cleanup. It also implements bootstrapping of the database, so now you don't see "install complete" until we've succesfully connected to and selected the database. A pre-patched tarball to ease testing can be found here.

The actual help messages generated by the installer are being looked at, and will be made less confusing soon.

Awesome! Seemed to work perfectly on Windows XP with Apache 2.0.55.

A few issues though:

- Is there any way to detect a successful installation of Drupal? For example. if I goto http://www.example.com to install Drupal and I haven't already, is it possible to make the installer smart enough to detect Drupal has not been installed and to *automatically* call install.php ? This would certainly make the process more usable.

- I got a warning running install.php right away:

The following errors must be resolved before you can continue the installation process:
Your Drupal site has not been configured.

Obviously I haven't installed it, I'm running it for the first time. Seems a strange error to get as I attempt to install Drupal.

- Anyway for the installer to create the Drupal database, user and pass for MySQL?

Otherwise, things are shaping up quite well!

is it possible to make the installer smart enough to detect Drupal has not been installed and to *automatically* call install.php ?

Absolutely, but that will come after the installer itself is fully functional.

I got a warning running install.php right away:

Thanks for testing! Fixed, the installer doesn't show a "this site has not been configured" error anymore.

Anyway for the installer to create the Drupal database, user and pass for MySQL?

That's beyond the intended scope of the installer for now...

I've implemented initial support for profiles in the attached patch. Most of the file "database.mysql" has been moved into "system.include", and a new "./profiles/default.profile" file has been created. At this point, all the Drupal tables should be properly created for you, and the specified modules enabled, but the default values aren't set yet (I'm out of time for tonight.) More details and a pre-patched tarball can be found here.

(Known outstanding functionality includes: the ability to select from multiple profiles, the ability to set default database values, the _requirements_post hook, and preventing the installer from being run multiple times. Also, at this time the installer only works for MySQL, an install.pgsql.inc file still needs to be created.)

Screen shot from my test.
IIS6, MySQL4 php4.x

PHP is reporting that index.php, cron.php, and xmlrpc.php are all writable. You need to use windows explorer to make the files read only. Try right-clicking each the files and going to properties (or something like that, I've not used Windows in a long time).

Okay, the installer is now capable of doing a basic Drupal installation. The help/error messages still need to be greatly improved, but the basic functionality is there. Table prefixing and PostgreSQL are not yet supported. The general idea is described here. A pre-patched tarball can be downloaded here.

Jeremy, maybe it's an idea to let the Documentation team have a look at your docs? You can reach them by mailing the drupal-docs@drupa.org mailinglist...

Support for table prefixing and PostgreSQL was added in this patch. Pre-patched tarball available here.

I have opened a new issue in which I've created a patch with just the requirements api utilized by this installer. In the new patch, the requirements api is used by the core system module to verify the requirements of already enabled modules, and to verify the requirements of modules before they are enabled.

How can you use the installer to install Drupal in a subdirectory?

I tried installing a copy of Drupal on http://localhost/~dries/drupal-cvs/. After the installation, it said "Congratulations, Drupal has been successfully installed. Please review any warnings or errors that you might see above before continuing on to your new website.". The link to the new website pointed to http://localhost/.

When I went to the proper URL, I started getting "warning: Cannot modify header information - headers already sent by (output started at settings.php:109)" messages.

Looks like it wrote $installed = 'default'; after my closing php tag. Odd.

When you already have a site setup it claims it installed Drupal, but in reality it didn't do anything. Or so it seems.

Resynched to head, using the new conf_path() function instead of conf_init(). Pre-patched tarball available here.

Big update to add better profile support and to resync with CVS HEAD. Prepatched tarballs can be found here.

trying to fix HTML.

still trying.

I'm quite unhappy to see that we still have that giant deferred $requirements array in there. I keep reading through the code, and it is not obvious at all. It reminds me of the update system, as in everything the update system does right, this seems to do wrong.

• With the exception of drupal_verify_version_php() (which is a special case), each verify function consists of a foreach statement of the $requirements array. No specific code seems to ever be placed outside this foreach. There is a clear redundancy here.
• drupal_verify_install_php() (and its re-implementation of PHP operators) is a telltale sign of bad code.
• You touted the merging of requirements as one of the advantages here. But, e.g. the PHP requirements can take complicated forms (smaller than, greater than, not equal, ...). Yet, when requirements are merged, you always take the largest required_value, regardless of which operator is used. This would seem to indicated that the supposed flexibility is simply not there.
• Many rules carry small exceptions. For example, the different database types or settings.php checking. In this way, attempting to deal with all requirements in a unified fashion only obscures these exceptions.
• When requirements are checked, errors are passed back through the global message system. The requirements array is not used in any other fashion, which means it is not as critical as it would seem.

Keeping in mind that checking requirements is an easy and fast process, which does not need to happen often... what downsides other than redundancy would there be if we did not merge requirements and checked each requirement directly? You might say that it is useless to present the same requirement violation twice. I would like to turn that around: as an administrator, wouldn't it be nicer to know exactly which modules I cannot run on my install? And how often do requirements duplicate anyway? Drupal itself is the most demanding factor in 90% of the cases, I'd imagine. Being able to see whether my install fails because one module or because of the base layer + 10 modules is significant. If we couple requirements with module dependencies, you can avoid many redundant checks.

If you look at all the requirements checking, there are three big pieces of code which are IMO unnecessary:

1. Retrieving and merging of the huge $requirements array (_merge_requirements() and its invocations).
2. Examining up the array by type and redirecting to the right file (drupal_verify_install()).
3. In each requirement-type-implementing file, iterating over the requirements and verifying them with the appropriate function (drupal_verify_install_apache(), drupal_verify_install_mysql() and even indirections beyond this level!).

If you remove this, all that remains is a bunch of independent functions, which each can be used to verify single conditions. It is still a requirements API. If a module needs to verify requirements at any phase in the installation, it would implement a hook (e.g. install_pre) where, instead of building a giant $requirements array and returning it, it simply evaluates each requirement directly. Just as now, drupal_set_message(... 'error') would be used to signal errors. The difference is minor:

/* Before: */
  $requirements['php']['safe_mode'] = array(
    'required_value' => 0,
    'message'        => 'PHP safe_mode is currently enabled. You must disable safe_mode for Drupal to properly function.',
    'message_type'   => 'error'
  );

/* After: */
  if (ini_get('safe_mode') != 0) {
    drupal_set_message('PHP safe_mode is currently enabled. You must disable safe_mode for Drupal to properly function.', 'error');
  );

Where appropriate, we can still provide an API function to check a single requirement:

/* Before: */
  $requirements['file']['./cron.php'] = array(
     'type'           => 'file',
      'mask'           => FILE_EXIST|FILE_READABLE|FILE_NOT_WRITEABLE,
      'message'        => 'The cron.php file provides automation for Drupal functionality. Most installations will want this file properly installed. The file should be readable not writeable (The file must not be writeable as that would be a security risk).',
      'message_type'   => 'error'
  );

/* After: */
  if (!drupal_verify_file('./cron.php', FILE_EXIST|FILE_READABLE|FILE_NOT_WRITEABLE)) {
    drupal_set_message('The cron.php file provides automation for Drupal functionality. Most installations will want this file properly installed. The file should be readable not writeable (The file must not be writeable as that would be a security risk).', 'error');
  }

Forms API-like abstractions are tempting because they look clean at first. But that needs to be contrasted to the entire support infrastructure which is needed to maintain that abstraction. As I've pointed out above, I think the abstraction fails in several ways (because it is not consistent enough) and does not offer enough benefits to justify it.

Other than that, I spotted some bad code. I remember fixing many of these things in statistics.module a while ago, which I think you wrote as well. Please pay attention to code quality. It saves considerable time later.

• There appears to be some dead code. _using_mysql(), aside from being a rediculously long function for such a simple check, is never called.
• Curly braces for string offsets are deprecated in favor of square brackets (the opposite of before).
• Placeholder usage is still all over the place. This refers to the placement of dynamic, possibly user-submitted data, into text output. Vital string checks are missing and their formatting is inconsistent (as I described a long time ago). Please see the handbook section about writing secure code. Note the use of theme_placeholder() in core as well.
• Still some unquoted function names used in the code.
• Usage of "(click) here" links.
• drupal_detect_database_types() is plain bad code. You can transform it into a simple loop.
• Various minor code style violations (spacing, capitalization, ...).
• No use of double spaces after a period. They do not show up in HTML and bloat the code.

I have to agree with Steven. The requirement checking is somewhat clumsy. Weren't we go to submit the requirement checking as a separate patch?

When I was testing out the installer, by accident, I pointed the installer to an existing drupal database and not the empty db that I intended it to be pointed to. Of course the install failed with "unable to create table" statements. But what I did not like was that the installer gave me no chance to correct my mistake. To correct that mistake, I had to unroll the tarball agian, because I could not figure out what I needed to change to restart the installer. To me this was no big issue, but I feel that this issue should be addressed.

The second thing that I ran into was when I attempted to install the civicspace profile. I got to the error screen telling me that I did not have the required modules (civicrm) that I needed. That was fine, but the installer did not give me the option of continuing the install or tell me how I should continue the module were installed.

Also a nice addition to the installer would be to allow the option for you to enter a Database Admin account info, and then the installer would also create the database and the drupal db user. I have seen this done on other projects (gallery2, and some other CMS).

Are there any plans to add a downloader to the installer? The install downloader could download submitted install profiles, themes and modules from drupal.org. Extract them into correct directories. Just an Idea.

The installer seems like a good start, I just needs a like more polish.

If things go as I plan then this evening I will be able to apply Steven's review to this patch.

I miss four important options and features. Maybe I did not fully understand what is going on, that could be the main problem :)

1. A way, (permission?) to NOT have this install stuff available at all. Update.php also 'lacks' this feature, it could be a good chance to fix this for both. Very usefull for hosts and other limited installations of Drupal.
2. A way to tie into the install system. including files and /or calling Drupal in various bootstrapped versions, when not yet installed, failed for me. We really need this, for people who want to automate instalations, or who want to tie 3rd party installers to Drupal.
3. What happens to hosts that do not allow writing of PHP[1] in the sam place where that PHP is ran? We die with an error, but ofer no proper alternative, it seems.[2].
4. We cannot create a database table, so we miss a large part of the installation process. Creating a database is harder (IMO) then editing a settings.php file. Is there a way to catch this?

[1] Allowing the same application, in the same runtime environment to "write its own code" is considered a security hole on the "better" hosts. I was told that Joomla breaks on about 1/4th of the hosts because of this. We do the same: we allow the application (Drupal) to srite into its own code, from the web. Debian and SuSE (and I was told RedHat too) being the biggest chunk of servers out there, do *not* allow PHP writing executable files in their default configuration.
[2] An alternative could be to fall back to the default installation, with a proper message that the user should fall back to that default installation, usnig FTP/CLI etc.

typo under four: "We cannot create a database table" should be: "We cannot create a *database* with a database user"

Another thing I cannot find. Again, maybe its just me.
A way to provide a different directory for files then /files/.
My sites all have their files under sites/example.com/files but I did not manage to get those working with the installers. Other sites prefer to not have thier files in a public space (private files).

Let's continue at http://drupal.org/node/68926 .