Skip to content

[3.0] Rewrites installer and upgrader #8652

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

Open
wants to merge 51 commits into
base: release-3.0
Choose a base branch
from

Conversation

Sesquipedalian
Copy link
Member

@Sesquipedalian Sesquipedalian commented May 19, 2025

This is the new installer and upgrader code. It is based on the work in #8093.

It works in my tests, but it probably has bugs. Let's find 'em and fix 'em.

Fixes #7954
Fixes #8261
Fixes #8570
Fixes #8599
Fixes #8616
Fixes #8648

Signed-off-by: Jon Stovell <jonstovell@gmail.com>
Signed-off-by: Jon Stovell <jonstovell@gmail.com>
Signed-off-by: Jon Stovell <jonstovell@gmail.com>
Signed-off-by: Jon Stovell <jonstovell@gmail.com>
Signed-off-by: Jon Stovell <jonstovell@gmail.com>
Signed-off-by: Jon Stovell <jonstovell@gmail.com>
Signed-off-by: Jon Stovell <jonstovell@gmail.com>
Signed-off-by: Jon Stovell <jonstovell@gmail.com>
Signed-off-by: Jon Stovell <jonstovell@gmail.com>
Signed-off-by: Jon Stovell <jonstovell@gmail.com>
Signed-off-by: Jon Stovell <jonstovell@gmail.com>
Signed-off-by: Jon Stovell <jonstovell@gmail.com>
Signed-off-by: Jon Stovell <jonstovell@gmail.com>
Signed-off-by: Jon Stovell <jonstovell@gmail.com>
Signed-off-by: Jon Stovell <jonstovell@gmail.com>
Copy link
Member

@jdarwood007 jdarwood007 left a comment

Choose a reason for hiding this comment

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

Is 2.0 and before stuff going up separately?

@Sesquipedalian
Copy link
Member Author

Is 2.0 and before stuff going up separately?

This PR already includes the necessary migration steps to upgrade from 2.0. We had previously discussed the plan to make upgrading from 1.x a separate converter.

Signed-off-by: Jon Stovell <jonstovell@gmail.com>
Signed-off-by: Jon Stovell <jonstovell@gmail.com>
Signed-off-by: Jon Stovell <jonstovell@gmail.com>
Signed-off-by: Jon Stovell <jonstovell@gmail.com>
@jdarwood007
Copy link
Member

Is 2.0 and before stuff going up separately?

This PR already includes the necessary migration steps to upgrade from 2.0. We had previously discussed the plan to make upgrading from 1.x a separate converter.

I had planned to bake the converter logic into the installer/upgrader to make it still easy to convert Yabb, SMF 1.x and 2.0. The reason is that we are still supporting them, but only through a conversion process. Yabb and SMF 1.x cost a lot of technical debt to maintain the upgrader logic for. So by moving into converters, we reduce that to maintaining how to convert data, which becomes easier with a select this and insert that process.

@Sesquipedalian
Copy link
Member Author

Sesquipedalian commented May 21, 2025

Sure. But that would all just be a matter of writing Migration and Cleanup classes, plus a Tools class to call them, right?

@sbulen
Copy link
Contributor

sbulen commented May 21, 2025

I just ran a 2.1 =>3.0 upgrade & it went very well.

EDIT: I had to rerun for... reasons... Different issues reported below...

On completion, I got this error in the Cleanup phase:
Critical Error!
fileinode(): Argument # 1 ($filename) must be of type string, false given
D:\wamp64\www\van2130\Sources\Maintenance\Cleanup\v3_0\TasksDirCase.php:62

@sbulen
Copy link
Contributor

sbulen commented May 29, 2025

Just ran the 2.1 => 3.0 upgrade via browser... All the same comments as above, but a minor nit about the graphics - the weird two dark grey lines below the 'please be patient' line. Note I did not ask to have details displayed. I think this is the box for the details:
image

@sbulen
Copy link
Contributor

sbulen commented May 29, 2025

Just ran a 3.0 => 3.0 upgrade via CLI and this is the bottom part of the log:

...
...
+++ Converting "smf_topics" to utf8mb4... skipped.
+++ Converting "smf_user_alerts" to utf8mb4... skipped.
+++ Converting "smf_user_alerts_prefs" to utf8mb4... skipped.
+++ Converting "smf_user_drafts" to utf8mb4... skipped.
+++ Converting "smf_user_likes" to utf8mb4... skipped.

Warning: Undefined array key 72 in D:\wamp64\www\van2130\Sources\Maintenance\Utf8ConverterStep.php on line 667

