Deleted modules are not removed from system table

Dave Reid - July 31, 2008 - 06:06
Project:Drupal
Version:7.x-dev
Component:base system
Category:bug report
Priority:normal
Assigned:Dave Reid
Status:duplicate
Description

Steps to reproduce:

  • Install a module in sites/all/modules/modulename
  • Enable modulename in Drupal
  • Confirm that there is an record in the system table for modulename
    "SELECT COUNT(*) FROM {system} WHERE name = 'sites/all/modules/modulename'" == 1
  • Disable/uninstall modulename in Drupal
  • Delete the folder modulename from sites/all/modules
  • Perform an action that will call module_rebuild_cache (e.g. visit 'admin/build/modules' or run update.php)

Expected behavior:

The modulename record should no longer exist the the system table since it no longer exists in the filesystem.
"SELECT COUNT(*) FROM {system} WHERE name = 'sites/all/modules/modulename'" == 0

Actual behavior:

The modulename record persists even though the actual module no longer does.
"SELECT COUNT(*) FROM {system} WHERE name = 'sites/all/modules/modulename'" == 0

Proposed corrective action:

The problem lies in the function module_rebuild_cache. It's very similar to it's sister-theme-function, system_theme_data. Both module_rebuild_cache and system_theme_data first scan the file system for module or themes, then grab all the relevant details for each of those files from the system table. But here's where the two functions differ: system_theme_data then deletes all the theme records in the system table and rebuilds it; module_rebuild_cache only performs updates and inserts, leaving orphaned module records.

As I've been studying the code, there are two solutions. I'm not sure which approach would be best (especially considering performance).
1. Change module_rebuild_cache to emulate the delete-rebuild method in system_theme_data.
2. Change system_theme_data to delete records that are not found in the file system and change system_theme_data only insert/update records.

Will post patch shortly...

#1

Dave Reid - July 31, 2008 - 10:26
Assigned to:Anonymous» Dave Reid
Status:active» needs review

Here's my patch for solution #1. Needs to be reviewed to make sure it has the intended effect on the system table.
Edit: patch removed

#2

Dave Reid - July 31, 2008 - 10:25

Ok so first patch didn't remember module weights. I created this patch against 6.x CVS and works for me now.

AttachmentSize
289369.patch 2.96 KB

#3

Damien Tournoud - July 31, 2008 - 11:36
Version:6.x-dev» 7.x-dev
Status:needs review» needs work

It would be vastly superior to DELETE only files that are not found, rather than to introduce yet another broken DELETE-ALL-then-INSERT pattern. Oh, and please fix system_theme_data(), while you are at it.

This issue is valid for 7.x, so bumping version number.

#4

Dave Reid - September 8, 2008 - 04:00
Status:needs work» duplicate

Marking this a duplicate of #147000: Regression: Unify and rewrite module_rebuild_cache() and system_theme_data() where I've concentrated my issues for fixing this issue. Please help test the patch in that issue!

 
 

Drupal is a registered trademark of Dries Buytaert.