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

Fix for 5.59 upgrade on multilingual #25716

Merged
merged 1 commit into from
Mar 4, 2023

Conversation

MegaphoneJon
Copy link
Contributor

Overview

https://lab.civicrm.org/dev/core/-/issues/4155

Before

Can't access CiviCRM after upgrading to 5.59.0.

After

Can access site after upgrading to 5.59.1.

Technical Details

The fix in CRM_Upgrade_Incremental_Base is what should have been there to begin with, and will prevent this in the future. I've tested that successfully. The fix in CRM_Upgrade_Incremental_php_FiveFiftyNine is untested because I'm not 100% sure this is all that's needed, but I'm sure whomever reviews this will point out my errors :). Mainly I just want a fix available ASAP.

@civibot
Copy link

civibot bot commented Mar 2, 2023

(Standard links)

@civibot civibot bot added the 5.59 label Mar 2, 2023
@MegaphoneJon MegaphoneJon changed the title Overview ---------------------------------------- https://lab.civicrm.org/dev/core/-/issues/4155 Before ---------------------------------------- Can't access CiviCRM after upgrading to 5.59.0. After ---------------------------------------- Can access site after upgrading to 5.59.1. Technical Details ---------------------------------------- The fix in CRM_Upgrade_Incremental_Base is what should have been there to begin with, and will prevent this in the future. I've tested that successfully. The fix in CRM_Upgrade_Incremental_php_FiveFiftyNine is untested because I'm not 100% sure this is all that's needed, but I'm sure whomever reviews this will point out my errors :). Mainly I just want a fix available ASAP. Fix for 5.59 upgrade on multilingual Mar 2, 2023
@eileenmcnaughton
Copy link
Contributor

Fail relates to version - note that we still need to merge this to 5.60 before it can go into 5.59 so maybe just change the target to the rc & it will pass & then in the backport the version can be incremented

In Form.php line 509:

[CRM_Core_Exception]
Malformed upgrade sequence. The incremental update 5.59.1 exceeds target v
ersion 5.59.0

Exception trace:
at /home/jenkins/bknix-dfl/build/build-0/web/sites/all/modules/civicrm/CRM/Upgrade/Form.php:509
CRM_Upgrade_Form::buildQueue() at phar:///home/jenkins/bknix-dfl/bin/cv/src/Command/UpgradeDbCommand.php:117
Civi\Cv\Command\UpgradeDbCommand->execute() at phar:///home/jenkins/bknix-dfl/bin/cv/vendor/symfony/console/Command/Command.php:255
Symfony\Component\Console\Command\Command->run() at phar:///home/jenkins/bknix-dfl/bin/cv/vendor/symfony/console/Application.php:1009

if ($locales) {
CRM_Core_I18n_Schema::rebuildMultilingualSchema($locales, NULL, TRUE);
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@MegaphoneJon need a blank line added after this line

@MegaphoneJon
Copy link
Contributor Author

Today got busy - I can make fixes tomorrow (in ~16 hours) but if someone wants to push this ahead, they should feel free.

@totten
Copy link
Member

totten commented Mar 3, 2023

I did an r-run and got similar issue as CI. We can clear that up by either dropping FiveFiftyNine patch or bumping up the version#. So that' leads into Jon's question at the top:

The fix in CRM_Upgrade_Incremental_Base is what should have been there to begin with, and will prevent this in the future. I've tested that successfully. The fix in CRM_Upgrade_Incremental_php_FiveFiftyNine is untested because I'm not 100% sure this is all that's needed, but I'm sure whomever reviews this will point out my errors :). Mainly I just want a fix available ASAP.

After locally r-running the patch and then rereading it, I think we can remove the part from FiveFiftyNine. It helps to consider two upgrade-paths:

  1. (One Step) 5.58.1 => 5.59.1: In my test, the Base patch appeared sufficient to fix the DB Error: -32. The FiveFiftyNine step is extraneous here.

  2. (Two Step) 5.58.1 => 5.59.0 => 5.59.1: After the first upgrade, the site is left in non-functional condition -- it needs to run rebuildMultilingualSchema(). In theory, the FiveFiftyNine step would help fix-up the site during the second upgrade.

    But it doesn't work. If you have a broken 5.59.0 site, then you can't even run the upgrader. Both /civicrm/upgrade?reset=1 and cv upgrade:db raise DB Error: -32.

    Found CiviCRM database version 5.59.0.
    Found CiviCRM code version 5.59.1.
    Checking pre-upgrade messages...
    
    In api.php line 135:
    
      [CRM_Core_Exception]
      DB Error: -32
    
    
    Exception trace:
      at /Users/totten/bknix/build/dmaster/web/sites/all/modules/civicrm/api/api.php:135
     civicrm_api3() at /Users/totten/bknix/build/dmaster/web/sites/all/modules/civicrm/CRM/Upgrade/Incremental/General.php:137
     CRM_Upgrade_Incremental_General::setPreUpgradeMessage() at /Users/totten/bknix/build/dmaster/web/sites/all/modules/civicrm/CRM/Upgrade/Form.php:877
     CRM_Upgrade_Form->setPreUpgradeMessage() at phar:///Users/totten/bknix/bin/cv/src/Command/UpgradeDbCommand.php:92
     Civi\Cv\Command\UpgradeDbCommand->execute() at phar:///Users/totten/bknix/bin/cv/vendor/symfony/console/Command/Command.php:255
     Symfony\Component\Console\Command\Command->run() at phar:///Users/totten/bknix/bin/cv/vendor/symfony/console/Application.php:1009
     Symfony\Component\Console\Application->doRunCommand() at phar:///Users/totten/bknix/bin/cv/vendor/symfony/console/Application.php:273
     Symfony\Component\Console\Application->doRun() at phar:///Users/totten/bknix/bin/cv/src/Application.php:82
     Civi\Cv\Application->doRun() at phar:///Users/totten/bknix/bin/cv/vendor/symfony/console/Application.php:149
     Symfony\Component\Console\Application->run() at phar:///Users/totten/bknix/bin/cv/src/Application.php:49
     Civi\Cv\Application::main() at phar:///Users/totten/bknix/bin/cv/bin/cv:27
     require() at /Users/totten/bknix/bin/cv:14
    

    As Jon mentioned on Gitlab, you can fix it by running this step manually (cv api System.rebuildmultilingualschema). But if you do that, then you don't need the upgrader to do it.

Maybe we can figure some other intervention (e.g. another patch/pr) to help with the "Two Step" scenario. For the moment, I'll push an update so Jenkins can give feedback on just the "Base" part.

@totten totten force-pushed the 559-multilingual2 branch from 45705bc to 8aa0fa5 Compare March 3, 2023 00:41
@totten
Copy link
Member

totten commented Mar 3, 2023

Exception trace:
at ..../api/api.php:135
civicrm_api3() at ..../CRM/Upgrade/Incremental/General.php:137
CRM_Upgrade_Incremental_General::setPreUpgradeMessage() at ..../CRM/Upgrade/Form.php:877

We've seen that line before...

@totten
Copy link
Member

totten commented Mar 4, 2023

Since tests pass and there's no objection to the smaller patch, I'll go ahead and merge -- the best way to head-off problems is to get the fix out there.

@totten totten merged commit aa70cdb into civicrm:5.59 Mar 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants