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.0] Add exceptions to deleted file check for plugins which are uninstalled on update by script.php #41751

Conversation

richard67
Copy link
Member

Pull Request for Issue # .

Summary of Changes

Sine 5.0.0-beta1, a few more plugins are uninstalled by the uninstallExtensions method in the script.php file:

The files and folders of these plugins shall not be in the lists of files and folders to be deleted on update by the deleteUnexistingFiles method in script.php, otherwise you will get warning alerts and PHP notices at the end of the update. See PR #41065 for details.

This PR here adds the corresponding files and folders to the lists of exceptions in the "build/deleted_file_check.php" script so they are not falsely reported as to be deleted by that tool, which is used by maintainers and release managers to check for deleted files and folders and by me to maintain the lists in script.php.

In addition, it adds the files and folders from the invisible recaptcha plugin. This one is not uninstalled on update, and its files and folders shall not be deleted, but it's also not included in new releases, so the "build/deleted_file_check.php" script currently reports them as to be deleted.

Testing Instructions

Code review, or if you want to make a real test:

It needs a current 5.0-dev branch on a git clone and PHP CLI available.

  1. Download the 5.0.0-alpha4 full installation package and unpack it into a folder, e.g. folder "Joomla_5.0.0-alpha4-Alpha-Full_Package" in the "tmp" folder of your Joomla root.
  2. Download the 5.0.0-beta1 full installation package and unpack it into a folder, e.g. folder "Joomla_5.0.0-beta1-Beta-Full_Package" in the "tmp" folder of your Joomla root.
  3. In the Joomla root folder in a command shell (e.g. Bash on Linux or CMD on Windows), run the "build/deleted_file_check.php" script to compare the folder from step 1 with the folder from step 2, e.g.:
php ./build/deleted_file_check.php --from=./tmp/Joomla_5.0.0-alpha4-Alpha-Full_Package --to=./tmp/Joomla_5.0.0-beta1-Beta-Full_Package
  1. Check the output of the script on the command line.
  2. Save files "build/deleted_files.txt", "build/deleted_folders.txt" and "build/renamed_files.txt" for later comparison.
  3. Apply the changes of this PR.
  4. Repeat steps 3 to 5.
  5. Compare the files from step 5.

Actual result BEFORE applying this Pull Request

Output of the script:

There are 32 deleted files, 22 deleted folders and 0 renamed files in comparison to "./tmp/Joomla_5.0.0-alpha4-Alpha-Full_Package"

Files "build/deleted_files.txt" and "build/deleted_folders.txt" contain files and folders from the plugins mentioned in the description of this PR. "build/deleted_folders.txt" contains only such files.

File "build/renamed_files.txt" is empty as it should be.

Expected result AFTER applying this Pull Request

Output of the script:

There are 5 deleted files, 0 deleted folders and 0 renamed files in comparison to "./tmp/Joomla_5.0.0-alpha4-Alpha-Full_Package"

Files "build/deleted_files.txt" and "build/deleted_folders.txt" don't contain files and folders from the plugins mentioned in the description of this PR.

See comparison of "build/deleted_files.txt", left side with the PR applied, right side without:
2023-09-14_deleted-file-check_1

File "build/deleted_folders.txt" is empty now as it should be.

File "build/renamed_files.txt" is empty as it should be.

Link to documentations

Please select:

  • No documentation changes for docs.joomla.org needed

  • No documentation changes for manual.joomla.org needed

@wilsonge
Copy link
Contributor

So the recaptcha stuff is clearly correct. As we're migrating the scheduled task plugin stuff over do we want to keep it? I'm concerned we're going to be double triggering events between existing system plugins and new scheduled plugins? I assumed that was why we migrated the parameters over.

@richard67
Copy link
Member Author

richard67 commented Sep 14, 2023

So the recaptcha stuff is clearly correct. As we're migrating the scheduled task plugin stuff over do we want to keep it? I'm concerned we're going to be double triggering events between existing system plugins and new scheduled plugins? I assumed that was why we migrated the parameters over.

@wilsonge As my description says, and PR #41065 explains more detailed, the deletedUnexistingFiles method is called before the update method in script.php is called. This would cause the uninstallation fail, which happens in the (new in J5) uninstallExtensions method. This then uninstalls the plugins, which also removes their files. So I don't understand why you ask if we want to keep the files.

See #41065:

... the "deleteUnexistingFiles" method is not only called from function "update" in script.php but also from the "finalisation.php" file of the update component.

That means the plugin's files and folders are deleted before the function to uninstall the plugin is executed.

@wilsonge
Copy link
Contributor

I just didn't read carefully enough. All good!

@alikon
Copy link
Contributor

alikon commented Sep 14, 2023

I have tested this item ✅ successfully on 2b28535


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

@HLeithner HLeithner merged commit aea6eed into joomla:5.0-dev Sep 16, 2023
@HLeithner
Copy link
Member

thx

@richard67 richard67 deleted the 5.0-dev-deleted-file-check-exceptions-2023-09-14 branch September 16, 2023 09:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants