Closed (won't fix)
Project:
Drupal core
Version:
5.7
Component:
update.module
Priority:
Normal
Category:
Bug report
Assigned:
Unassigned
Reporter:
Created:
21 May 2008 at 08:49 UTC
Updated:
21 Jun 2011 at 23:55 UTC
When a module is installed and never defined a schema version with hook_udpate_N, its schema version is saved as SCHEMA_INSTALLED.
However, when reinstalling it without running an uninstall, possibly because the module does not implement hook_uninstall, that schema version set at 0 means the check for the schema version in system_modules_submit returns SCHEMA_INSTALLED instead of SCHEMA_UNINSTALLED. As a consequence, the module is not added to the $new_modules array, causing its requirements not to be checked, because these are only checked within a foreach ($new_modules as $key => $module) loop.
This does not happen if the module implements hook_uninstall, in which case it can be uninstalled, and the schema is then reset to SCHEMA_UNINSTALLED instead of SCHEMA_INSTALLED.
Although the symptoms are mostly the same, this is not the same issue as http://drupal.org/node/110981, in which the same behaviour is caused by a file scanning issue.
Comments
Comment #1
gpk commentedI guess you would invoke hook_requirements() from hook_update_1()? As you say, a module with no update hooks is regarded as having schema version 0, which makes sense to me.
Strictly speaking therefore when you run a "reinstall" without uninstalling it first you are upgrading/updating.
Changing status, but maybe I've missed the point!
Comment #2
fgmI understand your point, but even if it has to be considered an upgrade/udpate, requirements should still be checked, shouldn't they ?
Comment #3
gpk commentedLooking at http://api.drupal.org/api/function/hook_requirements/6 (OK I know it's the wrong version but the documentation is better), the $phase == 'install' case is intended for "checking installation requirements", so it seems to me that currently Drupal is doing what it says on the tin.
If requirements change from one version of a module to another then the relevant update should I imagine check the new/additional requirements. And presumably the module should disable itself (if this is possible!) if the new requirements are not met! Since otherwise we will be in an inconsistent state.
I can't think of any situation where running the requirements hook automatically would be required on module upgrade. It is run in any case whenever the status page is visited.
Leaving status active in case you can think of some!
Comment #4
gpk commentedOK someone else thought of an example where requirements/dependencies needed to be checked on upgrade. Well, it bit them!
Marking as duplicate of http://drupal.org/node/228860.
Also changing the title. If you change a module's schema or requirements and don't define uninstall or update hooks then you can't blame Drupal for not realising that we are actually doing an update (or a reinstall).
Comment #5
fgmYou're really changing the subject. I don't think it was ever documented that a module that didn't actually perform anything in an update had to implement one. Is this really what we want ?
Comment #6
gpk commentedre. the subject, sorry if it's wrong now ... hook_requirement *is* invoked on reinstalls, but as you originally pointed out the module has to be uninstalled first, so I was hoping that the revised subject reflected more accurately the points you made in subsequent comments.
>I don't think it was ever documented that a module that didn't actually perform anything in an update had to implement one
Erm, I'm afraid I've slightly lost track of what the problem is that you are trying to highlight with this issue!! If it's that hook_requirements should be invoked on upgrades, that's why I marked this a duplicate of http://drupal.org/node/228860 since that issue is already highlighting the problem with changed dependencies not being checked on upgrades, so it makes sense to me to highlight the lack of checking of changed "requirements" in the same issue since it's very similar. On the other hand if you had something else in mind then if you can elaborate/clarify so we can really pinpoint the problem and maybe get it fixed ... :) Thanks!
Comment #7
dpearcefl commentedConsidering the time elapsed between now and the last comment plus the fact that D5 is no longer supported, I am closing this ticket.