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 the installer/upgrader #7849

Merged
merged 11 commits into from
Nov 27, 2023

Conversation

jdarwood007
Copy link
Member

@jdarwood007 jdarwood007 commented Nov 5, 2023

( ! ) Fatal error: Uncaught Error: Typed static property SMF\Db\DatabaseApi::$db must not be accessed before initialization in /Sources/Config.php on line 1326
( ! ) Error: Typed static property SMF\Db\DatabaseApi::$db must not be accessed before initialization in /Sources/Config.php on line 1326
Call Stack
#	Time	Memory	Function	Location
1	0.0055	1343864	{main}( )	.../upgrade.php:0
2	0.0213	1703288	SMF\Config::generateSeed( )	.../upgrade.php:277
3	0.0213	1703664	SMF\Config::updateModSettings( $change_array = ['rand_seed' => 1108340315], $update = ??? )	

This is occuring because $modSettings['rand_seed'] is not set. All of $modSettings is not set, besides the default theme that gets set later in upgrade.php

This logic is called by this in ugprade.php

// Old DBs won't have this
if (!isset(Config::$modSettings['rand_seed']))
	Config::generateSeed();

We only reload the data when ssi is in the url, so this will always reseed the settings every time.

// Are we going to be mimic'ing SSI at this point?
if (isset($_GET['ssi']))
{
	User::load();
	User::$me->loadPermissions();
	Config::reloadModSettings();
}

Note also prior to this we call loadEssentialData, which also attempts to make use of $modSettings.

Well digging back further I locate this call:

		return throw_error(sprintf(Lang::$txt['error_sourcefile_missing'], 'Db/APIs/' . Config::$db_type . '.php'));

Change that over to a die and..

Unable to find the Sources/Db/APIs/mysql.php file. Please make sure it was uploaded properly, and then try again.

It can't find it because in my Settings.php, $db_type = 'mysql';

To work around this, added a helper method in DatabaseApi to determine the type based on the passed type. Choose to pass a type so we can always get back a result even for other database types.

This also does fix the seed issue since the database is now able to connect.

@jdarwood007
Copy link
Member Author

I'm on a linux os and its case sensitive:

Linux, the UNIX system, and Windows® observe different conventions for case-sensitivity in file name lookup: Linux and the UNIX system are case sensitive; Windows is not case sensitive.

As well, all I did so far is copy my 2.1 forum into a new database and directory, then uploaded the 3.0 files.

@Sesquipedalian
Copy link
Member

Sesquipedalian commented Nov 5, 2023

I haven't tested yet, but it looks good to the eye.

One request: please change the name of the method to something more descriptive, such as getClass. Generally speaking, property names should be nouns while method names should use verbs. There's no programming reason for this; it just makes it easier for humans reading the code later to understand. It's the reason why, for example, PHP offers \DateTime::getTimestamp() rather than \DateTime::timestamp().

@jdarwood007 jdarwood007 changed the title Fix the upgrader not finding the proper db type Fix the upgrader Nov 5, 2023
@jdarwood007
Copy link
Member Author

Done. I've also included some other fixes that I needed to get the ugprade.php to run.

@Sesquipedalian
Copy link
Member

Sesquipedalian commented Nov 5, 2023

Nice. I will look forward to testing as soon as I can. It might take me a little to get to it, though; this week is going to be a very busy one for me. Of course, someone else can always test and approve instead.

@jdarwood007 jdarwood007 changed the title Fix the upgrader Fix the installer/upgrader Nov 6, 2023
@jdarwood007
Copy link
Member Author

Added fixes for #7852
The autoloader needs to ignore trying to load integrations during install.

@Sesquipedalian
Copy link
Member

Conflicts need to be fixed.

Copy link
Member

@Sesquipedalian Sesquipedalian left a comment

Choose a reason for hiding this comment

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

Ran into problems due to typos when testing the installer.

After correcting the typos in my local test copy, I then received this error during step 2 of the install:

Fatal error: Uncaught TypeError: mysqli_query(): Argument #1 ($mysql) must be of type mysqli, null given in /Users/jon/Sites/temp/Sources/Db/APIs/MySQL.php:237 Stack trace: #0 /Users/jon/Sites/temp/Sources/Db/APIs/MySQL.php(237): mysqli_query(NULL, '\n\t\t\tSELECT VERS...', 0) #1 /Users/jon/Sites/temp/Sources/Db/APIs/MySQL.php(1066): SMF\Db\APIs\MySQL->query('', '\n\t\t\tSELECT VERS...', Array) #2 /Users/jon/Sites/temp/Sources/Db/APIs/MySQL.php(1933): SMF\Db\APIs\MySQL->get_version() #3 /Users/jon/Sites/temp/Sources/Db/DatabaseApi.php(334): SMF\Db\APIs\MySQL->__construct(Array) #4 /Users/jon/Sites/temp/install.php(882): SMF\Db\DatabaseApi::load(Array) #5 /Users/jon/Sites/temp/install.php(178): DatabaseSettings() #6 {main} thrown in /Users/jon/Sites/temp/Sources/Db/APIs/MySQL.php on line 237

I have not tried to investigate that error further.

other/install.php Outdated Show resolved Hide resolved
other/install.php Outdated Show resolved Hide resolved
@jdarwood007
Copy link
Member Author

jdarwood007 commented Nov 6, 2023

I can't get your error to occur. But that sounds like a mysql error occurred, most likely in the database connection since mysqli param should be the connection resource.

I'm even more certain seeing the stacktrace shows myqi_query first param is null. The connection failed but it continued. Need to investigate to ensure that the installer does not allow/try to continue if the database connection fails.

@tyrsson
Copy link
Collaborator

tyrsson commented Nov 7, 2023

Just as a FYI, I ran the install late last night.

Windows 10, php 8.2, 10.10.2-MariaDB

I did not tie it to an existing DB. Install ran without issue.

@jdarwood007
Copy link
Member Author

See my earlier note about case sensitivity. Windows won't see this issue. Only linux/unix based oses.

@tyrsson
Copy link
Collaborator

tyrsson commented Nov 7, 2023

I saw it, I was just confirming it runs in windows and providing what its running on ;)

Sources/Db/APIs/MySQL.php Outdated Show resolved Hide resolved
Sources/Db/APIs/MySQL.php Outdated Show resolved Hide resolved
Sources/Db/APIs/MySQL.php Outdated Show resolved Hide resolved
Sources/Db/APIs/MySQL.php Outdated Show resolved Hide resolved
Sources/Db/APIs/PostgreSQL.php Show resolved Hide resolved
Sources/Db/APIs/MySQL.php Outdated Show resolved Hide resolved
Copy link
Member

@Sesquipedalian Sesquipedalian left a comment

Choose a reason for hiding this comment

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

Works nicely for me. :)

@Sesquipedalian Sesquipedalian merged commit ad83575 into SimpleMachines:release-3.0 Nov 27, 2023
3 checks passed
@jdarwood007 jdarwood007 deleted the UpgradeDbType branch November 28, 2023 03:27
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.

3 participants