View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 1933 | Composr alpha bug reports | General / Uncategorised | public | 2015-05-27 17:28 | 2015-06-10 02:51 |
| Reporter | Jason Verhagen | Assigned To | Chris Graham | ||
| Priority | normal | Severity | major | ||
| Status | resolved | Resolution | fixed | ||
| Summary | 1933: Problem with uninstalling addons with dependencies | ||||
| Description | Uninstalling addons with dependencies is also running the uninstall function of the dependency even if the dependency isn't selected to be uninstalled. For example, in Step 10 of the Setup Wizard, if the Gallery addon was selected to be installed and the Workflow addon was selected to be uninstalled earlier in the Setup Wizard, then the Workflow uninstallation also triggers the Gallery addon uninstall function to run. | ||||
| Tags | No tags attached. | ||||
| Attach Tags | |||||
| Sponsorship open | |||||
|
|
To add some more details here, I added some logging above the uninstall_addon call in Step 10 of the Setup Wizard and to the uninstall function of four addons that were missing tables after the Setup Wizard completed. Here is the output: Uninstalling: aggregate_types Uninstalling: authors Uninstalling: backup Uninstalling: cns_clubs Uninstalling: collaboration_zone Uninstalling: google_appengine Uninstalling: installer Uninstalling: ldap Uninstalling: msn Uninstalling: rootkit_detector Uninstalling: sms Uninstalling: ssl Uninstalling: supermember_directory Uninstalling: textbased_persistent_cacheing Uninstalling: tickets Uninstalling: ad_success Uninstalling: addon_publish Uninstalling: bankr Uninstalling: banner_click_points Uninstalling: bantr Uninstalling: better_mail Uninstalling: browser_detect Uninstalling: buildr Uninstalling: calculatr Uninstalling: calendar_from_6am Uninstalling: challengr _UNINSTALLING QUIZ HERE_ Uninstalling: charity_banners Uninstalling: cleanout Uninstalling: cleanup_repository Uninstalling: comcode_html_whitelist Uninstalling: composer Uninstalling: composr_homesite Uninstalling: composr_homesite_support_credits Uninstalling: composr_release_build Uninstalling: composr_tutorials Uninstalling: crosswordr Uninstalling: custom_ratings Uninstalling: db_schema Uninstalling: directory_protect Uninstalling: disastr Uninstalling: external_db_login _UNINSTALLING LOGIN HERE_ Uninstalling: geshi Uninstalling: giftr Uninstalling: google_similar_sites Uninstalling: group_points Uninstalling: idolisr Uninstalling: jestr Uninstalling: justgiving Uninstalling: lang_tools Uninstalling: lastfm Uninstalling: less Uninstalling: mentorr Uninstalling: multi_domain_login Uninstalling: nested_cpf_csv_lists Uninstalling: newsletter_no_members Uninstalling: performance_compile Uninstalling: purrrr Uninstalling: referrals Uninstalling: release_compile Uninstalling: shoutr Uninstalling: simplified_emails Uninstalling: static_export Uninstalling: stealr Uninstalling: stress_test Uninstalling: tester Uninstalling: testing_platform Uninstalling: theme_debug Uninstalling: transcoder Uninstalling: trickstr Uninstalling: user_simple_csv_sync Uninstalling: webdav Uninstalling: wordpress_style_page_children Uninstalling: workflows _UNINSTALLING GALLERIES HERE_ Uninstalling: xml_db_manage Uninstalling: xmppchat _UNINSTALLING CHAT HERE_ Some more examination reveals it's not really a dependency related issue, but related to overrides that are listed in the addons_files table for the addons being uninstalled. Unchecking challengr, external_db_login, workflows, and xmppchat addons earlier in the Setup Wizard also causes quiz, login, galleries, and chat addons to be partially uninstalled (files not removed, but their database tables were dropped). What I think is happening is the uninstall_addon function reads the addons_files table and can see the module overrides and initiates an uninstall of those files via the uninstall_module function. It looks like the uninstall_module function can see the uninstall method of the original module file and executes that function during the uninstallation of the override. |
|
|
Thanks for the analysis Jason. |
|
|
That fix took care of the unwanted partial uninstall, but introduced another issue. It looks like _custom module/block files that aren't overrides aren't having their uninstall() function called unless it's part of an addon_registry hook. For example, the Tester module was unselected in the Setup Wizard and the files were uninstalled and the config options removed via the config hooks. But the Tester uninstall() function wasn't called, which left behind the database tables and the privileges that were added by the Tester module. I added some more debug logging in the uninstall_addon() function to see what it was doing and I think I've got a solution. In plain words, I think the logic needed for modules and blocks in the addons2.php uninstall_addon() function would be something like: If block/module file is not in a _custom directory, run the uninstall code. Or, if the block/module file is in a _custom directory and not also in non-_custom directory (not an override), run the uninstall code. With a tweak to the addons2.php uninstall_addon() code to follow that logic, I think I have it working with this (lines 709-718): if (preg_match('#([^/]*)/?pages/modules(_custom)?/(.*)\.php#', $filename, $matches) != 0) { if ($matches[2] != '_custom' || ($matches[2] == '_custom' && !is_file(get_file_base() . '/' . str_replace('_custom/', '/', $filename)))) { uninstall_module($matches[1], $matches[3]); } } if (preg_match('#sources(_custom)?/blocks/(.*)\.php#', $filename, $matches) != 0) { if ($matches[1] != '_custom' || ($matches[1] == '_custom' && !is_file(get_file_base() . '/' . str_replace('_custom/', '/', $filename)))) { uninstall_block($matches[2]); } } That should execute the uninstall_module and uninstall_block calls if the file being uninstalled is not a non-_custom file, or if the file being uninstalled is a _custom file but not an override of an existing file. With the 70-80 addons that uninstalled during my run through of the Setup Wizard, it seems to be working. The only major problem I can foresee here is if an addon contained a _custom override of another existing addon file, and that _custom override also contains an uninstall function. If an addon contained an override file of another addon's file, and that override file contained an uninstall function, then that override uninstall function would be skipped. The uninstall (and install) functions would have to be moved to another file, like the addon_registry hook. |
|
|
I just wanted to make sure you caught the previous note. The tweak I made seems to be working and I have now been able to run completely through the installer and setup wizard a couple of times with no bail outs or stack traces, except for one when uninstalling the warnings addon...line 85 of adminzone\pages\modules\admin_cns_members.php references the warnings module and needs an "if (addon_installed('warnings'))" check added. I'll do some more installs with different install profiles and see how it reacts. I did see one other addon issue related to uninstalling zones (I'm not sure yet if it affects other addons). In the case of the Collaboration Zone, the Collaboration Zone link in the homepage auto-generated main menu isn't deleted when the zone is uninstalled. The drop-down menu links under the Collaboration Zone menu item are removed, but the main Collaboration Zone link remained in the menu. I don't know if it matters much and I haven't poked around the database too much yet to know if it affects other addons, I also noticed the collaboration icon files were deleted but the icon info remained in the theme_images database table. And, in general, uninstalling zones seems to leave behind a bunch of empty folders...but maybe that's more my OCD talking :) |
|
|
Hi Jason, I'm bogged down on finishing off our Tapatalk addon. I'll look at all the bug reports when I'm done, but if you want to make any commits they'd be welcome :). |
|
|
I committed the module and block uninstall logic tweaks and the fix for admin_cns_members.php error when warnings addon is uninstalled. I haven't looked at the zone uninstallation yet for the Collaboration Zone issue. |
|
|
Thanks Jason. Your fix is what I was intending to do in the first place, I must have been tired. "The only major problem I can foresee here is if an addon contained a _custom override of another existing addon file, and that _custom override also contains an uninstall function. If an addon contained an override file of another addon's file, and that override file contained an uninstall function, then that override uninstall function would be skipped. The uninstall (and install) functions would have to be moved to another file, like the addon_registry hook." To me that sounds a pretty unlikely event; that an addon would amend a default installation file and then need it's own uninstall for that, and that the user also wants to uninstall it. It's ambiguous anyway, because we don't know if that overrides uninstall is going to nuke the non-overridden tables too (certainly it could be written like that by the addon author if it was written as a 100% new version, rather than as an "implemented on top" kind of thing). In this kind of case people can share custom Commandr commands, temporary scripts, or actually the addon_registry solution you gave is a better solution (better to just fully install/uninstall in the addons own hook and avoid all that!). It's nice we can do this now in v10, we couldn't in v9. In some ways it is cleaner to handle all this in addon_registry hooks, but we kept the default code in modules/blocks on the principle of KISS for the basic functionality. I'll take a look at collaboration zone uninstall now. |
|
|
"the Collaboration Zone link in the homepage auto-generated main menu isn't deleted when the zone is uninstalled" Sitemap was assuming zone existed. Fixed. "I also noticed the collaboration icon files were deleted but the icon info remained in the theme_images database table." Sorted. "And, in general, uninstalling zones seems to leave behind a bunch of empty folders" Ok. This was a bit tricky. I made it delete folders that had become emptied after deleting the last file in them. But that was not enough, because of the supermember/collaboration_zone addon entanglement. So I made it delete the directories of removed zones explicitly. |
| Date Modified | Username | Field | Change |
|---|---|---|---|
| 2023-02-26 18:29 | Chris Graham | Category | General => General / Uncategorised |
