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

[5.1.0-beta1]Installation Problem #42971

Closed
jschmi102 opened this issue Mar 6, 2024 · 18 comments
Closed

[5.1.0-beta1]Installation Problem #42971

jschmi102 opened this issue Mar 6, 2024 · 18 comments

Comments

@jschmi102
Copy link

Steps to reproduce the issue

Upgrade from 5.1.0-alpha4

Expected result

Upgrade process should work w/o problems

Actual result

Install log shows sql-problem:

[2024-03-06T08:36:20+00:00 INFO 127.0.0.1 update Starting installation of new version.
2024-03-06T08:36:41+00:00 INFO 127.0.0.1 update Finalising installation.
2024-03-06T08:36:41+00:00 INFO 127.0.0.1 update Start of SQL updates.
2024-03-06T08:36:41+00:00 INFO 127.0.0.1 update The current database version (schema) is 5.1.0-2024-02-10.
2024-03-06T08:36:41+00:00 INFO 127.0.0.1 update Ran query from file 5.1.0-2024-02-24. Query text: CREATE TABLE IF NOT EXISTS #__tuf_metadata ( id int NOT NULL AUTO_INCREMEN.
2024-03-06T08:36:41+00:00 INFO 127.0.0.1 update Ran query from file 5.1.0-2024-02-24. Query text: INSERT INTO #__tuf_metadata (update_site_id, root) VALUES ((SELECT ue.`upd.
2024-03-06T08:36:41+00:00 INFO 127.0.0.1 update JInstaller: :Install: Error SQL Subquery returns more than 1 row
2024-03-06T08:36:41+00:00 INFO 127.0.0.1 update End of SQL updates - INCOMPLETE.
2024-03-06T08:36:41+00:00 WARNING 127.0.0.1 jerror JInstaller: :Install: Error SQL Subquery returns more than 1 row
2024-03-06T08:36:41+00:00 ERROR 127.0.0.1 update An error has occurred while running "installer::parseSchemaUpdates". Code: 0. Message: installer::parseSchemaUpdates finished with "false" result..
2024-03-06T08:36:41+00:00 INFO 127.0.0.1 update Cleaning up after installation.
2024-03-06T08:36:41+00:00 INFO 127.0.0.1 update Update to version 5.1.0-beta1 is complete.]

Update result
System (frontend) works fine after update
after fixing database at backend (select 'Database->maintenance') installation seems to be OK (did not see any errors).

System information (as much as possible)

php-V8.2
mySQL V8.1

Additional comments

@brianteeman
Copy link
Contributor

Did Database Maintenance report any errors before you clicked fix?

@jschmi102
Copy link
Author

jschmi102 commented Mar 6, 2024

hi,
yes - after install beta1 "CMS Joomla! 5 Content Management System." showed that there are problems with database version "5.1.0-2024-02-25 ( ‎5.1.0-beta1)"

@HLeithner
Copy link
Member

We tried to find the reason for this but the query that fails shouldn't be possible (yeah I know not possible doesn't exists).

It seems the query which inserts the new update site (which didn't exists yet) returns 2 update site ids for joomla core (type=file, extension=joomla), can you check your database if you have more then one for this combination in the #__extensions table?

@jschmi102
Copy link
Author

jschmi102 commented Mar 6, 2024

hi,
there are two entries listed in "Update sites" for Joomla CMS:
id=1: Joomla! Core -> https://update.joomla.org/core/test/list_test.xml
id=35: http://update.joomla.org/core/list.xml
looking into #__updates it shows only entry with id=1 which points to entry with id=700 of #__extensions. This id (700) has type="file" element="joomla" and has a manifest cache entry.
There is only 1 entry in #__extensions.

@jschmi102
Copy link
Author

jschmi102 commented Mar 6, 2024

hi,
had a loook at apache error log and found this:

[06-Mar-2024 09:35:19 Europe/Berlin] PHP Deprecated:  Creation of dynamic property Joomla\CMS\Updater\Update::$folder is deprecated in C:\myjoomla4\libraries\src\Updater\Update.php on line 452
[06-Mar-2024 09:36:40 Europe/Berlin] PHP Fatal error:  Uncaught Error: Class "Joomla\Filesystem\Path" not found in C:\myjoomla4\administrator\components\com_admin\script.php:2557
Stack trace:
#0 C:\myjoomla4\administrator\components\com_joomlaupdate\finalisation.php(71): JoomlaInstallerScript->deleteUnexistingFiles()
#1 C:\myjoomla4\administrator\components\com_joomlaupdate\extract.php(1963): finalizeUpdate('C:\\myjoomla4', 'C:/myjoomla4/ad...')
#2 {main}
  thrown in C:\myjoomla4\administrator\components\com_admin\script.php on line 2557

@HLeithner
Copy link
Member

@richard67 is the something wrong in the script.php in the beta1 release? btw. I think it's unreleated to the issue tuf issue here.

@richard67
Copy link
Member

@richard67 is the something wrong in the script.php in the beta1 release? btw. I think it's unreleated to the issue tuf issue here.

@HLeithner I don't see any mistake in the actual file, especially no reason for the Uncaught Error: Class "Joomla\Filesystem\Path" not found in line 2557. Maybe the file from the old version was still in use? But even there when going through the history I can't see any call to Path in that line. The use of Path was introduced with PR #40131 .

@richard67
Copy link
Member

richard67 commented Mar 6, 2024

In the script.php file in 5.1-dev (and beta 1) the change from PR #40131 is in, both the use Joomla\Filesystem\Path; from line 25 and the call to Path::clean in line 2557. So if something was missing, the line number would not be 2557 in the error message. So I have no idea where the Uncaught Error: Class "Joomla\Filesystem\Path" comes from.

@richard67
Copy link
Member

@HLeithner Hmm I think I know what the problem is. In some cases the extract.php can find the filesystem (or other) libraries, and in some cases not. That's why we have mocks for some library functions in the finalization-php file, like e.g. here for the filesystem class from the CMS: https://github.com/joomla/joomla-cms/blob/4.4-dev/administrator/components/com_joomlaupdate/finalisation.php#L87

This is missing for the filesystem class from the framework. @Hackwar has simply forgotten that when making PR #40131 .

The strange thing is that sometimes the extract.php can find the real libraries and so does not need the mocks from the restore.php, and sometimes not. I never understood why this happens. That might be the reason why for most testers there was not such an error.

But it needs to be fixed. It needs to add that namespace namespace Joomla\CMS\Filesystem with a class Path being defined if it doesn't exist yet, similar like done for the File class of the CMS filesystem library here: https://github.com/joomla/joomla-cms/blob/4.4-dev/administrator/components/com_joomlaupdate/finalisation.php#L90 => Ping @Hackwar .

@joomdonation
Copy link
Contributor

The strange thing is that sometimes the extract.php can find the real libraries and so does not need the mocks from the restore.php, and sometimes not. I never understood why this happens. That might be the reason why for most testers there was not such an error.

I don't think extract.php can find the real library. The reason is because that file is running as standalone php script, it is not running inside Joomla environment, thus the classes from Joomla libraries is not available for using.

To be more clear, at the end of extraction process, it is calling this function finalizeUpdate https://github.com/joomla/joomla-cms/blob/5.1-dev/administrator/components/com_joomlaupdate/finalisation.php#L54

From the code, it requires the file com_admin/script.php, create an instance of the class JoomlaInstallerScript and calling deleteUnexistingFiles method https://github.com/joomla/joomla-cms/blob/5.1-dev/administrator/components/com_joomlaupdate/finalisation.php#L71

As it is running in a standalone PHP script, all the code in the method deleteUnexistingFiles could not use code from Joomla libraries. If certain classes are using, we will need to define it as how we do in the file https://github.com/joomla/joomla-cms/blob/4.4-dev/administrator/components/com_joomlaupdate/finalisation.php#L90 as @richard67 pointed out.

@brianteeman
Copy link
Contributor

I don't think extract.php can find the real library. The reason is because that file is running as standalone php script, it is not running inside Joomla environment, thus the classes from Joomla libraries is not available for using.

Correct. This was done to avoid previous problems where files were being removed/replaced/edited during the update

@richard67
Copy link
Member

richard67 commented Mar 6, 2024

Maybe we should just remove that call to "Path:clean" as the mock for "Folder:delete" in the restore.php already does some cleaning? That could be easier than trying to mock that method.

Update: I remember now that the deleteUnexistingFiles is called 2 times, one time from extract.php via the finalisation.php, and later again by the UpdateModel. The latter uses the real libraries and not the mocks, and this might require to use Path::clean, while the former with the mock does that itself inside the Folder::delete mock.

@joomdonation
Copy link
Contributor

@richard67 You are right that the method is called two times. Personal, I think Path::clean could be removed because paths to all folders are provide by us and it's clean already https://github.com/joomla/joomla-cms/blob/5.0-dev/administrator/components/com_admin/script.php#L2199-L2431. The is_file check earlier also does not have Path::clean applied https://github.com/joomla/joomla-cms/blob/5.0-dev/administrator/components/com_admin/script.php#L2437

But if we want to be safe, better revert the change made to that file

@HLeithner
Copy link
Member

Adding the Path::clean blindly doesn't make sense... in this case I think we can simple remove Path::clean the code the source is known and is part of the cms and not from an external source. as alternative extract.php or finalisation.php can include the Path class which might isn't a bad idea anyway.

the main question on this is, is it related to the tuf update, because I don't think so.

@richard67
Copy link
Member

the main question on this is, is it related to the tuf update, because I don't think so.

@HLeithner I don't think so either.

@richard67
Copy link
Member

For the SQL error when there is more than one update site for the Joomla core I will make a PR on weekend, maybe before.

For the issue with class „Path“ not being found I can make a PR on weekend, too, if nobody else is faster.

@richard67
Copy link
Member

I just see for the 2nd issue I mentioned there is already a PR: #42976 .

@richard67
Copy link
Member

For the SQL error see pull request (PR) #42988 . For the Uncaught Error: Class "Joomla\Filesystem\Path" not found error we have PR #42978 . @jschmi102 please test both.

Closing as having 2 pull requests, one for each of the issues.

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

No branches or pull requests

6 participants