Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Wrong or missing config.inc.php variable breaks the upgrade #9781

Open
marcbria opened this issue Mar 8, 2024 · 1 comment
Open

Wrong or missing config.inc.php variable breaks the upgrade #9781

marcbria opened this issue Mar 8, 2024 · 1 comment

Comments

@marcbria
Copy link
Collaborator

marcbria commented Mar 8, 2024

Describe the bug
The issue was initially posted in the support forum here.

In short, if config.inc.php is not properly updated between minor upgrades, the upgrade can't finish the process and left a corrupted DB.

To Reproduce
Steps to reproduce the behavior:

  1. Upgrade any OJS 3.3, to 3.4 without changing config.inc.php (ie: be sure sendmail vars are not set).
  2. See error.

What application are you using?
OJS, OMP or OPS version 3.3.x

Additional information
Detailed error log is:

/opt/php80/bin/php tools/upgrade.php check
PHP Fatal error: Uncaught Exception: Mailer driver isn’t specified in the application’s config in /var/www/user/data/www/ojs/lib/pkp/classes/core/PKPContainer.php:315
Stack trace:
#0 /var/www/user/data/www/ojs/lib/pkp/classes/core/PKPContainer.php(268): PKP\core\PKPContainer::getDefaultMailer()
#1 /var/www/user/data/www/ojs/lib/pkp/classes/core/PKPContainer.php(117): PKP\core\PKPContainer->loadConfiguration()
#2 /var/www/user/data/www/ojs/lib/pkp/classes/core/PKPApplication.php(231): PKP\core\PKPContainer->registerConfiguredProviders()
#3 /var/www/user/data/www/ojs/lib/pkp/classes/core/PKPApplication.php(203): PKP\core\PKPApplication->initializeLaravelContainer()
#4 /var/www/user/data/www/ojs/classes/core/Application.php(50): PKP\core\PKPApplication->__construct()
#5 /var/www/user/data/www/ojs/lib/pkp/includes/bootstrap.php(37): APP\core\Application->__construct()
#6 /var/www/user/data/www/ojs/lib/pkp/classes/cliTool/CommandLineTool.php(41): require_once(‘/var/www/user…’)
#7 /var/www/user/data/www/ojs/tools/bootstrap.php(17): require(‘/var/www/user…’)
#8 /var/www/user/data/www/ojs/tools/upgrade.php(19): require(‘/var/www/user…’)
#9 {main}
thrown in /var/www/user/data/www/ojs/lib/pkp/classes/core/PKPContainer.php on line 315

As far as config.inc.php usually change between minor or major upgrades, this becomes a quite usual issue.
As far as the DB is left corrupted and some fellows don't remember to backup-before-upgrade, I feel this need to be properly priorized.

A possible minimal solution is adding pre-fight check of the config.inc.php to confirm all the essential/new vars are included (or at least, the new ones added between usual upgrade paths). List of new vars is published in the Release Notes.

A middle ground solution would be adding a config validator that could be cached based on the modified date + application version.

An improved solution (suggested by Andrew Gearhart) would be create a "config builder". It will be able to handle valid, discontinued, and required settings across different software versions, specifying supported values (like "On/Off, True/False, 1/0, String, Integer, Float"). Using YAML/JSON file will facilitate the editing and potentially implementing a front-end TUI for creating the config file can streamline the process and address previous challenges in passing configurations.

The final solution will probably be stop maintaining a homebrew config file and adopt standard tools for config file maintenance. Preferably that would mean Laravel's (since we're going that way with so much else).

For any final solution other issues also need to be taken in consideration as:

@asmecher
Copy link
Member

asmecher commented Mar 8, 2024

I've just double-checked what happens when the driver isn't specified on a local 3.4.0-x checkout, and running php tools/upgrade.php upgrade dies immediately with Mailer driver isn't specified in the application's config error message. The database isn't corrupted in the process, so it should just be necessary to add the missing configuration (as documented in the release notes) and re-run the upgrade.

Can anyone verify that the database can be corrupted without this config being added?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants