Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.Encountered while installing Drupal 8 on Acquia Dev Desktop (ADD), but this is a Drupal bug.
Steps to reproduce
- Install Drupal using ADD or while running mysql on a non-standard MySQL port (i.e. not 3306) or postgres on a non-standard port (not 5432)
- Empty the database and remove the config directories and then re-install be going to core/install.php
- At the database page, note that your database name has been pre-filled, but the database port is 3306 (for mysql)
- Open the existing setting.php file and note that the port number is set to your correct data base port.
Conclusion:
- Drupal is not correctly reading the port from settings. Same is true of table prefix
| Comment | File | Size | Author |
|---|---|---|---|
| #22 | interdiff.txt | 926 bytes | sun |
| #22 | install.port_.22.patch | 5.83 KB | sun |
| #21 | interdiff.txt | 5.04 KB | sun |
| #21 | install.port_.21.patch | 5.75 KB | sun |
| #18 | default-db-2212947-18.patch | 8.16 KB | pwolanin |
Comments
Comment #1
YesCT CreditAttribution: YesCT commentedComment #2
nyirocsaba CreditAttribution: nyirocsaba commentedYou are on MAC, right? PDO will use unix socket when connecting to localhost. Try to replace the database host from "localhost" to "127.0.0.1".
Comment #3
ianthomas_ukNo, that was on Windows. I'm not sure why that's relevant anyway - whatever ADD added to the settings file (which gets included in settings.php) should be the default values when you get to the database stage of the install process. If it had put the wrong values into settings.php then that would clearly be an ADD bug.
Comment #4
YesCT CreditAttribution: YesCT commentedI suspect we might be able to provide steps to reproduce that dont involve ADD.
Maybe just changing the port in settings and then seeing if the installer picks up that different port.
tagging novice for trying that.
https://drupal.org/contributor-tasks/add-steps-to-reproduce
Comment #5
dmitry_bezer CreditAttribution: dmitry_bezer commentedAcquia Dev Desktop is not needed to reproduce this - just edit settings.php before you start installing Drupal 8 and you will see it. Suggested patch is attached.
Comment #6
pwolanin CreditAttribution: pwolanin commentedI just wrote this exact same patch. setting to CNR, since the patch it not getting tested.
Comment #7
pwolanin CreditAttribution: pwolanin commentedSame problem for the postgres diver. This should address both.
Comment #8
pwolanin CreditAttribution: pwolanin commentedComment #9
pwolanin CreditAttribution: pwolanin commentedI tested my patch with mysql and it found the pre-existing port setting correctly.
Comment #10
Anonymous (not verified) CreditAttribution: Anonymous commentedseems like a simple fix to me.
Comment #11
webchickFix looks great, but it'd be wonderful if we could add a test for this. I see some stuff in core/modules/system/lib/Drupal/system/Tests/Installer but not sure. We don't want to go all the way through with the install, since it'll obviously break, but being able to make the assertion that these form values should be pre-populated by existing settings.php settings seems like a really good idea, so we don't inadvertently break this in the future.
Comment #12
pwolanin CreditAttribution: pwolanin commentedI just spent an hour trying to write such a test. I think it's currently impossible.
Drupal throws a AlreadyInstalledException if there are existing but invalid DB credentials, there seems no way to test the non-standard port bug default value we're fixing here.
Comment #13
pwolanin CreditAttribution: pwolanin commentedOMG, beating my head on a test that would just test pre-filled DB credentials (though it won't actually test that the port is picked up on testbot, though it does for me locally with DevDesktop) and I found ANOTHER bug, in that the existing prefix is not used (as well as some small bugs in the base classes).
This non-prefixing was very ugly - actually broke the parent site by running the test without the fix, don't try it at home.
Note that the majority of the new test setUp() is just copy/paste from InstallerTestBase so that we can change the step of the DB settings form and send an empty POST instead of posting the parameters.
Comment #15
pwolanin CreditAttribution: pwolanin commentedthis passes locally both CLI and UI, so this is annoying
Comment #16
pwolanin CreditAttribution: pwolanin commentedMaybe has something to do with file perms? Adding a couple extra chmod calls.
Comment #18
pwolanin CreditAttribution: pwolanin commentedArgh - the form doesn't actually populate the password value, which causes this failure. I didn't see it locally because I had no password for my local dev database. According to sun, this is by design, so working around by posting just the password.
Comment #19
pwolanin CreditAttribution: pwolanin commentedComment #20
Anonymous (not verified) CreditAttribution: Anonymous commentedyay, green with a new test, RTBC.
Comment #21
sunProperly extend InstallerTestBase.
Verbose browser debug output reveals that the default port of the pre-configured database connection from settings.php is taken over for all database drivers. → In the edge-case that a user switches to a different driver (despite pre-configured settings.php), the default value for the different driver has a wrong/faulty default value for the port.
Comment #22
sunUse translatePostValues().
Comment #23
pwolanin CreditAttribution: pwolanin commentedThanks for simplifying the code to use more of the parent class. I think this can be back to RTBC.
Comment #24
webchickAwesome, thanks for that. It's good to get this expected behaviour documented explicitly.
Committed and pushed to 8.x. Thanks!
Comment #26
sunThe follow-up for that @todo is essentially:
#2156401: Write install_profile value to configuration and only to settings.php if it is writeable
Slightly different symptom, but identical root cause.