Automatically install/uninstall schema

Actually this would be a perfect use for #253569: DX: Add hook_modules to act on other module status changes! Yay! Working on patch...

Here's just some functions that would show what would be changed. Not a patch.

That pseudo patch is quite nice. Anyone available to refine and re-submit?

Trying to keep all these module uninstall issues in one post...
#306027: user_modules_uninistalled() is missing
#320303: Remove module menus on uninstall
#335237: DX: Remove module filters on uninstall
#335240: DX: Remove module blocks on uninstall

Subscribing.

Adding #420124: DX: Remove module actions on uninstall.

Subscribing.

Re-titling, interesting patch.

     // Uninstall the module.
-    module_load_install($module);
+    drupal_uninstall_schema($module);
     module_invoke($module, 'uninstall');

(1) why removing the module_load_install() call?
(2) I would rather see the schema installation happen *after* we called the uninstall hook.

Sure he meant "would rather see the schema uninstallation happen *after* we called the uninstall hook". I agree that this sounds safer. Some uninstall logic may depend on the state of the tables.

The uninstall hook absolutely has to run while the tables are still there - if I need to call an API function for the uninstalled module to do clean up something in my module, the tables have to be there for that to work (say cleaning out vocabulary related tables when taxonomy module is uninstalled).

I would love to see this fixed before code freeze. It's always embarrassing explaining to people that they need to explicitly install/uninstall their tables.

About time we used the schema properly...

This patch would make it impossible to uninstall a whole bunch of modules via the Drupal user interface - since only modules which implement hook_uninstall() show up on the list.

I've leaving this RTBC because that can probably be fixed in a very simple followup, but it's critical that it be fixed if this patch goes in.

Should be easy to add. Just show any module with an uninstall hook OR a schema hook. Problem solved.

Hm, it's a good question why there is no test that catches this. I checked, and we do have a test that uninstalls modules via the UI, but it uses the aggregator module when doing so, and that module still has a hook_uninstall() even after this patch, so that's why it doesn't catch it. I suppose we could change it to use a different module that would catch this case, but it would be somewhat fragile to rely on that anyway, so maybe not worth doing.

Anyway, as was said above, this is easy to fix. The attached patch is identical to the previous one, except for the addition of this:

Index: modules/system/system.admin.inc
===================================================================
RCS file: /cvs/drupal/drupal/modules/system/system.admin.inc,v
retrieving revision 1.197
diff -u -p -r1.197 system.admin.inc
--- modules/system/system.admin.inc	26 Aug 2009 10:53:45 -0000	1.197
+++ modules/system/system.admin.inc	9 Sep 2009 03:56:17 -0000
@@ -1051,10 +1051,10 @@ function system_modules_uninstall($form_
     // Grab the module info
     $info = unserialize($module->info);
 
-    // Load the .install file, and check for an uninstall hook.
+    // Load the .install file, and check for an uninstall or schema hook.
     // If the hook exists, the module can be uninstalled.
     module_load_install($module->name);
-    if (module_hook($module->name, 'uninstall')) {
+    if (module_hook($module->name, 'uninstall') || module_hook($module->name, 'schema')) {
       $form['modules'][$module->name]['name'] = array('#markup' => $info['name'] ? $info['name'] : $module->name);
       $form['modules'][$module->name]['description'] = array('#markup' => t($info['description']));
       $options[$module->name] = '';

Pretty trivial, so still RTBC.

To see the difference between this patch and the previous one, try disabling the database logging module and then going to the uninstall page. The previous patch would not give you an option to uninstall the module via the UI (and therefore you'd be stuck with the watchdog table around forever), but this one does.

Great little improvement. Committed this to CVS HEAD. This needs to be documented in the upgrade instructions so marking this as 'code needs work'. Set to 'fixed' after it has been properly documented. Thanks!

Needs Documentation

Awesome to the max!

I know this was marked as done quite some time ago, but I notice that with the latest dev release of Drupal 7 that if a module doesn't have hook_uninstall defined it will not show up in the uninstall list or uninstall the schema, which I distinctly see discussed in this issue.

Was it rolled back? Do I need to have hook_uninstall defined even when it has no contents?

Cheers,
Deciphered.

No, it should list any modules that have hook_uninstall *or* hook_schema.

Dave,

A module I was using definitely wasn't showing up in the uninstall list when it did have hook_schema defined but not hook_uninstall.

It does appear to be working now. I did update to todays dev release (from yesterdays) between the time of creating my previous post, so that could have been the cause.

Either way, it definitely wasn't showing up and is now, so I will re-close this unless I manage to uncover anymore information.

Cheers,
Deciphered.

Hmm... #1031120: Uninstallation doesn't remove tables.

Dubble Hhhhmmmm:

This isn't fixed:
http://drupal.org/node/1031120#comment-4863198

Just now I tryed to activate User Revisions as the core tell my that it exist. But that didn't worked. I was given a PDOException:
---------------------------
PDOException: SQLSTATE[42S02]: Base table or view not found: 1146 Table 'web25518_drup1.user_revision' doesn't exist: update user_revision set vid=0 where uid=0;; Array ( ) in user_revision_install() (line 165 of /home/web25518/domains/fmedk.se/public_html/d/sites/all/modules/user_revision/user_revision.install).
---------------------------
I use Core version 7.7

Thanks / Tomas