Allow db drivers outside of 'core'

One way of doing that could be to have a path in the $databases array, like this:

'driver path' could probably be detected during installation.

Well, its not really a patch, but it is requiring modules to put their code inside of /includes which is just as bad as a core patch.

Install profiles don't require you to copy code into a core directory since they are already packaged that way. So its a bit different IMO. I'm not going argue about priority. Normal is fine. I do think we should endeavour to fix this. I'll see if I can find that original discussion.

This is a work in progress but something is happening. I came up with a directory name 'db_drivers' to be put alongside modules and themes in the Drupal root, this is not something I plan to pre-create unlike modules and themes. The screenshot is from an earlier version where i called it 'database', nonetheless, it shows the install screen working.

Let's do this instead. This uses function isCustom { return TRUE;} and writes accordingly into settings.php. It's a lot cleaner.

Here's one that peeks at the Tasks classname during install to figure out whether it's a core driver and gets rid of the Tasks method. Less code.

I think it's okay to not document here how to create a custom db driver, as that's an advance topic, which could be maybe written somewhere else.

All these code lines looks fine, but yeah in a perfect world you would have a fully decoupled system, where you don't need to distinct between core and custom drivers, but this patch is already a huge improvement.

This looks mostly reasonable, aside from this line as we cannot hard code Drupal functions into the Database core classes.

Instead, I wonder if we couldn't specify a class prefix (namespaces) in the database definition in settings.php, and then just rely on the autoloader on its own? If you're setting up a 3rd party driver, it should be expected that you need to take care of its autoloading. Consider, eg, if 3rd party drivers were brought in via composer. Then all you need to define is the name prefix the classes, use, which don't even have to be in the Drupal namespace technically.

Injecting the classloader is ugly, too. The database manager shouldn't have to think about autoloading. That's not its job.

What if we put the /db_drivers/ check in where we setup the classloader? If it exists, register it as a possible source for \Drupal\Core\Database\Driver classes. That's one file_exists() check, but in the grand scheme of things there are far bigger performance fish to fry. Then the DB manager can just use the name of the class as normal (keeping the magic naming just as we do now), and the autoloader can do what the autoloader is good at: Load classes.

Also, Re #8, I'd be fine with the base classes being abstract if the language would let us. But I don't think you can make a class abstract unless it has at least one unimplemented method, which we have no need for as most driver-provided classes don't need to override anything. So, meh. If $driver_class doesn't exist, that's a broken state anyway. We either let it fail hard, or we catch it and throw an exception. I'm fine either way.

Well, enough of this. I know that dawehner only didn't RTBC this because we wanted Larry to see this and he said "This looks mostly reasonable, aside from this line".

Dries, I ask you to decide whether Larry is is right and preferably commit #12 (or #18 if you really must). While #12 indeed adds a call outside of the DBTNG directory this should be no problem -- just do not add a custom => TRUE to your config, done. If the time comes to separate DBTNG out, adding a function_exists there won't be a problem. There is, or should be, a reason of why we have a Drupal\Core and a Drupal\Component directory and as of right now Database is still in Core.

As I stated in the past: I find the double measure of Drupal 8 absolutely unfair where people can freely add any hundreds or thousands of code that does not fit Drupal in any way or form (and, ironically then I work hard to make it fit somehow). and I can't fix anything if it does not fit their agenda. Well, I try really hard not to complain any more for the sake of progress. Can we cut a one line slack for me -- once again, in the name of progress?

This is the result of an extremely fruitful conversation with catch. Small, pretty and extremely powerful. You can now put memcache and whatnot in 'drivers/lib'.

I'm ok in principle but it seems like the code could be cleaned up a bit.

Specifically, I'm ok with adding a 'drivers' directory -- not sure I'm fond of 'db_drivers'. We've kept the Drupal root free of underscores, which is kinda nice.

Should we call this function 'drupal_get_database_drivers()' instead? Seems weird to request 'types' and to call the result 'drivers'.

Wouldn't this make more sense to register the classloader where we already register other classloaders?

I'm not sure I like this. Could you add some code comments?

Well, if we have to do this anyway:

... why don't we allow modules to ship database drivers?

It's good to be consistent here with both the namespace and the folder!

Maybe it would be just be helpful to put http://www.php.net/manual/en/language.variables.basics.php as reference.

Thanks for adding the comment.

I think Dries reviewed some old patch cos #22 already addressed everything in #23 :D

ack. too much xposting.

OK we discussed at length and agreed on actually not changing anything. This just adds a comment to link to the PHP docs. Yes there are warts: we do "drivers/lib/Drupal/Driver/Database/*" while elsewhere we would have "drivers/*/lib/Drupal/Driver/Database" but resolving that would be a rather complicated process -- we would need to register the drivers/* roots for this which would require another settings.php global or even worse drupal_classloader_register calls in settings.php. Neither is a good idea for such an edge case.

#33 looks good by me.

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