Call Stack:
0.0005 394264 1. {main}() D:\wamp64\www\van2130\upgrade.php:0
0.0219 1417704 2. SMF\Maintenance\Maintenance->execute($type = 2) D:\wamp64\www\van2130\upgrade.php:24
1.3207 6775704 3. call_user_func:{D:\wamp64\www\van2130\Sources\Maintenance\Maintenance.php:294($callback = [0 => class SMF\Maintenance\Utf8ConverterStep private int $SMF\Maintenance\Step}id = 5; private string $SMF\Maintenance\Step}name = 'Convert to UTF-8'; private string $SMF\Maintenance\Step}title = 'Convert to UTF-8'; private array|string $SMF\Maintenance\Step function = ...; private string ${SMF\Maintenance\Steptemplate = 'convertUtf8'; private int $SMF\Maintenance\Step}progress = 30; public array $supported_charsets = [...]; public string $lang_charset = 'UTF-8'; public array $charset_maps = [...] , 1 => 'convertDatabase']) D:\wamp64\www\van2130\Sources\Maintenance\Maintenance.php:294
1.3207 6775888 4. SMF\Maintenance\Utf8ConverterStep->convertDatabase() D:\wamp64\www\van2130\Sources\Maintenance\Maintenance.php:294

Warning: Attempt to read property "name" on null in D:\wamp64\www\van2130\Sources\Maintenance\Utf8ConverterStep.php on line 667

Call Stack:
0.0005 394264 1. {main}() D:\wamp64\www\van2130\upgrade.php:0
0.0219 1417704 2. SMF\Maintenance\Maintenance->execute($type = 2) D:\wamp64\www\van2130\upgrade.php:24
1.3207 6775704 3. call_user_func:D:\wamp64\www\van2130\Sources\Maintenance\Maintenance.php:294($callback = [0 =&gt; class SMF\Maintenance\Utf8ConverterStep private int ${SMF\Maintenance\Stepid = 5; private string $SMF\Maintenance\Step}name = 'Convert to UTF-8'; private string $SMF\Maintenance\Steptitle = 'Convert to UTF-8'; private array|string $SMF\Maintenance\Step}function = ...; private string $SMF\Maintenance\Steptemplate = 'convertUtf8'; private int $SMF\Maintenance\Stepprogress = 30; public array $supported_charsets = [...]; public string $lang_charset = 'UTF-8'; public array $charset_maps = [...] }, 1 => 'convertDatabase']) D:\wamp64\www\van2130\Sources\Maintenance\Maintenance.php:294
1.3207 6775888 4. SMF\Maintenance\Utf8ConverterStep->convertDatabase() D:\wamp64\www\van2130\Sources\Maintenance\Maintenance.php:294

Step 7: Finalize Upgrade
Upgrade complete!

@sbulen
Copy link
Contributor

sbulen commented May 29, 2025

That's enough for tonight... See you in a week.

Your meditation exercise for the week: contemplate a 2.1.2 => 3.1.3 upgrade. Will the id_board index be there?

And when working on 3.1, do the 3.0 & 3.1 migration release branches for 3.0 need to be kept in sync? How?

@sbulen
Copy link
Contributor

sbulen commented May 29, 2025

One last question: Where's the log?

Ah... Saw your note above, it's only around when it fails...

Hmmm... I'm thinking support board questions... Not sure I'd want it deleted. Might want to consult @Kindred-999 ???

@sbulen
Copy link
Contributor

sbulen commented May 29, 2025

A thought...

If you had migration folders for the point releases, you're probably golden... Just step through each.

In 2.1, you started saving the version in the settings table that way - e.g., "2.1.4". So you know the precise starting point.

We'd need a 2.1.5 one for the two changes in question.

And that way, the exact same process could be used in point releases as in the upgrader. All locked in step. The only difference is the starting point.

@Sesquipedalian
Copy link
Member Author

Sesquipedalian commented May 30, 2025

But it is needed in 2.1 also - its a 2.1 change.

I'm thinking about 3.0 & 3.1 as well, when both are being worked on. The change should really only live in one release.

Your meditation exercise for the week: contemplate a 2.1.2 => 3.1.3 upgrade. Will the id_board index be there?

And when working on 3.1, do the 3.0 & 3.1 migration release branches for 3.0 need to be kept in sync? How?

I have thought about these things. I respectfully suggest that you are thinking about them wrongly, my friend, and in two key ways.

First, you seem to be thinking as though the upgrader needs to replay the evolution of our database structure with a patch release level of granularity, as though we need to model 2.1.0 → 2.1.1 → 2.1.2 → 2.1.3 → 2.1.4 → 2.1.5 → ... → 3.0.0 in the upgrader. Because of this way of thinking about it, you think that 2.1.5's fix to the log_notify table's primary index needs to be handled in an upgrade step that happens before the 2.1 → 3.0 upgrade.

However, the upgrader actually replays the evolution of the database structure with a point release level of granularity, e.g. 2.0.x → 2.1.x, or 2.1.x → 3.0.x. That means a database change introduced in a patch release needs to be reflected in two places: in a 2.1.x → 2.1.x upgrade (i.e. rerunning the upgrader), and in a 2.1.x → 3.0.x upgrade.

That brings us to the second point. You seem to be thinking of the upgrader as though it were an un-versioned, universal thing that applies to all SMF versions equally—we just tack on new sets of changes with each new version of SMF.

But that's not the case. Each version of SMF has a corresponding version of the upgrader, and these versions are indeed distinct. Specifically, only the 2.1 version of the upgrader will ever perform a 2.1.x → 2.1.x upgrade, while only the 3.0 version will ever perform a 3.0.x → 3.0.x upgrade, etc. This difference is why the change to the database in 2.1.5 needs to be reflected in two different places. In the 2.1 upgrader, it needs to be reflected in the 2.1.x → 2.1.x steps. But in the 3.0 upgrader (and beyond), it needs to be reflected in the 2.1.x → 3.0.x steps, because no 2.1.x → 2.1.x steps are or could ever be performed by the 3.0 upgrader.

@Sesquipedalian
Copy link
Member Author

Sesquipedalian commented May 30, 2025

And when working on 3.1, do the 3.0 & 3.1 migration release branches for 3.0 need to be kept in sync? How?

This, like everything else, is managed by Git. As I said in my previous reply, the upgrader is, in fact, versioned along with SMF as a whole and as part of SMF as a whole. When 3.1 is forked from 3.0, it will inherit the migration steps from the 3.0 branch. As new database changes are introduced in 3.1, migration steps will be added for them in the 3.1 branch. If we ever find ourselves needing to introduce a database change in a patch to 3.0, a migration step for that will end up being added in both the 3.0 branch as part of the 3.0x → 3.0.x steps and, separately, in the 3.1 branch as part of the 3.0.x → 3.1.x steps. That's all there is to it.

@Sesquipedalian
Copy link
Member Author

  • After upgrade, it took me a few attempts to login.

That's actually not the upgrader's fault. It's due to the changes introduced in #8464 (specifically, in fed16cc). At the time when I was working on that PR, I was flipping back and forth so much between the old and new password hash values that I didn't notice that it would take two tries to log in successfully once the change was made. We need to make some more changes— probably in SMF\Actions\Login2—in order to handle the change seamlessly.

@Sesquipedalian
Copy link
Member Author

  • I'm missing the id_board index on smf_log_notify
  • I'm missing smf_version on smf_log_packages

Both should be fixed with the latest commits.

Signed-off-by: Jon Stovell <jonstovell@gmail.com>
@Sesquipedalian
Copy link
Member Author

Sesquipedalian commented May 30, 2025

One last question: Where's the log?

Ah... Saw your note above, it's only around when it fails...

Hmmm... I'm thinking support board questions... Not sure I'd want it deleted. Might want to consult @Kindred-999 ???

Log files are now saved in a secured ./logs directory. If that directory can't be created or can't be secured, then the log file is deleted after the tool completes its process.

Signed-off-by: Jon Stovell <jonstovell@gmail.com>
Signed-off-by: Jon Stovell <jonstovell@gmail.com>
@Sesquipedalian
Copy link
Member Author

  • I'm getting cron errors on entity decode, see pic below

b2a365f should fix that.

Just ran a 3.0 => 3.0 upgrade via CLI and this is the bottom part of the log:

Warning: Undefined array key 72 in D:\wamp64\www\van2130\Sources\Maintenance\Utf8ConverterStep.php on line 667

54c268a should fix that.

@Sesquipedalian
Copy link
Member Author

No luck with the installer...

I copied down 3 files from /other: install.php, Settings.php & Settings_bak.php. If I should be doing something different than that, let me know.

PHP 8.4.5, MySQL 8.4.4.

Also tried with PHP 8.4.5 & Postgresql 14, & got the same result.

image

Hm. Weird. I'll need to experiment to see if I can reproduce that and figure out a cause.

Signed-off-by: Jon Stovell <jonstovell@gmail.com>
Signed-off-by: Jon Stovell <jonstovell@gmail.com>
Signed-off-by: Jon Stovell <jonstovell@gmail.com>
Signed-off-by: Jon Stovell <jonstovell@gmail.com>
Signed-off-by: Jon Stovell <jonstovell@gmail.com>
Signed-off-by: Jon Stovell <jonstovell@gmail.com>
Signed-off-by: Jon Stovell <jonstovell@gmail.com>
Signed-off-by: Jon Stovell <jonstovell@gmail.com>
Signed-off-by: Jon Stovell <jonstovell@gmail.com>
@Sesquipedalian
Copy link
Member Author

Latest commits should fix all reported issues with the installer (and several unreported ones, too).

Signed-off-by: Jon Stovell <jonstovell@gmail.com>
Signed-off-by: Jon Stovell <jonstovell@gmail.com>
Signed-off-by: Jon Stovell <jonstovell@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment