It is time for the fixed_list parameter to module_list() to be retired. It makes the function more or less unreadable, and if I remember correctly was the biggest barrier to refactoring it in Drupal 7 to be more sane.

Pretty sure we can kill it using the trick in http://drupal.org/sandbox/catch/1152890

It is still ugly, but it is ugly that is confined to the particular special case of the installer, instead of ugly that is buried deep in the heart of the critical path, so a good trade IMO.

To remove the ugly from the installer, we would need to allow includes/plugins to define their own schemas instead of having system.module do it. That is part of the long game of #679112: Time for system.module and most of includes to commit seppuku but is dozens or hundreds of patches away. In the short term it's better to have bad hair than a bad stomach.

Or in visual terms, when we eventually remove that ugly, it will be:

Instead of

No patch yet.

CommentFileSizeAuthor
#8 install.patch48.42 KBcatch
#6 make_system_module_a_module.patch35.81 KBcatch
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

catch’s picture

Another patch that depends on #1061924: system_list() memory usage.

sun’s picture

Not able to access the referenced sandbox project currently, but in general, +1

sun’s picture

catch’s picture

Another possible approach.

In install.inc define drupal_common_schema() - this includes system module, possibly session table, whatever is absolutely necessary to install a module via batch API.

system_schema_alter() adds that back into the proper schema cache.

The schema API can make db tables just given a schema array, it doesn't have a hard relationship to hook_schema()

This would get us towards removing the hard dependency on system module for everything else in Drupal.

Moving the system table to the new config layer might help with this, but there is information in {system} that isn't configuration, and there's other stuff we need for the installer, so it's likely not enough.

catch’s picture

Restoring tags.

catch’s picture

Status: Active » Needs review
FileSize
35.81 KB

Here's a start on that.

I moved {variable}, {system}, {registry} and {registry_files} to includes/install.inc

All 'required' modules get enabled in one request.

Yes this adds some risk to timeouts on the install. I don't care because there should be less (zero) required modules. Also patches like #1167144: Make cache backends responsible for their own storage and others being worked on with reduce the size of the schema that system module has to define.

Patch does not yet remove the fixed_list parameter, installer passes locally but I want to see how we're doing on tests before getting much further.

Status: Needs review » Needs work

The last submitted patch, make_system_module_a_module.patch, failed testing.

catch’s picture

Title: Kill fixed list parameter for module_list() » Decouple system module from the installer
Status: Needs work » Needs review
FileSize
48.42 KB

Alright it is going to be hard to remove the $fixed_list parameter without a lot more refactoring, one of the reasons that exists is to avoid the drupal_alter() in system_rebuild_theme_data() from firing when setting the maintenance theme - i.e., because the maintenance theme is dependent on the modules system of course, even though it's used for the install.

So this is just a start but I'll leave it here to get some feedback. Moving all this code around is brute force, but this feels like a decent chance to start breaking the circular dependencies.

Status: Needs review » Needs work

The last submitted patch, install.patch, failed testing.

boombatower’s picture

subscribe, discussing with catch in IRC.

boombatower’s picture

I have a very rough initial installer written that actually seems to work.

I am working with the following approach.

- hit install.php
- generates extremely minimal database in sqlite in temporary directory so as not to require any setup
- you are then redirected to drupal site running off the sqlite database
- the installation wizard runs as a module inside a standard drupal environment
- user fill in database information, select profile, etc.
- the installer module then installs all the modules for the selected profile (which will then include system) on the actual database
- the temporary sqlite database is destroyed and the user is redirected to the installed drupal site

I currently have the initial sqlite database being generated in a temporary directory such that you can actually view the drupal site running off it with only system and user partial enabled (don't need all the tables, we could install them all just for simplicity sake and leave the schema definitions there instead of dealing with common schema and what not).

The current installer code that gets the basic drupal site running is very small (which is the idea). It will be the only piece of the installer that does not run in a drupal environment. This allows the rest of the installer to run with proper caches, form system, hooks, etc so that it no longer behaves weirdly and has an entirely separate code base. The current code I wrote is copied and pasted (with lots of commented out code and some hard coded data) and remains at just over 200 lines (it's VERY minimal).

The next step then is to write the installer.module which I would say we do from scratch (most likely lots of copy and paste, but massive restructuring) as we have a complete new methodology, environment, and options. We can mimic the old one or make something totally new I mean you could download and browse modules from d.o during initial installer...all kinds of crap. For now I vote we just mimic the old one. The only option I would open to discussion is the use of the maintenance theme or not, since we can easily use a real theme since we have the full environment...then it will look and feel as a regular drupal site.

An option to consider is that once we get the real database information we copy the sqlite database into it and reload the site...so we are simply building from there. I somewhat like the idea of simply switching database connections and installing all the modules in the real database, while the installer runs (boots) off the sqlite database. This ensure that system module can be installed in a drupal environment as well.

Using this new installation method one could write a script to invoke initial install, bootstrap, and simply run module_enable(array()) instead of having a huge scripted set of crap.

// custom install script (rough idea)
install_initial(); // w/e
install_database_info($database);
module_enable(array('my_module'));
sun’s picture

@boombatower's proposal in #11 sounds interesting, but I think it deserves a separate discussion and follow-up issue. There are many more detailed questions involved with it (such as general availability of sqlite on all platforms), which do not have to be answered for the rather simple split and separation being proposed in this issue.

@catch: I'd also suggest to move the removal of $fixed_list into a separate issue. That only looks "related" and like a possible benefit from doing this to me, but it has the potential to block this entire patch.

boombatower’s picture

David_Rothstein’s picture

Subscribing. Note that for this part of the patch:

-  $module_list['system']['filename'] = 'modules/system/system.module';
-  $module_list['user']['filename'] = 'modules/user/user.module';
-  module_list(TRUE, FALSE, FALSE, $module_list);
-  drupal_load('module', 'system');
-  drupal_load('module', 'user');

I had some code at #782672: Loosen the coupling between the user module and the installer/maintenance theme a while ago that tried to get rid of the 'user' part of that only. It may make sense to tackle that independently, since there were a couple of tricks to that (though overall much simpler to deal with than removing the dependency on 'system').

catch’s picture

I agree, let's do user module first since that's a more manageable change.

sun’s picture

Assigned: catch » Unassigned

Most of this is obsolete.

The primary remaining dependency on System module are the stream wrappers that it registers; i.e., public://

andypost’s picture

now seems this one could be closed as obsolete

sun’s picture

Issue summary: View changes
Status: Needs work » Closed (duplicate)

The primary remaining dependency on System module are the stream wrappers that it registers; i.e., public://

The topic of stream wrappers is attacked in #2028109: Convert hook_stream_wrappers() to tagged services.

Some further aspects remain (such as system_requirements() and system_schema()), but those are equally complex topics like stream wrappers and need to be addressed individually, and this meta-ish issue doesn't really contribute towards solving them.

Therefore, closing this issue - as "duplicate" (of many others).