[3.0] Miscellaneous fixes for the installer and upgrader#9162
Conversation
|
So... Test here next??? |
|
On this PR... Results are actually very good. Only a couple minor nits & questions. 2.1 => 3.0:
Install: Ran fine. Only minor nits...
General observations... These are both maybe just me & my system, not sure... Any thoughts would be welcome.
I think the performance is actually a 3.0 thing. I don't see that elsewhere. I think the self-signed cert is a me thing... I see that elsewhere. No matter what I've tried, if I try to force https, I get sslRedirect within the app. And the installer never detects the cert, e.g., the ssl box is grayed out. I hate putzing with certs... |
|
I'd be OK with this going in as-is. Only nits. |
|
I could dive deeper & try 2.0 upgrades if needed. Sound like a plan? |
Hm. That's not good.
Should be an easy fix.
I haven't noticed that in my tests, but I wasn't particularly looking for it either.
Yeah, that's probably just a you thing. Rather than a self-signed certificate, why not just use one from Let's Encrypt? |
Sounds like a great plan! Thanks, @sbulen. 😊 |
Most of my testing is on localhost. Once things progress, I do final testing on a hosted test environment. For a long time, no issues whatsoever, but browsers have been getting more picky over time. For years, a self-signed cert worked perfectly. Until it didn't... For a while there, I had to create my own CA for local use. Now, that is either unsupported, or, I'm doing it wrong... |
2.0 latin1 => 3.0:
php/apache/mysql logs: Nothing upgrader log:
2.0 utf8 => 3.0:
php/apache/mysql logs: Nothing upgrader log:
pg => 3.0:
php/apache/mysql logs: Nothing upgrader log:
MariaDB 11 => 3.0:
php/apache/mysql logs: Nothing upgrader log:
|
1e36b28 to
727233c
Compare
These two should be fixed in the latest commits. |
Hm. Does the admin_info_files table not exist in the database before the upgrade starts? |
Bleh. Please run it again with this modified version of ./Sources/Maintenance/Migration/v2_1/CustomFieldsPart1.php. It will once again dump a bunch of crap into upgrade.log so I can figure out where things are going wrong. <?php
/**
* Simple Machines Forum (SMF)
*
* @package SMF
* @author Simple Machines https://www.simplemachines.org
* @copyright 2026 Simple Machines and individual contributors
* @license https://www.simplemachines.org/about/smf/license.php BSD
*
* @version 3.0 Alpha 4
*/
declare(strict_types=1);
namespace SMF\Maintenance\Migration\v2_1;
use SMF\Db\DatabaseApi as Db;
use SMF\Db\Schema;
use SMF\Maintenance\Maintenance;
use SMF\Maintenance\Migration\MigrationBase;
class CustomFieldsPart1 extends MigrationBase
{
/*******************
* Public properties
*******************/
/**
*
*/
public string $name = 'Upgrade Custom Fields (Preparing)';
/****************
* Public methods
****************/
/**
*
*/
public function execute(): bool
{
Maintenance::$tool->logProgress(__LINE__, true);
$start = Maintenance::getCurrentStart();
if ($start <= 0) {
$table = new Schema\v2_1\CustomFields();
Maintenance::$tool->logProgress(__LINE__, true);
$table->normalize();
Maintenance::$tool->logProgress(__LINE__, true);
$this->handleTimeout(++$start);
}
if ($start <= 1) {
Maintenance::$tool->logProgress(__LINE__, true);
$table->populate();
Maintenance::$tool->logProgress(__LINE__, true);
$this->handleTimeout(++$start);
}
if ($start <= 2) {
Maintenance::$tool->logProgress(__LINE__, true);
// Add an order value to each existing cust profile field.
$ocf = $this->query(
'SELECT id_field
FROM {db_prefix}custom_fields
WHERE field_order = 0',
);
// We start counting from 5 because we already have the first 5 fields.
$fields_count = 5;
Maintenance::$tool->logProgress(__LINE__, true);
while ($row = Db::$db->fetch_assoc($ocf)) {
++$fields_count;
Maintenance::$tool->logProgress($row['id_field'], true);
$this->query(
'UPDATE {db_prefix}custom_fields
SET field_order = {int:field_count}
WHERE id_field = {int:id_field}',
[
'field_count' => $fields_count,
'id_field' => $row['id_field'],
],
);
}
Db::$db->free_result($ocf);
Maintenance::$tool->logProgress(__LINE__, true);
$this->handleTimeout(++$start);
}
return true;
}
} |
2ab8fe3 to
e9dbf3c
Compare
Yes, the table exists, & the prefix is specified in Settings.php. It's a simple 2.1 install. I suspect the problem here, though, is that somehow the prefix was lost... It's not "admin_info_files", it's "smf_admin_info_files". |
|
Ah, yes. That would likely explain it. I'll take a look when I have time later. |
MariaDB 2.1 => 3.0:Refreshed the code, implemented your tweak above & reattempted the upgrade. It actually got much further... 
php/apache/mysql logs: Nothing upgrader log (bottom):
|
Should be fixed in latest commits.
Should be fixed in latest commits. If not, things might get a lot harder. Ready for another round of testing, @sbulen. 🙂 |
3.0 Install:
2.1 => 3.0:
2.0 latin1 => 3.0:
php/apache/mysql logs: Nothing upgrade.log (bottom):
2.0 utf8 => 3.0:
php/apache/mysql logs: Nothing upgrade.log (bottom):
pg 2.1 => 3.0:
php/apache/mysql logs: Nothing upgrade.log: MariaDB11 2.1 => 3.0:
php/apache/mysql logs: Nothing upgrade.log (bottom):
|
|
Thanks, @sbulen. It's been a while since I have been able to look at this PR again. I appreciate both your help and your patience.
Should be fixed in latest commit.
For diagnostic purposes, please run it again using this modified version of ./Sources/Maintenance/Migration/v2_1/CreateAlerts/php:
For diagnostic purposes, please run it again using this modified version of ./Sources/Maintenance/Migration/v2_1/PostgreSqlSequences.php:
For diagnostic purposes, please run it again using this modified version of ./Sources/Maintenance/Migration/v2_1/CustomFieldsPart1.php: |
|
Suggestion: Maybe we should just keep the debug stuff in there. That will help reduce back & forth. Consider even making it a switch. Can't have too much diagnostic info & options in these upgrades, IMO... 2.0 latin1 => 3.0:
php/apache/mysql logs: Nothing upgrade.log:
2.0 utf8 => 3.0:
php/apache/mysql logs: Nothing upgrade.log:
pg 2.1 => 3.0:
php/apache/mysql logs: Nothing upgrade.log (portion - this went on for days...):
MariaDB11 2.1 => 3.0:Doesn't look like if failed in the same place... Not sure why: Note the weird display - all the debug lines are still on the same line, no line feeds: 
php/apache/mysql logs: Nothing upgrade.log (bottom only): |
|
A clue, not sure how much it helps.... The generic 'Critical Error' errors appear to be on the calls to handleTimeout() within a step. More likely to happen when attempting a restart. Sometimes, it just seems to fall thru to exit... Trace follows... This logic is new to me, but it seems to be checking for json in $_GET, that isn't there? |
|
OK, did some more sleuthing, & have a couple of things to share...
I'm not quite sure what the fix is unless the upgrader can force a hard refresh to ensure new .js files are loaded. I think this is causing all of the CRITICAL errors w/o messages.
...is being rejected in MariaDB. I'm not sure why exactly yet... Maybe a default or a SQL_MODE issue set for the session. Works OK without the quotes, but I see the same behavior in MySQL. Something appears to be making them behave differently at runtime. |
|
That is all very helpful, @sbulen. Thanks! I'll see what I can do about them. |
|
I do see this in the browser console, it might help? |
Signed-off-by: Jon Stovell <jonstovell@gmail.com>
Signed-off-by: Jon Stovell <jonstovell@gmail.com>
a0301c7 to
7c2887e
Compare
Signed-off-by: Jon Stovell <jonstovell@gmail.com>
7c2887e to
bae2aec
Compare
|
That was helpful, @sbulen, thanks. I think the latest commits have solved that issue. When a timeout occurred, we were sending an empty response, which the JavaScript interpreted as an error. Now we send an appropriate JSON response. |
3.0 InstallRan clean thru completion! MySQL/php/apache logs: Clean 2.1 => 3.0Ran clean thru completion! Nits:
MySQL/php/apache logs: Clean Upgrade.log (bottom):
2.0 latin1 => 3.0
MySQL/php/apache logs: Nothing Upgrade.log (bottom):
2.0 utf8 => 3.0Odd, this is the first thing that pops up, none of the normal welcome banners, login, etc. I repeated this test a couple times, same results: 
MySQL/php/apache logs: Nothing Upgrade.log (complete):
|
Ugh, these wretched IP columns! Can you show me the CREATE TABLE statement for the rul_log_online table from that database before you started the upgrade, and again immediately after that error occurs? I want to see what its structure looks like before and after. |
Was there an errant |
BEFORE (Restored from a fresh 2.0 install without the utf8 box checked, matches install definition):
AFTER (After upgrade attempt - note it didn't get as far as utf8 conversion):
That default value looks sketchy - note it would be "0x30".... Which looks like we were aiming for "0x0"... Also note the InnoDB conversion was completed? I thought that came later.... |
Sorry about that - must've been... I swear I double-checked that & repeated the test. Acting normal today... PEBKAC... (There was NO beer involved, I swear...) The utf8 test now behaves like the latin1 test, which is a good thing: 
|
Yup, I agree. Latest commit explicitly sets the default to null for all inet columns (except for smf_log_flood_control.ip, which cannot be null). I think that will resolve this problem. |
Signed-off-by: Jon Stovell <jonstovell@gmail.com>
0f516cc to
d0e3ab1
Compare
3.0 InstallRan clean thru completion! MySQL/php/apache logs: Clean 2.1 => 3.0Ran clean thru completion! Nits:
MySQL/php/apache logs: Clean 2.0 latin1 => 3.0Still getting this, no idea why: DB - Note the other changes are in place, e.g., the session size doesn't match 2.0, nor does the URL definition or log_time. So the other changes are happening OK: Note the default looks null in the code, both the 2.1 branch & the 3.0 branch: So, no idea where the 0x30 thing is coming from. MySQL/php/apache logs: Nothing |
|
Observation on the IP size issue: We can't just put the target 3.0 definition out there for the IPs... There's a whole conversion from ipv4 to ipv6 that needs to take place, that took place between 2.0 & 2.1. Until that takes place, the ipv4 definition needs to stand. (Which goes back to my old concern that some of these need to go stepwise thru SMF versions, like the old model...) Note the old 2.0 default is '0', which is where I suspect the 0x30 is ultimately coming from. The tables that look OK (log_errors) are empty. BUT... The definition looks good. Other random observation: log_online has this step, which the other records don't:
|
Signed-off-by: Jon Stovell <jonstovell@gmail.com>
That led me to go back and look again at the old upgrade_2-1_mysql.sql file. As it turns out, the approach taken there was to drop the column and then add it back again with an updated definition. I'm going to guess that this was done in order to avoid the same issue we've been facing here. Latest commit follows the same drop-and-add strategy. |
This was no longer used for anything in SMF 2.1, but the column was retained in the members table. Signed-off-by: Jon Stovell <jonstovell@gmail.com>
|
@sbulen, do you think you will have time to test this again in the next little while? I'd really like to put this one to bed. |
3.0 InstallRan clean thru completion, but with this error: 
These errors occur upon every screen refresh within the app. MySQL/php/apache logs: the php log includes the above error 2.1 => 3.0Ran clean thru completion! Note the same error occurs: Undefined array key "secureCookies" navigating the forum. Nits:
MySQL/php/apache logs: the php log includes the above error Let me know if you want me to power ahead, or if you want to tweak this first. |
That makes absolutely no sense to me. That line hasn't changed since 2023, and it looks like this: $this->secure = $secure ?? !empty(Config::$modSettings['secureCookies']);It's wrapped in a call to At any rate, I find it hard to believe this has anything to do with the upgrader directly. EDIT: I think you must have had some mixed code in there. Line 208 in ./Sources/Cookie.php did change in 81b9abf, but that commit isn't part of this branch. It will need to be fixed, but it's not part of this PR.
Yes, please keep going. |
|
Clarification: I refresh & rebuild before every round of testing, & always have. For any PR, including my own. I need to test what would happen if the PR were merged today, not weeks/months ago... (The upgrader borrows many functions from /Sources.) So the 'secure cookies' errors appear in all tests below, in the SMF & php error logs. Good news: IMO, I think we're close enough to merge & treat the remaining issues standalone. Only one outstanding hard issue with PG. Minor nits in logon post conversion, as reported earlier. If you want to push ahead, I'll log the above separately. 3.0 InstallRan clean thru completion & forum works fine! MySQL/php/apache logs: Clean 2.1 => 3.0Ran clean thru completion & forum works fine! Nits:
MySQL/php/apache logs: Clean Upgrade log: Clean 2.0 latin1 => 3.0Ran clean thru completion & forum works fine! *** log_online structure looks good, matches install (varbinary(16) NULL). Nits:
Upgrade.php deleted properly (unlike 2.1 => 3.0 above). MySQL/php/apache logs: Clean Upgrade log: Clean 2.0 utf8 => 3.0Ran clean thru completion & forum works fine! *** log_online structure looks good, matches install (varbinary(16) NULL). Nits:
Upgrade.php deleted properly (unlike 2.1 => 3.0 above). MySQL/php/apache logs: Clean Upgrade log: Clean 2.1 MariaDB => 3.0Ran clean thru completion & forum works fine! Interesting:
MySQL/php/apache logs: Clean Upgrade log: Clean 2.1 PG => 3.0
MySQL/php/apache logs: Clean Upgrade log (bottom):
|
|
Update: I was trying to drill down on the composer error reported earlier, & I discovered that that error, plus the null error, were both from BEFORE the upgrade... What I think happened was that I had 2.1.7 forum up in the background while copying the 3.0 files over on top of the 2.1.7 code (mimicking an upgrade); I hadn't yet copied upgrade.php to root. Index.php was invoked in that mixed state. (I suspect either the ajax alert popup, or a brain fart on my part pulling up the tab too soon...) TL;DR, I reran both MariaDB & Postgresql this am, without 2.1.7 running in the background, and there were no null or composer errors. I updated the report above accordingly. |
|
Lovely! Thank you, @sbulen. I agree that it makes sense to merge now and deal with remaining problems separately. Please do report those remaining problems as their own issues. I really appreciate your faithful testing of all these changes, my friend! |
3 participants
Fixes a bunch of stuff that has come up while testing the installer and upgrader.
@sbulen, we can use this PR to do all our testing to get 2.1 → 3.0 and 2.0 -> 3.0 upgrades fully working, as well as deal with any issues in the installer. Then we'll go back to #9120 afterward.
Fixes #9160