Refactor _drush_sql_get_credentials() to allow empty passwords

site-install assumed that database name is preceded by a space character, and blatantly only supported MySQL by using the string constant 'mysql' for the command, as well as 'information_schema' for the database and -e for single-query-execution.

This patch removes any assumption about the syntax used in the credentials string, and also removes some of the explicit MySQL features in drush_core_pre_site_install(). There are probably other such assumptions in site-install related to the scheme and MySQL — I have not tested with other schemes, and was depending on the Postgres documentation linked in the description for some of these changes.

This patch also includes changes from the preceding patch, and in addition allows _drush_sql_get_credentials() to return credentials for a connection to the database server, but not the database. It uses MySQL's built in information_schema database for this. Postgres already supported this with the template1 database.

Understood. Is there a test suite for drush?

I'll use drush with this patch applied and aim to keep this issue node up to date. It would be great if others could also test in this way.

Perhaps we could apply to HEAD to encourage further testing?

Hmmm. This is rather challenging, since any command that bootstraps to DRUSH_BOOTSTRAP_DRUPAL_DATABASE or higher, as well as several other commands (like site-install) uses _drush_sql_get_credentials(). Testing this manually would be somewhat tedious and time-consuming. Perhaps I am missing something?

Oh I've done that already. I've used most of the drush pm-* commands and various others already. It just needs pgsql testing before it can be RTBC then?

Moshe; Good catch.

Yes, -U is gone and that is intentional. -U is replaced by --user="bevan", much like like -ubevan" becomes --username="bevan" in MySQL.

How is PgSQL testing going? I asked Josh Waihi (fiasco) to give this a spin too. How thoroughly do we need to test this before it is RTBC for CVS HEAD?

I'm now using HEAD with Moshe's patch applied with MySQL. drush site-install, at least, is good.

Re-rolled for head. This patch includes a change that was committed to head to add backticks around the db name and escape them with backslashes. Actually the original patch here was already adding backticks, but not backslashes.

Before re-rolling I found a bug where the port number is wrapped in quotes. It shouldn't be.

The backslashes introduced in #15 are not needed. I don't know why they are in HEAD (Bug?).

This patch removes them, reverting back to patch from comment #12, so site-install works again

In addition it fixes the issue raised in #15. This was not related to the port number specifically being wrapped in quotes, but arises with a bash command like $ `drush sql-connect` < file.sql, because the backtick operator on the command line doesn't play nicely with quotes in the output of the command which the backticks wrap around.

This patch fixes that by not wrapping $value from --$key='$value' in single quotes. The original issue (#925764: SQL connection string for empty password is broken) is still fixed because --password= is equivalent of --password=''. (However Shorthand -p has no equivalent of --password='').

We are starting to find only very edge-case arcane bugs here now. Both Moshe and I have been using drush with this patch for a while. It's now ready to be committed to HEAD in my humble opinion. This is important to introduce even more testing before it gets put into a stable release or branch.

Yay! Thanks! (: