Joomla! Issue Tracker - CMS

Download
Launch
Feeling Lucky

[#43717] - [5.1] Add update channel reset to Joomla Update Component

Language Change Release Blocker PR-5.1-dev Pending

User tests: Successful: Unsuccessful:

avatar richard67
richard67
28 Jun 2024

Pull Request for Issue #43591 .

Summary of Changes

This pull request (PR) adds a function to the Joomla Update Component to automatically reset the update source (update channel) to "Default" in the component's options at the end of each CMS core update when it was set to "Joomla Next" before the update.

The reset is done for both the backend and the CLI core update.

The user is notified about the reset, and it is logged in the Joomla Update log file.

If an error happens with the reset, a warning is shown to the user and logged in the Joomla Update log file, but the update is still considered as successful if that was the only error during the update.

There is one difference between the update in the Administrator and the CLI:

  • The Administrator uses the updated code AFTER the update, so it does not need to have this PR applied before the update. The reset will happen when necessary with the updated code of the update model.
  • The CLI uses still the old code so the reset does not happen when updating to 5.1.2 with the CLI, but it will happen with future updates with the CLI when necessary.

The reason for the reset is that with the change to TUF-based updates in 5.1.0, the update channels are handled differently to how it was done before (and still is done with 4.4).

  • With the old XML update sites we could have a particular update several times in different channels so there was no need to switch back.
  • With TUF the update site URL is the same for the two update sources „Default“ and „Joomla Next“, and they are mapped to channels with JVERSION . '.x' for the "Default" and JVERSION+1 . '.x' for "Joomla Next".
    So when you update e.g. from 5.1 to a 6.0 nightly build, "Default" is mapped to channel "5.x" and "Joomla Next" is mapped to channel "6.x" before the update, but after the update "Default" is mapped to channel "6.x" and "Joomla Next" is mapped to channel "7.x".

And when your update source was "Joomla Next" before the update you can only have come from the previous major version. E.g. when you have updated to 5.1 with "Joomla Next" you for sure came from 4.4, and when you have updated to 6.0 with "Joomla Next" you for sure came from the latest 5.x (currently 5.2. but will be 5.4 at the end of the 5.x cycle).

Therefore we can reset from "Joomla Next" to "Default" after every update, there is no need for additional version checks.

The language string COM_JOOMLAUPDATE_VIEW_DEFAULT_UPDATES_INFO_NEXT is changed so it doesn't tell about updates for the current major version, which was wrong since the introduction of the TUF-based updates.

Testing Instructions

Test 1: Administrator Live Update - Successful Reset

  1. On a current 5.1 nightly build or the current 5.1-dev branch or a 5.1.1 without the changes from this PR applied, edit file administrator/components/com_joomlaupdate/src/Model/UpdateModel.php and change line 115 from
    $updateURL = 'https://update.joomla.org/cms/';
    to
    $updateURL = 'https://artifacts.joomla.org/drone/joomla/joomla-cms/5.1-dev/43717/downloads/77094/pr_list.xml';
    so the update site is the one created by Drone for this PR.
  2. Go to the Joomla Update Component's settings and change the update source to "Joomla Next".
  3. If no update is found yet, use the "Check for Updates" button.
  4. Perform the update.
  5. Check the result.
  6. Check the update source in Joomla Update Component's settings.

Test 2: Administrator Upload & Update - Successful Reset

  1. On a current 5.1 nightly build or the current 5.1-dev branch or a 5.1.1 without the changes from this PR applied, go to the Joomla Update Component's settings and change the update source to "Joomla Next".
  2. Update the CMS core by uploading in the Joomla Update Component the patched zip package created by Drone for this PR.
    You can download that package here: https://artifacts.joomla.org/drone/joomla/joomla-cms/5.1-dev/43717/downloads/77094/Joomla_5.1.2-rc2-dev+pr.43717-Development-Update_Package.zip
  3. Check the result.
  4. Check the update source in Joomla Update Component's settings.

Test 3: Administrator Live Update - Error on Reset

  1. On a current 5.1 nightly build or the current 5.1-dev branch or a 5.1.1 without the changes from this PR applied, edit file administrator/components/com_joomlaupdate/src/Model/UpdateModel.php and change line 115 from
    $updateURL = 'https://update.joomla.org/cms/';
    to
    $updateURL = 'https://test5.richard-fath.de/pr-43717-test-error_list.xml';
    This makes the update site point to a modified update package which provokes a database error at the reset of the update source. Besides that the package is equal to the one created by Drone for this PR.
  2. Go to the Joomla Update Component's settings and change the update source to "Joomla Next".
  3. If no update is found yet, use the "Check for Updates" button.
  4. Perform the update.
  5. Check the result.
  6. Check the update source in Joomla Update Component's settings.

Test 4: Administrator Upload & Update - Error on Reset

  1. On a current 5.1 nightly build or the current 5.1-dev branch or a 5.1.1 without the changes from this PR applied, go to the Joomla Update Component's settings and change the update source to "Joomla Next".
  2. Update the CMS core by uploading in the Joomla Update Component a modified update package which provokes a database error at the reset of the update source.
    You can download that package here: https://test5.richard-fath.de/Joomla_5.1.2-rc2-dev+pr.43717-Development-Update_Package_error.zip
  3. Check the result.
  4. Check the update source in Joomla Update Component's settings.

Test 5: Administrator Live Update - No Reset Needed

  1. On a current 5.1 nightly build or the current 5.1-dev branch or a 5.1.1 without the changes from this PR applied, edit file administrator/components/com_joomlaupdate/src/Model/UpdateModel.php and change line 115 from
    $updateURL = 'https://update.joomla.org/cms/';
    to
    $updateURL = 'https://artifacts.joomla.org/drone/joomla/joomla-cms/5.1-dev/43717/downloads/77094/pr_list.xml';
    so the update site is the one created by Drone for this PR.
  2. Go to the Joomla Update Component's settings and change the update source to "Default".
  3. If no update is found yet, use the "Check for Updates" button.
  4. Perform the update.
  5. Check the result.
  6. Check the update source in Joomla Update Component's settings.

Repeat the test but this time change the update source to "Custom URL" in step 2, and use the custom URL created by Drone for this PR: https://artifacts.joomla.org/drone/joomla/joomla-cms/5.1-dev/43717/downloads/77094/pr_list.xml

The result should be the same.

Test 6: Administrator Upload & Update - No Reset Needed

  1. On a current 5.1 nightly build or the current 5.1-dev branch or a 5.1.1 without the changes from this PR applied, go to the Joomla Update Component's settings and change the update source to "Default".
  2. Update the CMS core by uploading in the Joomla Update Component the patched zip package created by Drone for this PR.
    You can download that package here: https://artifacts.joomla.org/drone/joomla/joomla-cms/5.1-dev/43717/downloads/77094/Joomla_5.1.2-rc2-dev+pr.43717-Development-Update_Package.zip
  3. Check the result.
  4. Check the update source in Joomla Update Component's settings.

Repeat the test but this time change the update source to "Custom URL" in step 1, and use the custom URL created by Drone for this PR: https://artifacts.joomla.org/drone/joomla/joomla-cms/5.1-dev/43717/downloads/77094/pr_list.xml

The result should be the same.

Test 7: CLI - Successful Reset

  1. On a current 5.1 nightly build or the current 5.1-dev branch or a 5.1.1, apply the changes from this PR.
  2. Edit file administrator/components/com_joomlaupdate/src/Model/UpdateModel.php and change line 115 from
    $updateURL = 'https://update.joomla.org/cms/';
    to
    $updateURL = 'https://artifacts.joomla.org/drone/joomla/joomla-cms/5.1-dev/43717/downloads/77094/pr_list.xml';
    so the update site is the one created by Drone for this PR.
  3. If you have applied this PR by using the custom update URL or the patched package created by Drone for this PR, you have to edit file libraries/src/Version.php and change line 69 from
    public const EXTRA_VERSION = 'rc2-dev+pr.43717';
    to
    public const EXTRA_VERSION = 'rc2-dev';
    so that an update will be found.
    If you have used Patchtester to apply the PR you don't need to do that.
  4. Go to the Joomla Update Component's settings and change the update source to "Joomla Next".
  5. If no update is found yet, use the "Check for Updates" button.
  6. Perform the update using the CLI:
    php ./cli/joomla.php core:update`
  7. Check the result.
  8. Check the update source in Joomla Update Component's settings.

Test 8: CLI - Error on Reset

  1. On a current 5.1 nightly build or the current 5.1-dev branch or a 5.1.1, apply the changes from this PR.
  2. Edit file administrator/components/com_joomlaupdate/src/Model/UpdateModel.php and change line 115 from
    $updateURL = 'https://update.joomla.org/cms/';
    to
    $updateURL = 'https://test5.richard-fath.de/pr-43717-test-error_list.xml';
    This makes the update site point to a modified update package which provokes a database error at the reset of the update source. Besides that the package is equal to the one created by Drone for this PR.
  3. In the same file, change line 2055 from
    ->set($db->quoteName('params') . ' = :params')
    to
    ->set($db->quoteName('paramsx') . ' = :params')
    This will provoke an SQL error when trying to reset the update source.
  4. If you have applied this PR by using the custom update URL or the patched package created by Drone for this PR, you have to edit file libraries/src/Version.php and change line 69 from
    public const EXTRA_VERSION = 'rc2-dev+pr.43717';
    to
    public const EXTRA_VERSION = 'rc2-dev';
    so that an update will be found.
    If you have used Patchtester to apply the PR you don't need to do that.
  5. Go to the Joomla Update Component's settings and change the update source to "Joomla Next".
  6. If no update is found yet, use the "Check for Updates" button.
  7. Perform the update using the CLI:
    php ./cli/joomla.php core:update
  8. Check the result.
  9. Check the update source in Joomla Update Component's settings.

Test 9: CLI - No Reset Needed

  1. On a current 5.1 nightly build or the current 5.1-dev branch or a 5.1.1, apply the changes from this PR.
  2. Edit file administrator/components/com_joomlaupdate/src/Model/UpdateModel.php and change line 115 from
    $updateURL = 'https://update.joomla.org/cms/';
    to
    $updateURL = 'https://artifacts.joomla.org/drone/joomla/joomla-cms/5.1-dev/43717/downloads/77094/pr_list.xml';
    so the update site is the one created by Drone for this PR.
  3. If you have applied this PR by using the custom update URL or the patched package created by Drone for this PR, you have to edit file libraries/src/Version.php and change line 69 from
    public const EXTRA_VERSION = 'rc2-dev+pr.43717';
    to
    public const EXTRA_VERSION = 'rc2-dev';
    so that an update will be found.
    If you have used Patchtester to apply the PR you don't need to do that.
  4. Go to the Joomla Update Component's settings and change the update source to "Default".
  5. If no update is found yet, use the "Check for Updates" button.
  6. Perform the update using the CLI:
    php ./cli/joomla.php core:update
  7. Check the result.
  8. Check the update source in Joomla Update Component's settings.

Repeat the test but this time change the update source to "Custom URL" in step 4, and use the custom URL created by Drone for this PR: https://artifacts.joomla.org/drone/joomla/joomla-cms/5.1-dev/43717/downloads/77094/pr_list.xml

The result should be the same.

Test 10: Check information on "Joomla Next" update channel

With the changes from this PR applied, check the text shown about the update channel in the update component.

Actual result BEFORE applying this Pull Request

When you are still on the "Joomla Next" update channel after an update from 4.4 to 5.1, there will not be any update found for the CMS core until 6.0.0-alpha1 will be released, and then only updates to 6.0 releases will be found, so you will miss any further updates for 5.1 until that time has come.

There is no notification about that and no automatic reset after the update.

When being on the "Joomla Next" update source, the information shown about the update channel contains wrong information (see red underlined part):
update-channel-next-text-wrong

Expected result AFTER applying this Pull Request

Tests 1 and 2: Administrator - Successful Reset

When having the update source (update channel) set to "Joomla Next" in the Joomla Update Component's options before that update, the update source is rest to "Default" at the end of the update, and a success message is shown about that:
test-core-update-reset-ok

The Joomla Update log administrator/logs/joomla_update.php contains an information log about the reset:

2024-07-06T12:46:46+00:00	INFO 192.168.98.1	update	Deleting removed files and folders.
2024-07-06T12:46:51+00:00	INFO 192.168.98.1	update	The update channel has been reset from "Joomla Next" to "Default".
2024-07-06T12:46:51+00:00	INFO 192.168.98.1	update	Cleaning up after installation.
2024-07-06T12:46:52+00:00	INFO 192.168.98.1	update	Update to version 5.1.2-rc2-dev+pr.43717 is complete.

Tests 3 and 4: Administrator - Error on Reset

When having the update source (update channel) set to "Joomla Next" in the Joomla Update Component's options before that update and an error happens with reset of the update source, e.g. due to a database problem, a warning message is shown at the top, telling the user to manually reset the update source, but if there was no other error the update is still shown as successful:
test-core-update-reset-error

The Joomla Update log administrator/logs/joomla_update.php contains a warning log about the error details and another one about the failed reset:

2024-07-06T13:09:39+00:00	INFO 192.168.98.1	update	Deleting removed files and folders.
2024-07-06T13:09:46+00:00	WARNING 192.168.98.1	update	An error has occurred while running "resetUpdateSource". Code: 1054. Message: Unknown column 'paramsx' in 'field list'.
2024-07-06T13:09:46+00:00	WARNING 192.168.98.1	update	Failed to reset the update channel from "Joomla Next" to "Default". Please change it in the Joomla Update Component's options so you don't miss future updates.
2024-07-06T13:09:47+00:00	INFO 192.168.98.1	update	Cleaning up after installation.
2024-07-06T13:09:47+00:00	INFO 192.168.98.1	update	Update to version 5.1.2-rc2-dev+pr.43717 is complete.

Tests 5 and 6: Administrator - No Reset Needed

When the update source is not "Joomla Next" so it is "Default" or "Custom URL", the update source is not changed, and the update of the CMS core works as it does without this PR:
test-core-update-no-reset

The Joomla Update log administrator/logs/joomla_update.php does not show anything about reset of the update source:

2024-07-06T12:49:08+00:00	INFO 192.168.98.1	update	Deleting removed files and folders.
2024-07-06T12:49:13+00:00	INFO 192.168.98.1	update	Cleaning up after installation.
2024-07-06T12:49:13+00:00	INFO 192.168.98.1	update	Update to version 5.1.2-rc2-dev+pr.43717 is complete.

Test 7: CLI - Successful Reset

When having the update source (update channel) set to "Joomla Next" in the Joomla Update Component's options before that update, the update source is rest to "Default" at the end of the update, and a success message is shown about that:
test-core-update-reset-ok-cli

The Joomla Update log administrator/logs/joomla_update.php contains an information log about the reset:

2024-07-06T13:58:54+00:00	INFO -	update	Deleting removed files and folders.
2024-07-06T13:58:55+00:00	INFO -	update	The update channel has been reset from "Joomla Next" to "Default".

Test 8: CLI - Error on Reset

When having the update source (update channel) set to "Joomla Next" in the Joomla Update Component's options before that update and an error happens with reset of the update source, e.g. due to a database problem, a warning message is shown, telling the user to manually reset the update source, but if there was no other error the update is still shown as successful:
test-core-update-reset-error-cli
The Joomla Update log administrator/logs/joomla_update.php contains a warning log about the error details and another one about the failed reset:

2024-07-06T14:04:42+00:00	INFO -	update	Deleting removed files and folders.
2024-07-06T14:04:43+00:00	WARNING -	update	An error has occurred while running "resetUpdateSource". Code: 1054. Message: Unknown column 'paramsx' in 'field list'.
2024-07-06T14:04:43+00:00	WARNING -	update	Failed to reset the update channel from "Joomla Next" to "Default". Please change it in the Joomla Update Component's options so you don't miss future updates.

Test 9: CLI - No Reset Needed

When the update source is not "Joomla Next" so it is "Default" or "Custom URL", the update source is not changed, and the update of the CMS core works as it does without this PR:
test-core-update-no-reset-cli

The Joomla Update log administrator/logs/joomla_update.php does not show anything about reset of the update source:

2024-07-06T14:02:59+00:00	INFO -	update	Deleting removed files and folders.

Test 10: Check information on "Joomla Next" update channel

update-channel-next-text-ok

Link to documentations

Please select:

  • No documentation changes for docs.joomla.org needed

  • No documentation changes for manual.joomla.org needed

avatar richard67 richard67 - open - 28 Jun 2024
avatar richard67 richard67 - change - 28 Jun 2024
Status New ⇒ Pending
avatar joomla-cms-bot joomla-cms-bot - change - 28 Jun 2024
Category ⇒ Administration com_joomlaupdate Language & Strings
avatar richard67 richard67 - change - 28 Jun 2024
The description was changed
avatar richard67 richard67 - edited - 28 Jun 2024
avatar richard67 richard67 - change - 28 Jun 2024
The description was changed
avatar richard67 richard67 - edited - 28 Jun 2024
avatar richard67 richard67 - change - 28 Jun 2024
The description was changed
avatar richard67 richard67 - edited - 28 Jun 2024
avatar brianteeman
brianteeman - comment - 28 Jun 2024

If we can offer the updates in the same way as before then do we really need to even update the update component?

avatar richard67
richard67 - comment - 28 Jun 2024

If we can offer the updates in the same way as before then do we really need to even update the update component?

@brianteeman Like I wrote in the description:

The reset after each update provided with part 1 of the other PR or with this PR here makes still sense, except if we want to provide updates on both channels in parallel also in the future and not only this one time to fix the issue.

avatar brianteeman
brianteeman - comment - 28 Jun 2024

The reset after each update provided with part 1 of the other PR or with this PR here makes still sense, except if we want to provide updates on both channels in parallel also in the future and not only this one time to fix the issue.

that doesnt answer my question. never mind. i obviously speak a different language to everyone else and cannot express myself

avatar richard67
richard67 - comment - 28 Jun 2024

@brianteeman It does answer your question. When updates will always be provided on both channels then this PR is not necessary. But I doubt that will be the case, I think it will be only done this time. Anyway that’s not my decision.

avatar richard67
richard67 - comment - 28 Jun 2024

P.S. And as described, this PR here doesn’t include the part from the other PR which requires the extra component update.

avatar brianteeman
brianteeman - comment - 28 Jun 2024

@brianteeman It does answer your question. When updates will always be provided on both channels then this PR is not necessary. But I doubt that will be the case, I think it will be only done this time. Anyway that’s not my decision.

Why not - it has been that way for ages and if it has now been established that it can be done that way again why change. I just dont see the reason for the change.

I am on holiday now so cant check but what happens when you update from the cli

avatar richard67
richard67 - comment - 28 Jun 2024

@brianteeman Sorry, you were right, I misunderstood your question first.

Why not - it has been that way for ages and if it has now been established that it can be done that way again why change. I just dont see the reason for the change.

Yes, that would be the best. But it might require some additional work for those who deploy the updates to TUF - I am not involved in that so I don’t know if that is the way to go.

I am on holiday now so cant check but what happens when you update from the cli

I haven’t checked that either so it would be good if someone could check.

avatar brianteeman
brianteeman - comment - 30 Jun 2024

I am having some issues testing this but so far I have observed that the error page is
administrator/index.php?option=com_joomlaupdate&view=joomlaupdate&layout=complete&tmpl=component

This produces a "naked" page
image

Should the error page url not be
administrator/index.php?option=com_joomlaupdate&view=joomlaupdate&layout=complete

So that you get a fully styled page
image

avatar richard67
richard67 - comment - 30 Jun 2024

@brianteeman That in case of error the component layout is used is nothing new from my PR. It was once introduced by Fedir when he improved error reporting and logging of the update component.

avatar HLeithner
HLeithner - comment - 2 Jul 2024

to test the update with the update server please change the 115 line in
administrator/components/com_joomlaupdate/src/Model/UpdateModel.php
from
$updateURL = 'https://update.joomla.org/cms/';
to
$updateURL = 'https://update.joomla.org/alpha/';

then change the channel in the update component options form stable to next

(with stable and next channel) you should get version 5.1.2 stable offered for version 5.1.0 and 5.1.1

this might only work today till the original update server does an timestamp update and the updater might refuse an outdated / different update information depending on the status of the data cached.

avatar HLeithner HLeithner - test_item - 2 Jul 2024 - Tested successfully
avatar HLeithner
HLeithner - comment - 2 Jul 2024

I have tested this item ✅ successfully on 45e1091

works with tuf alpha update from 5.1.1

This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/43717.

avatar brianteeman brianteeman - test_item - 2 Jul 2024 - Tested unsuccessfully
avatar brianteeman
brianteeman - comment - 2 Jul 2024

I have tested this item ? unsuccessfully on 45e1091

This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/43717.

avatar brianteeman
brianteeman - comment - 2 Jul 2024

I set the channel to Next
I edited the file as per @HLeithner instructions
I performed the update.

image

avatar HLeithner
HLeithner - comment - 2 Jul 2024

which version was the installed one?

avatar brianteeman
brianteeman - comment - 2 Jul 2024

which version was the installed one?

2024-07-02T12:27:13+00:00	INFO 127.0.0.1	update	Test logging
2024-07-02T12:27:13+00:00	INFO 127.0.0.1	update	Update started by user admin (132). Old version is 5.1.2-dev+pr.43717.
2024-07-02T12:27:15+00:00	INFO 127.0.0.1	update	Downloading update file from https://artifacts.joomla.org/drone/joomla/joomla-cms/5.1-dev/43717/downloads/76941/Joomla_5.1.2-dev+pr.43717-Development-Update_Package.zip.
2024-07-02T12:27:21+00:00	INFO 127.0.0.1	update	File Joomla_5.1.2-dev+pr.43717-Development-Update_Package.zip downloaded.
2024-07-02T12:27:21+00:00	INFO 127.0.0.1	update	Starting installation of new version.
2024-07-02T12:27:39+00:00	INFO 127.0.0.1	update	Finalising installation.
2024-07-02T12:27:39+00:00	INFO 127.0.0.1	update	Start of SQL updates.
2024-07-02T12:27:39+00:00	INFO 127.0.0.1	update	The current database version (schema) is 5.1.1-2024-04-18.
2024-07-02T12:27:39+00:00	INFO 127.0.0.1	update	End of SQL updates.
2024-07-02T12:27:39+00:00	INFO 127.0.0.1	update	Uninstalling extensions
2024-07-02T12:27:39+00:00	INFO 127.0.0.1	update	Deleting removed files and folders.
2024-07-02T12:27:40+00:00	INFO 127.0.0.1	update	Cleaning up after installation.
2024-07-02T12:27:40+00:00	INFO 127.0.0.1	update	Update to version 5.1.2-dev+pr.43717 is complete.
avatar HLeithner
HLeithner - comment - 2 Jul 2024

strange are you sure this wasn't an error message form before?

avatar brianteeman
brianteeman - comment - 2 Jul 2024

I do however ask again that as you have demonstrated that the Joomla Next channel can show the next joomla 5 release why do we need to have this PR to change to the Stable channel AND if production insists that this change must be done then it is incomplete as the language string below will now give incorrect information

COM_JOOMLAUPDATE_VIEW_DEFAULT_UPDATES_INFO_NEXT="You are on the "%s" update channel. Through this channel you'll receive notifications for all updates of the current Joomla release (5.x) and you will also be notified when the future major release (6.x) will be available. Before upgrading to 6.x you'll need to assess its compatibility with your environment."

avatar brianteeman
brianteeman - comment - 2 Jul 2024

strange are you sure this wasn't an error message form before?

I am sure that the joomla update component was not displaying any error messages before I did the update

avatar richard67
richard67 - comment - 2 Jul 2024

@brianteeman According to Harald’s instructions you fist have to change the URL in the PHP file and then change the channel from Default to Next. You have done it in the opposite order.

avatar brianteeman
brianteeman - comment - 2 Jul 2024
chrome_Nem5QRzoux.mp4
avatar brianteeman
brianteeman - comment - 2 Jul 2024

@brianteeman According to Harald’s instructions you fist have to change the URL in the PHP file and then change the channel from Default to Next. You have done it in the opposite order.

i cant see how that can have any impact at all - and anyway that is how the second test was done.

avatar HLeithner
HLeithner - comment - 2 Jul 2024

ah ok that's expected and is a warning provided by tuf, after the update it checks the "main" update server which only provides an older version and it doesn't allow a downdate.

avatar brianteeman
brianteeman - comment - 2 Jul 2024

I am on holiday now so cant check but what happens when you update from the cli

I haven’t checked that either so it would be good if someone could check.

the channel is not changed from next to default when the cli is used to update

Code_6ZWlLjnCmK.mp4
avatar richard67
richard67 - comment - 2 Jul 2024

Yes, for the CLI it needs to add that, too. But I can’t do that before weekend.

avatar brianteeman
brianteeman - comment - 2 Jul 2024

The update logs show that the final two steps of the update are not completed when using the cli

2024-07-02T12:42:31+00:00	INFO 127.0.0.1	update	The current database version (schema) is 5.1.1-2024-04-18.
2024-07-02T12:42:31+00:00	INFO 127.0.0.1	update	End of SQL updates.
2024-07-02T12:42:31+00:00	INFO 127.0.0.1	update	Uninstalling extensions
2024-07-02T12:42:31+00:00	INFO 127.0.0.1	update	Deleting removed files and folders.
2024-07-02T12:42:31+00:00	INFO 127.0.0.1	update	Cleaning up after installation.
2024-07-02T12:42:31+00:00	INFO 127.0.0.1	update	Update to version 5.1.2-dev+pr.43717 is complete.

CLI

2024-07-02T12:48:50+00:00	INFO -	update	The current database version (schema) is 5.1.1-2024-04-18.
2024-07-02T12:48:50+00:00	INFO -	update	End of SQL updates.
2024-07-02T12:48:50+00:00	INFO -	update	Uninstalling extensions
2024-07-02T12:48:50+00:00	INFO -	update	Deleting removed files and folders.
avatar brianteeman
brianteeman - comment - 2 Jul 2024

Yes, for the CLI it needs to add that, too. But I can’t do that before weekend.

i still do not see why any of this is needed

avatar HLeithner
HLeithner - comment - 2 Jul 2024

Yes, for the CLI it needs to add that, too. But I can’t do that before weekend.

i still do not see why any of this is needed

because it makes no sense, if you update from 4.4 to 5.4 it needs to be a decision by you that you want to update to 6.0 and doesn't do this by accident. Also thinking of the future with auto updates this is something we have to consider if we like to automatically update 6.4 to 7.0 oder keep it at 6.4 series till it's eol or the user manually switch to next.

avatar richard67
richard67 - comment - 2 Jul 2024

i still do not see why any of this is needed

Well that’s a matter of the deployment process for TUF updates, how complicated it is to deploy the same update for both channels not only this one time but also in future. I am not familiar with that process. Maybe @HLeithner can answer that.

avatar brianteeman
brianteeman - comment - 2 Jul 2024

because it makes no sense, if you update from 4.4 to 5.4 it needs to be a decision by you that you want to update to 6.0 and doesn't do this by accident.

If it makes no sense then why has it always been this way?

avatar brianteeman
brianteeman - comment - 2 Jul 2024

But if you insist that its wrong and that the arbitrary change that was made without a PR or open discussion is correct then this PR only partially fixes the consequences of that change. This is why we have pull requests so that changes can be properly reviewed and not made silently in this way

avatar HLeithner
HLeithner - comment - 2 Jul 2024

because it makes no sense, if you update from 4.4 to 5.4 it needs to be a decision by you that you want to update to 6.0 and doesn't do this by accident.

If it makes no sense then why has it always been this way?

Don't know wasn't here at the time of introduction.

avatar HLeithner
HLeithner - comment - 2 Jul 2024

But if you insist that its wrong and that the arbitrary change that was made without a PR or open discussion is correct then this PR only partially fixes the consequences of that change. This is why we have pull requests so that changes can be properly reviewed and not made silently in this way

Our Tuf implementation can't handle 2 channels at this time (channel needs to be a regex for example).

avatar brianteeman
brianteeman - comment - 2 Jul 2024

So because of a limitation in the implementation of TUF we have a breaking change that was made without discussion or documentation and was not complete. Seems its one rule for some people's pull requests and something completely different for others. Clearly there are two tiers of contributors. Those how do what they want and those who have to jump through hoops to fix a bug. I have noted that and won't waste my time any further

avatar HLeithner
HLeithner - comment - 2 Jul 2024

stop blaming me thanks wasn't my fault or decision, I'm just finding a work around.

avatar LadySolveig
LadySolveig - comment - 2 Jul 2024

As I understand it, the "Next" update channel should primarily be a security mechanism to prevent accidental updates to the next major version.
All the time we had an "opt-out", so the user was responsible for remembering to reset the channel manually after the major update.
Otherwise, the additional security of the "Next" update channel would no longer apply in future anyway.

Regardless of whether it can now be implemented with TUF or not - does the "Next" channel even make sense in this scenario?
What am I missing here?
If I have understood you correctly, you would like to stay with the opt-out @brianteeman

In the long run, and considering that TUF was supposed to pave the way for secure automatic updates, this could actually lead to unwanted major updates at some point.

Maybe I just can't see the disadvantage of switching between opt-in and the previous opt-out.
Would you like to briefly explain what disadvantage you see?

avatar brianteeman
brianteeman - comment - 2 Jul 2024

stop blaming me thanks wasn't my fault or decision, I'm just finding a work around.

at no point have i said it was you or your decision. My entire point is that we should not need this "work around"

avatar alikon
alikon - comment - 2 Jul 2024

for what it worths, the TUF has been merged , too much "fast and happily", despite the warnings from some, in the other hand it is a good improvment, so let's fix it, and do a step in the future

avatar brianteeman
brianteeman - comment - 2 Jul 2024

for what it worths, the TUF has been merged , too much "fast and happily", despite the warnings from some, in the other hand it is a good improvment, so let's fix it, and do a step in the future

cant fix what we dont know

avatar richard67 richard67 - change - 6 Jul 2024
Labels Added: Language Change Release Blocker PR-5.1-dev
avatar joomla-cms-bot joomla-cms-bot - change - 6 Jul 2024
Category Administration com_joomlaupdate Language & Strings ⇒ Administration com_joomlaupdate Language & Strings Libraries
avatar richard67
richard67 - comment - 6 Jul 2024

I've just added the update source reset for the CLI update and will provide testing instructions and screenshots with expected results soon.

However, I think about making a further change.

Currently both the "normal" update as well as the CLI update use the collectError method of the update model when some database error happens so the update source reset fails to make sure that the reason for the error is logged in the log file and later shown with the result. The result then says the update has finished with errors and one should check the log file.

But when that was the only error, the update did not really fail. So the user could be confused and think his site is broken when it not really is.

So maybe I should change that to not using the collectError method so the user can see the update was successful, and only the warning message is shown in that case which tells what to do, and maybe I find a way to log a warning with details in the log file.

avatar richard67 richard67 - change - 6 Jul 2024
The description was changed
avatar richard67 richard67 - edited - 6 Jul 2024
avatar richard67 richard67 - change - 6 Jul 2024
The description was changed
avatar richard67 richard67 - edited - 6 Jul 2024
avatar richard67
richard67 - comment - 6 Jul 2024

I will provide an update later. Changing to draft meanwhile.

avatar richard67 richard67 - change - 6 Jul 2024
The description was changed
avatar richard67 richard67 - edited - 6 Jul 2024
avatar richard67 richard67 - change - 6 Jul 2024
The description was changed
avatar richard67 richard67 - edited - 6 Jul 2024
avatar richard67 richard67 - change - 6 Jul 2024
The description was changed
avatar richard67 richard67 - edited - 6 Jul 2024
avatar richard67 richard67 - change - 6 Jul 2024
The description was changed
avatar richard67 richard67 - edited - 6 Jul 2024
avatar richard67 richard67 - change - 6 Jul 2024
The description was changed
avatar richard67 richard67 - edited - 6 Jul 2024
avatar richard67 richard67 - change - 6 Jul 2024
The description was changed
avatar richard67 richard67 - edited - 6 Jul 2024
avatar richard67 richard67 - change - 6 Jul 2024
The description was changed
avatar richard67 richard67 - edited - 6 Jul 2024
avatar richard67
richard67 - comment - 6 Jul 2024

Ready for testing again.

avatar richard67 richard67 - change - 6 Jul 2024
The description was changed
avatar richard67 richard67 - edited - 6 Jul 2024
avatar brianteeman
brianteeman - comment - 6 Jul 2024

Without an additional change to COM_JOOMLAUPDATE_VIEW_DEFAULT_UPDATES_INFO_NEXT this PR is incomplete

avatar richard67 richard67 - change - 6 Jul 2024
The description was changed
avatar richard67 richard67 - edited - 6 Jul 2024
avatar richard67 richard67 - change - 6 Jul 2024
The description was changed
avatar richard67 richard67 - edited - 6 Jul 2024
avatar richard67
richard67 - comment - 6 Jul 2024

Without an additional change to COM_JOOMLAUPDATE_VIEW_DEFAULT_UPDATES_INFO_NEXT this PR is incomplete

@brianteeman Done. Wrong information removed from the language string.

avatar richard67
richard67 - comment - 6 Jul 2024

Hmm, in the backend the warning which is shown when the reset fails has the right colour, but it maybe has the wrong icon (check mark).

@brianteeman Should it be something else? Do you know which one?

Update: I just see when there is an other error it uses the same check mark icon, class="icon-check-circle":
https://github.com/joomla/joomla-cms/pull/43717/files#diff-04325b3920b8a824d5085b5e5dd8e85cc8e7e5110355829ca178c417e48b860cR44
... so if that should be fixed it might be something for another PR.

avatar richard67 richard67 - change - 6 Jul 2024
The description was changed
avatar richard67 richard67 - edited - 6 Jul 2024
avatar richard67 richard67 - change - 6 Jul 2024
The description was changed
avatar richard67 richard67 - edited - 6 Jul 2024
avatar richard67 richard67 - change - 6 Jul 2024
The description was changed
avatar richard67 richard67 - edited - 6 Jul 2024
avatar richard67
richard67 - comment - 7 Jul 2024

@brianteeman Could you check @tecpromotion 's suggested change on the language string? Would be good to have a native speaker instead of 2 Germans ?

avatar brianteeman
brianteeman - comment - 7 Jul 2024

@brianteeman Could you check @tecpromotion 's suggested change on the language string? Would be good to have a native speaker instead of 2 Germans ?

done

avatar richard67
richard67 - comment - 7 Jul 2024

Thanks

avatar brianteeman
brianteeman - comment - 7 Jul 2024

The more I look at this the more I see it as the wrong thing to do.

avatar richard67
richard67 - comment - 7 Jul 2024

The more I look at this the more I see it as the wrong thing to do.

@brianteeman I understand your point.

To all readers: Personally I would also prefer if things could work like in past before the change to TUF so it would not need this PR at all.

I was asked by other maintainers if I can make a pull request like this one now, and that's what I did. But I'm not the one to decide which way it should go.

avatar richard67 richard67 - change - 7 Jul 2024
The description was changed
avatar richard67 richard67 - edited - 7 Jul 2024
avatar richard67 richard67 - change - 7 Jul 2024
The description was changed
avatar richard67 richard67 - edited - 7 Jul 2024
avatar brianteeman
brianteeman - comment - 7 Jul 2024

The more I look at this the more I see it as the wrong thing to do.

@brianteeman I understand your point.

To all readers: Personally I would also prefer if things could work like in past before the change to TUF so it would not need this PR at all.

I was asked by other maintainers if I can make a pull request like this one now, and that's what I did. But I'm not the one to decide which way it should go.

Surprised the maintainers didnt also ask the people who forced these changes to also look at fixing it.

avatar richard67
richard67 - comment - 7 Jul 2024

Surprised the maintainers didnt also ask the people who forced these changes to also look at fixing it.

@brianteeman Sometimes people are busy with private life or job and don't have enough time when needed, so I stepped in to help.

avatar brianteeman
brianteeman - comment - 7 Jul 2024

I need to write more clearly.

This is a major issue that has a serious impact. There is no urgent need to make a release so better to spend the time to make the correct fix than settle on something we both agree is not an ideal solution.

avatar LadySolveig
LadySolveig - comment - 7 Jul 2024

Thank you @richard67 for your work!
For me, additional security through the Next Channel only makes sense if it is a conscious decision that I make as a user before the update and not months before and possibly forgotten again by the next major update. This PR ensures that no permanent workaround of the TUF updates is necessary and provides additional security for the end user.
Improvements are always welcome if they still need to be made.

avatar LadySolveig LadySolveig - change - 7 Jul 2024
Status Pending ⇒ Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 ⇒ 2024-07-07 21:45:28
Closed_By ⇒ LadySolveig
avatar LadySolveig LadySolveig - close - 7 Jul 2024
avatar LadySolveig LadySolveig - merge - 7 Jul 2024
avatar LadySolveig
LadySolveig - comment - 7 Jul 2024

Thank you @richard67 and also for suggestions, review and testing to all involved!

avatar brianteeman
brianteeman - comment - 8 Jul 2024

I have no idea at all what you are referring to when you say security

avatar brianteeman
brianteeman - comment - 8 Jul 2024

Why was this merged without a single successful test

image

Add a Comment

Login with GitHub to post a comment