backdrop / backdrop-issues

Issue tracker for Backdrop core.
144 stars 38 forks source link

Fixed: Install fails when using the dynamic config folder paths. #6499

Closed yorkshire-pudding closed 2 months ago

yorkshire-pudding commented 2 months ago

Description of the bug

Trying to install in the default way with dynamically generated config folder names fails as the config folder name is not written to settings.php

Steps To Reproduce

To reproduce the behavior:

  1. Download the 1.x-1.x branch of backdrop as zip
  2. Unzip to website docroot
  3. Ensure DB is empty
  4. On command line cd docroot
  5. ./core/scripts/install.sh --account-pass=password --db-url=mysql://db_user:db_pass@localhost/db_name (anonymised)
  6. It will say Backdrop installed successfully.
  7. head -n 60 settings.php
  8. Navigate to the home page of the site
  9. In command line do ls files
  10. Drop all tables in the database and delete the docroot folder
  11. Unzip to website docroot
  12. In browser refresh the page - it should go to core/install.php
  13. Enter database details and proceed to next step
  14. In command line do ls files
  15. In command line do head -n 60 settings.php

Actual behavior

Step 7 - Extract of settings.php ```php

Note the $database settings have been written to settings.php but not $config_directories

Step 8 image

Step 9

./  ../  .htaccess  README.md  config_a1614e81308b9e6b86be181ef0914335/  field/  styles/

Note the difference in config folder names between folder and error message: Error: config_b05ed1b853e0ec5082777affc41f2cae Folder: config_a1614e81308b9e6b86be181ef0914335

Step 13 image

Step 14

./  ../  .htaccess  README.md  config_a1614e81308b9e6b86be181ef0914335/

Note the difference in config folder names between folder and error message: Error: config_40a3365ba6fb24dd6f483b4734a29085 Folder: config_a1614e81308b9e6b86be181ef0914335

Step 15 - Extract of settings.php ```php

Note the $database settings have been written to settings.php but not $config_directories

Expected behavior

Backdrop installs normally using both the CLI script and the UI Confirmed in both Lando and natively (on host) that the latest release 1.27.1 works fine for both.

Additional information

Add any other information that could help, such as:

  • Backdrop CMS version: 1.28.x-dev
  • Web server and its version: Apache
  • PHP version: 8.1
  • Database sever (MySQL or MariaDB?) and its version: both
  • Operating System and its version: linux
  • Browser(s) and their versions: N/A
indigoxela commented 2 months ago

Confirmed. When using the dynamic directory names, the install via UI fails.

In my case it was:

./files/config_6c8e87b96ae385d096d72b7b6ac38f43/active/ not found.

vs. created directory (wrong!):

./files/config_a1614e81308b9e6b86be181ef0914335

:point_up_2: One is the md5 of the initial database value mysql://user:pass@localhost/database_name the other one the md5 of the actual db settings. The right one.

Seems like this md5 based directory creation happened too early, when the database credentials weren't even set yet?

quicksketch commented 2 months ago

Thanks @yorkshire-pudding and @indigoxela. Could you try out https://github.com/backdrop/backdrop/pull/4724? I haven't reproduced this problem locally, but I suspect it has to do with initializing config before settings.php is rewritten. The PR moves the initialization of the config after rewriting settings.php.

yorkshire-pudding commented 2 months ago

@quicksketch - I tested with that but it doesn't make any difference. I get exactly the same results.

herbdool commented 2 months ago

@quicksketch @yorkshire-pudding It worked for me.

I first tested a fresh install without the patch and I got:

Failed to write configuration file: ./files/config_ef6d450a67517cbc757971fc31e799c7/active/system.authorize.json

Then I applied the patch and got "Backdrop installed successfully." And I was able to log in. All looked fine.

totten commented 2 months ago

For me, that patch (b50117cb42f0d156ab16161dc0d12a9a3b1e6c8b) helps a little bit - but not decisively.

One thing to look out for in reproducing (when repeatedly reinstalling) -- the content of settings.php evolves. Depending on whether you reset it to baseline, the problem may or may not recur. Consider this procedure:

for trial in 1 2 ; do
  reset_mysql_databases
  git checkout settings.php
  rm -rf files/config_*
  ./core/scripts/install.sh --db-url=...
done

Compare:

  1. If I include the git checkout step in the middle, then it consistently reproduces the problem. The folder is always initialized as files/config_a1614e81308b9e6b86be181ef0914335 (aka md5('mysql://user:pass@localhost/database_name')).
  2. But if I skip the git checkout step in the middle, then the settings.php is a bit different, and it actually works.

Asides:

laryn commented 2 months ago

@totten If you get a chance to test removing the global $config_directories; line from backdrop_install_config_location() that's my current theory but I haven't had a chance to validate it yet. Maybe declaring $config_directories as an empty array again before the settings get read in as it was before:

https://github.com/backdrop/backdrop/commit/5faa5ed442d4bb2ad9f6688c20e7c0c9c4d8a07f#diff-5ad946c2d6abff202fe58bbc7b2b8c28af243b335436bdcf6948ca183cb46290

quicksketch commented 2 months ago

Thanks @totten, I appreciate your help! I pushed another commit to the PR that might resolve the problem more comprehensively. In my testing I was able to install manually and have the settings.php file rewritten correctly and be picked up in the remainder of the installer.

One thing to look out for in reproducing (when repeatedly reinstalling) -- the content of settings.php evolves.

Yep, I think that's where we're seeing the problem. I don't usually run into this issue because DDEV always provides the database connection string, so that step of the installer is skipped. When using the UI and using the stock settings.php file, it rewrites its contents based on the input from the UI.

So the first install fails, but it populates some values in settings.php. The next time, those values are provided and don't need to be changed, so the second install works properly.

I think the updated PR is now working properly, please give it a test if you can.

totten commented 2 months ago

Oh, nice. Thanks @laryn @quicksketch. I think either/both get it to work for me:

quicksketch commented 2 months ago

Thanks for testing @totten! With @herbdool's confirmation, I went ahead and merged https://github.com/backdrop/backdrop/pull/4724 to make testing easier. I'm feeling pretty good that this change reduces the logical differences in the installer code from what we have in 1.27.x.

@yorkshire-pudding If the problem is still yet occurring for you; it's possible there's another problem at play here still, but it looks as though this change has fixed at least one scenario. Please reopen if you still can reproduce the issue with the latest 1.x.

indigoxela commented 2 months ago

Fantastic! I can confirm, that the latest commit fixes the installer. :+1: Tested with the defaults (md5 based names for the config dir), on PHP 8.2, using the web UI.

yorkshire-pudding commented 2 months ago

I see this has all been sorted while I was out and then asleep. I've tested and confirm it all works.

klonos commented 2 months ago

Just for my benefit (because I've seen similar errors before), this was introduced in 1.28.x, right? (assuming with the changes for #2277) ...it wasn't reproducible before, in 1.27.x. Was it?

yorkshire-pudding commented 2 months ago

Just for my benefit (because I've seen similar errors before), this was introduced in 1.28.x, right? (assuming with the changes for #2277) ...it wasn't reproducible before, in 1.27.x. Was it?

Yes. That is correct