[UX][DX] Update.php throws error about install.php (and says to delete live config)

[jenlampton]

I'm experiencing some server issue (missing staging directory) that is preventing update.php from running.

When this happens, I would expect to see an error message letting me know that my staging directory is missing.

Unfortunately, instead I see an error about the installer that asks me to delete all my live config which is the last thing I should do.

Exception: The <em class="placeholder">../config/live-active</em> directory must be completely empty to proceed with installation. Empty this directory or specify a different directory in settings.php. in backdrop_install_config_directories() (line 851 of core/includes/install.inc).

It looks like update_prepare_bootstrap() calls backdrop_install_config_directories() even when the staging directory is missing, and that checks to confirm that the live-active directory is empty, since it's intended to be used for the installer.

Related:

• Staging directory should not need to be empty on install #2262
• Problem updating to Backrop 1.6.1  #2538

---

PR: backdrop/backdrop#2721

[jenlampton]

I've filed a PR that separates out problems with the staging directory form problems with the active directory. The staging directory is not required for this step, so a warning is printed to the screen letting people know about the problem - but not preventing them from proceeding with the updates.

[klonos]

Thanks for figuring this one out @jenlampton 👍

Pending testing this on my local (unless anyone else beats me to it), code-wise, everything seems to be in order, besides a missing period in the inline comment added in the PR. While at it, I recommended an alternative for consideration, that I believe better summarizes the situation.

[jenlampton]

I'd love some more testing on this one, bumping to 1.15.1

[jenlampton]

I ran into this problem again today, and was again baffled by the messaging about the wrong directory, and the dangerous recommendation. I'd love to get this resolved quickly, so I'm going to add it to the agenda for next week's meeting.

Thanks for the code review @klonos. It looks like I'd merged your PR to fix the inline comment :)

[ghost]

I was able to replicate the issue by installing an old version of Backup & Migrate, update it to the latest version, then delete my staging config directory. When I run update.php I got a fatal error about the active config directory (couldn't continue with updating the database).

I then added @jenlampton's patch and ran update.php again, and this time I simply got a warning about the staging config directory* and was able to continue with updating the database.

* My only concern before marking this RTBC is that the new warning message isn't very helpful... It simply says: "Unable to initialize the staging configuration directory [dir]"
Firstly, what does 'initialise the staging configuration directory' mean, and secondly, what do I need to do to fix it?

[ghost]

Marking as 'needs work' just for the message text. Otherwise the functionality works and the code is good.

[jenlampton]

Thanks @BWPanda. "initialize" is what was in the code, and not really suitable for people. How about Unable to locate the staging configuration directory %directory.
Would that be clear enough that what you need to is either create the directory, or update the value in settings.php to point to the correct location? Or should we add that into the message as well?

[ghost]

I think add it if possible. Something like:

"The staging configuration directory (%directory) could not be found. Please make sure it exists and is properly referenced in settings.php"
(or something along those lines)

[jenlampton]

done! (PR Updated)

[klonos]

Thanks for the code review @klonos. It looks like I'd merged your PR to fix the inline comment :)

Nope 😅 ...even if you decide to not accept my recommendation re the inline comment, it still has a missing period.

Thanks @BWPanda for providing steps to test this 👍 ...here's what is shown now:

First issue is that the file path is chopped off because the updater container is narrow. Not a biggie, but still.

Then the fact that there's a warning, makes me think twice before I click the continue button 🤔 ...should I address the problem that the message is warning me about, or is it safe to continue? ...would that break the db update process, and eventually my site, or what?

Since this warning is not relevant for the task at hand, how about we defer it for the next step?:

...or even display that in the status page perhaps?

[klonos]

...or even display that in the status page perhaps?

I'm leaning towards this ^^ ...if this check was in place regardless of running update.php, then as a site admin, I would have sought to fix it before running update.php. ...and if shown on the status page, then there is no need to have it shown when running update.php.

[jenlampton]

Can we make this a follow up?

…

On Thu, Aug 20, 2020, 4:32 PM Gregory Netsas ***@***.***> wrote: ...or even display that in the status page perhaps? I'm leaning towards this ^^ ...if this check was in place regardless of running update.php, then as a site admin, I would have sought to fix it before running update.php. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#3846 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AADBERZF63VEUSOS5P4QYNTSBWW73ANCNFSM4HSDVE7Q> .

[klonos]

Yes we can. In which case, this is RTBC if a period is added to this inline comment: https://github.com/backdrop/backdrop/pull/2721/files#diff-af3cd43667d2de1601d1e26a3f1165ecR96

[jenlampton]

Done!

[ghost]

Nice, RTBC from me.

[klonos]

Great teamwork 👍 ...and here's that follow-up: #4557

[ghost]

@quicksketch @herbdool Care to merge this please? It's been RTBC for two weeks but, since I marked it as such, I think someone else should do the honours...

[quicksketch]

Thanks! Merged backdrop/backdrop#2721 into 1.x and 1.16.x.