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.