Updated: Comment #321

Suggested commit message: "Issue #2003966 by fubhy, swentel, alexpott, ParisLiakos, bojanz, aspilicious, beejeebus, steinmb: Fixed Disabled modules are broken beyond repair so the 'disable' functionality needs to be removed."

Background

Problem

Details

Drupal has two kinds of dependencies between modules:

1. Explicit, hard dependencies, as defined in dependencies[] - module A depends on module C.
2. Soft, conditional dependencies, as a result of references to things provided by module A in configuration owned by module B (possibly provided as a default by module C).

The second kind of dependency is the one which has caused most of the bugs with disabled modules.

For example:

Module A provides a field type plugin.

Module B (field module) references this field type plugin in a field/field instance configuration entity.

Module C provides an entity type and bundle.

Module B references the entity type and bundle in the same field plugin.

Module B does not have an explicit dependency on module A or C, but the configuration that it owns does.

Some modules like Views are able to handle these cases from the perspective of 'broken handlers'.

However modules dealing with user content (i.e. entity/field but also hard-coded references such as comment_node_statistics) are unable to guarantee data integrity. The modules that are still enabled may need to act on either their configuration entities or user data that is based on the configuration entity, but are unable to because the plugin/entity type is unavailable. When the plugin/entity type because available again, things may have moved on.

This can result in anything from php notices due to 'missing' data, to fatal errors where things have moved on significantly i.e. a module referencing serial IDs from another module that is disabled, uninstalled then reinstalled could end up referencing different entities altogether without having any way of knowing this.

At the moment modules try to deal with this in myriad different ways, from the Views concept of 'broken handlers', to field_info_system_alter() attempting to convert soft dependencies to hard dependencies dynamically.

#2080823: Create API to discover content or config entities' soft dependencies and use this to present a confirm form on module uninstall (postponed on this issue) is necessary to resolve the situation for uninstalled modules.

David Rothstein has (rightly) pointed out that the same approach in #2080823: Create API to discover content or config entities' soft dependencies and use this to present a confirm form on module uninstall can also be applied to disabled modules too, if we additionally enforce modules are uninstalled in one step (i.e. remove the ability to go from disabled -> uninstalled). However, given a critical issue to restore the disabled modules UI in a more limited way, this ends up at the same end point, just via different routes.

There are various examples given in the issue, as well as many other bugs which are less fundamental, but still numerous, hard to solve and frequent due to the inherent instability/limbo state of disabled modules, and a lack of clarity in terms of what it means.

  • Field types disappear; all field values get stale.
  • Entity types disappear; all references are unresolvable, data is lost.
  • Plugin types disappear, and along with it, all plugin IDs; all unresolvable plugin references get orphaned.
  • ...
  • Configuration, content, and data of disabled modules cannot be staged. The system is unable to know how to deal with the data, since it is impossible to serialize/export and import it.
  • The current transitional disable » uninstall state in itself means that the API of a module is not invoked when its data is deleted, which consequently means that data integrity gets broken in enabled modules.
  • Users are currently required to use the dangerous facility of disabling modules and the system makes it look as if it was a safe and sane thing to do.
  • History

    Solution in #317 and API changes

    • Completely removes the concept of disabled modules.
    • system.module.disabled config is removed.
    • module_enable() is deprecated and now does an install.
    • module_disable() is removed as it does not make sense to change this to the destructive operation of uninstalling modules.
    • All usages of module_enable() and module_disable() are removed from core.
    • module_uninstall() is deprecated but core usages are left to be converted in a followup (this is less confusing than module_enable() and module_disable()).
    • moduleHandler::enable() and moduleHandler::disable() are removed.
    • moduleHandler::install() is added.
    • The following hooks are removed:
      • hook_modules_preenable()
      • hook_enable()
      • hook_modules_enabled()
      • hook_disable()
      • hook_modules_disabled()
      • hook_modules_preinstall()
    • The following hooks are added:
      • hook_module_preuninstall()
      • hook_module_preinstall()

    The change from invoking hook_modules_preinstall() with a list of modules to invoking hook_module_preinstall() with single module is so that we can guarantee the environment that will be running when the preinstall action is fired before installing a particular module. For example if both taxonomy and forum are being installed this change means that you now know that the taxonomy will be installed when the hook fires for the forum module - which is a good thing because taxonomy is a dependency for forum.

    In order to explore the ability to disable modules the contrib module https://drupal.org/project/disable_modules has been created

    Updating contrib code

    Functions

    • module_enable(array('mymodule')) becomes Drupal::moduleHandler()->install(array('mymodule'))
    • module_disable(array('mymodule')) should be removed.
    • module_uninstall(array('mymodule')) becomes Drupal::moduleHandler()->uninstall(array('mymodule'))

    Hooks

    • hook_enable() functionality should be moved to hook_install()
    • hook_modules_enabled($modules) functionality should be moved to hook_modules_installed($modules)
    • hook_disable() functionality should be moved to hook_uninstall()
    • hook_modules_disabled($modules) functionality should be moved to hook_modules_uninstalled($modules) or hook_module_preuninstall($module) (note per module not a list)
    • hook_modules_preenable($modules) functionality should be moved to hook_modules_installed($modules) or hook_module_preinstall($module) (note per module not a list)
    • hook_modules_preinstall($modules) functionality should be moved to hook_module_preinstall($module) (note per module not a list)

    Related issues

    Follow-up issues

    Files: 
    CommentFileSizeAuthor
    #434 disabled-fix.patch1.85 KBdawehner
    FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch disabled-fix_0.patch. Unable to apply patch. See the log in the details link for more information.
    [ View ]
    #433 disabled-fix.patch947 bytesdawehner
    FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch disabled-fix.patch. Unable to apply patch. See the log in the details link for more information.
    [ View ]
    #407 1199946-407.patch199.67 KBalexpott
    PASSED: [[SimpleTest]]: [MySQL] 58,266 pass(es).
    [ View ]
    #382 1199946-382.patch192.77 KBalexpott
    PASSED: [[SimpleTest]]: [MySQL] 58,437 pass(es).
    [ View ]
    #372 1199946-372.patch192.64 KBalexpott
    FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1199946-372_0.patch. Unable to apply patch. See the log in the details link for more information.
    [ View ]
    #370 1199946-370.patch191.98 KBalexpott
    FAILED: [[SimpleTest]]: [MySQL] Failed to run tests: failed to login to test site.
    [ View ]
    #358 356-358-interdiff.txt604 bytesalexpott
    #358 1199946-358.patch193.07 KBalexpott
    PASSED: [[SimpleTest]]: [MySQL] 57,571 pass(es).
    [ View ]
    #356 1199946-356.patch192.33 KBalexpott
    FAILED: [[SimpleTest]]: [MySQL] 57,686 pass(es), 1 fail(s), and 0 exception(s).
    [ View ]
    #343 333-343-interdiff.txt2.61 KBalexpott
    #343 1199946-343.patch192.31 KBalexpott
    PASSED: [[SimpleTest]]: [MySQL] 57,549 pass(es).
    [ View ]
    #333 328-333-interdiff.txt5.04 KBalexpott
    #333 1199946-333.patch194.38 KBalexpott
    PASSED: [[SimpleTest]]: [MySQL] 57,558 pass(es).
    [ View ]
    #332 Screen Shot 2013-09-02 at 12.38.49 PM.png49.3 KBwebchick
    #332 Screen Shot 2013-09-02 at 12.40.02 PM.png49.19 KBwebchick
    #331 Screen Shot 2013-09-02 at 12.31.52 PM.png66.85 KBwebchick
    #331 Screen Shot 2013-09-02 at 12.34.31 PM.png65.58 KBwebchick
    #328 324-328-interdiff.txt12.98 KBalexpott
    #328 324-328-non-whitespace-interdiff.txt2.71 KBalexpott
    #328 1199946-328.patch192.76 KBalexpott
    PASSED: [[SimpleTest]]: [MySQL] 55,863 pass(es).
    [ View ]
    #327 1199946-do-not-test.patch3.1 KBchx
    #324 322-324-interdiff.txt12.67 KBalexpott
    #324 1199946-324.patch196.27 KBalexpott
    PASSED: [[SimpleTest]]: [MySQL] 55,991 pass(es).
    [ View ]
    #322 321-322-interdiff.txt758 bytesalexpott
    #322 1199946-322.patch200.95 KBalexpott
    PASSED: [[SimpleTest]]: [MySQL] 55,929 pass(es).
    [ View ]
    #321 320-321-interdiff.txt2.52 KBalexpott
    #321 1199946-321.patch200.96 KBalexpott
    FAILED: [[SimpleTest]]: [MySQL] Setup environment: Test cancelled by admin prior to completion.
    [ View ]
    #320 319-320-interdiff.txt1.75 KBalexpott
    #320 1199946-320.patch198.88 KBalexpott
    PASSED: [[SimpleTest]]: [MySQL] 56,239 pass(es).
    [ View ]
    #319 1199946-319.patch197.77 KBalexpott
    PASSED: [[SimpleTest]]: [MySQL] 56,166 pass(es).
    [ View ]
    #317 305-317-interdiff.txt40.88 KBalexpott
    #317 1199946-317.patch198.19 KBalexpott
    FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1199946-317.patch. Unable to apply patch. See the log in the details link for more information.
    [ View ]
    #305 1199946-305.patch193.41 KBfubhy
    PASSED: [[SimpleTest]]: [MySQL] 56,554 pass(es).
    [ View ]
    #305 interdiff.txt1.72 KBfubhy
    #303 interdiff.txt11.35 KBfubhy
    #303 1199946-303.patch193.58 KBfubhy
    PASSED: [[SimpleTest]]: [MySQL] 56,089 pass(es).
    [ View ]
    #274 1199946-274.patch187.74 KBalexpott
    PASSED: [[SimpleTest]]: [MySQL] 55,941 pass(es).
    [ View ]
    #271 268-271-interdiff.txt582 bytesalexpott
    #271 1199946-271.patch187.97 KBalexpott
    PASSED: [[SimpleTest]]: [MySQL] 56,060 pass(es).
    [ View ]
    #268 1199946-268.patch187.28 KBalexpott
    FAILED: [[SimpleTest]]: [MySQL] 55,727 pass(es), 1 fail(s), and 0 exception(s).
    [ View ]
    #266 1199946-266.patch176.28 KBsteinmb
    FAILED: [[SimpleTest]]: [MySQL] Drupal installation failed.
    [ View ]
    #262 1199946-262.patch188.82 KBfubhy
    PASSED: [[SimpleTest]]: [MySQL] 54,979 pass(es).
    [ View ]
    #259 1199946-258.patch177.25 KBfubhy
    FAILED: [[SimpleTest]]: [MySQL] 54,855 pass(es), 108 fail(s), and 8 exception(s).
    [ View ]
    #259 interdiff.txt2.27 KBfubhy
    #257 1199946-257.patch177.34 KBfubhy
    FAILED: [[SimpleTest]]: [MySQL] Drupal installation failed.
    [ View ]
    #257 interdiff.txt5.88 KBfubhy
    #251 1199946-251.patch178.67 KBfubhy
    PASSED: [[SimpleTest]]: [MySQL] 54,821 pass(es).
    [ View ]
    #242 1199946-242.patch178.52 KBfubhy
    PASSED: [[SimpleTest]]: [MySQL] 54,274 pass(es).
    [ View ]
    #240 interdiff.txt5.51 KBfubhy
    #240 1199946-240.patch178.51 KBfubhy
    FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1199946-240.patch. Unable to apply patch. See the log in the details link for more information.
    [ View ]
    #237 disabled-modules-1199946-238.patch178.54 KBfubhy
    PASSED: [[SimpleTest]]: [MySQL] 54,181 pass(es).
    [ View ]
    #84 1199946-fix-disabled-modules-alt.patch20.69 KBbojanz
    FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1199946-fix-disabled-modules-alt.patch. Unable to apply patch. See the log in the details link for more information.
    [ View ]
    #79 1199946-fix-disabled-modules.patch15.9 KBbojanz
    FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1199946-fix-disabled-modules.patch. Unable to apply patch. See the log in the details link for more information.
    [ View ]

    Comments

    So either

    • Drupal can never be upgraded after contrib modules are installed.

    or

    • Drupal must successfully upgrade while contrib modules are installed and enabled.

    Pick one.

    Good luck with that.

    EDIT: After actually reading the linked issue -- this is not a "you can never upgrade" solution. This is a "the only way to upgrade is to export, then re-import your data".

    Which is really the only way to handle a moderately complex site anyway, because it's nearly certain that at least one of the d6 modules used will not have a smooth d7 upgrade path.

    So...

    The right way to upgrade Drupal is:
    1. Export all contrib data
    2. Uninstall all contrib modules
    3. Update (or re-install) Drupal core
    4. Import all contrib data

    "It would be very tempting to change the states we support to these:

    Uninstalled
    Installed"

    I don't see how we can do that... the ability to disable a module temporarily is important when troubleshooting. Uninstalling is destructive.

    Can disable module temporarily by just moving it out of webroot. I've done that, frequently. :-)

    Also, change "disable / re-enable" to "export data / uninstall / re-install / import data".

    Does make troubleshooting more difficult, but not impossible.

    Right, my position on that issue is that we currently pretend that you can install Drupal, then when it comes time to update it to the next major release, just run update.php and come out the other side. This is only true if you have an extremely basic site, otherwise we are lying to ourselves and everyone else by trying to maintain that fallacy. You can trace the history of that issue at least back to #194310: Check / run updates of disabled modules and #80526: Core modules need their updates in their own files.

    Extremely simple sites will have extremely simple migrations, and this would also enforce that people don't run upgrades on their actual live database ever (which lots of people do with simple sites, then end up with unrecoverable breakage when they hit a bug - even the upgrade instructions recommend 'backing up' instead of actually running the upgrade on a copy). It will not be simple to implement the migrate issue but it may be a pre-requisite for this one to be resolved properly.

    However it is the same overall issue for disabling and re-enabling modules without updates even - if you have a module that doesn't do much, and especially doesn't interact wi th other modules much, then it's fine to disable it or enable it as you like, but if it's even moderately complex or dealing with dynamic things like fields, anything that affects actual content, then it quickly becomes a nightmare where we are piling hacks upon hacks to support it being in that state.

    If the module is simple, then disabling is not really going to be much more destructive than uninstalling - you might lose some configuration options you set up - but config should be exportable in Drupal 8 so even that is not going to be such a big deal.

    If it has it's own non-configuration data, or other modules and data are in the system that depend on the module, then if you just disable it with current D6/7/8 your site is in some kind of inconsistent state already. Currently we run somewhere between allowing people to shoot themselves in the foot, implementing ugly workarounds, and one-offs that prevent modules being disabled when 'other stuff in the system' depends on them (as opposed to a listed dependency in a .info file for another module). That goes back to #232327: Deleting node type leaves orphan nodes and older issues as well.

    @webchick: disabling modules when existing data in the system depends on them is often destructive at the moment. The only data we can guarantee (in reality, not the current pretence) won't be completely destroyed or irreparably out of sync is the actual configuration data that the module itself provides (if another module depends on that, then it probably registers an explicit dependency - that way works at least).

    If that's the only thing being destroyed, then it's either not a great loss, or we can do import/export of the config.

    "If the module is simple, then disabling is not really going to be much more destructive than uninstalling - you might lose some configuration options you set up - but config should be exportable in Drupal 8 so even that is not going to be such a big deal."

    No, this isn't accurate. Take Aggregator module, for example. That table has 400+ rows of data in it in Drupal.org's database. Disable it, and nothing's likely to happen since nothing else depends on it (unless Aggregator items get made entities in D8, I suppose), but uninstall it and bye-bye Drupal Planet.

    I kind of liked the system we introduced in one of those issues where basically if there are any X left (where X is some kind of shared data type other modules depend on; in this case field data iirc), don't allow uninstallation unless they're manually removed first. That's a pattern that's pretty easy to enforce throughout core and contrib, if we can get agreement on it.

    If we allowed export and re-import of aggregator feed configuration, then the aggregator case would be partly covered (and it'd be a big fat warning to the person who decides to uninstall it). If aggregator feeds become entities then taking the 'reverse dependency' approach from that field_delete_field() issue, it'd not be allowed to be disabled until something happened to those entities and fields anyway. Aggregator module is rapidly becoming the exception rather than the rule.

    It's not entirely the case that "nothing's likely to happen" on Drupal.org right now if you disable aggregator module.

    Drupal Planet will disappear, the dashboard will be broken since it displays items from that field, and many thousands of RSS feed readers (personal and people's own aggregator modules on other sites) will start getting 404s. This is a very different situation to if you install aggregator module then immediately disable it again, but we don't do any kind of warning at all to people if they're disabling modules that will hide/render unusable or destroy data (whether data they store themselves, or data stored by other modules). The position in the past has been "if you're disabling /uninstalling a module, you know what you're doing", but with most modules you can no longer make that assumption, since the module may not be in any way directly related to the feature it provides.

    So aggregator is a good case for whether we can get this right or not, but I don't agree it's a counter-example to changing the distinction to just installed/uninstalled.

    For debugging:

    Add functionality similar to http://drupal.org/project/environment_modules to devel (or even a special admin page in core).

    Basically we'd remove the disabled vs. uninstall distinction in the modules screen, but allow people to actually disable module hooks from being run and/or code loaded at all from somewhere else. That somewhere else could make it clear this should only be used as a temporary measure and that modules should not be left in that state for long on a production site.

    For update.php:

    - have module_implements() thrown an exception if it's called during update.php - this would enforce use of the _update_1234_foo_bar() functions for updates. Either that or just have it return an empty array on there.

    Another example where this is completely broken #1204820: Entity providing modules must call field_attach_delete_bundle() in hook_uninstall() - entity module provides various stuff centrally for other modules that can specify it as a dependency. However if you disable all those modules and also disable entity module, then entity_modules_uninstalled() won't get run when you disable the dependent modules.

    subscribing

    Issue tags:+Framework Initiative

    Tagging with framework, since this is primarily about resolving interdependencies.

    Subscribe. This issue will be intetesting in a world of “plugins“. What happens when the module implementing a plugin instance is unavailable ?

    Although, as catch points, fields are a painful kind of plugin, because unlike views handlers, they hold user data.

    Subscribe. Tricky problem.

    This issue and #1052692: New import API for major version upgrades (was migrate module) are part of a new trend I'm noticing. We are acknowledging that we have tried but ultimately failed at robustly providing a few services in drupal core. It doesn't feel good to remove disabled modules. It doesn't feel good to remove one-click major upgrades. But I agree that we should. The core dev team can't keep fighting the same family of critical bugs forever. They will quit in frustration. I really hope the community does not reply with "Sorry, try harder".

    This issue is wise.

    This problem can be fixed by imposing a few rules:

    1) Modules can't implement more than one of hook_field_storage_info(), hook_field_info() or hook_entity_info() (and their _alter counterparts). This means refactoring storage and fields as separate modules, and modules using these fields must declare them as dependencies.

    2) Any module which can create content (entity, field, node) must clean up content after itself. 'Cleaning up after itself' means at least marking its content as deletable without having to call a module-based hook, and at best doing the actual deed.

    3) Modules should be able to block their own disabling and uninstallation: hook_can_disable(), hook_can_uninstall(). These hooks would live in the install file, and be called while creating the disable/uninstall module forms, not during the act. These hooks would return messages explaining to the user what needs to happen before the module can be disabled or uninstalled (or TRUE, because it can be disabled/uninstalled). A 'why can't I uninstall this?' link would be added to the form, similar to what Coder does with the 'code review' link, and following this link would show the explanation.

    Thoughts? :-)

    1) Modules can't implement more than one of hook_field_storage_info(), hook_field_info() or hook_entity_info() (and their _alter counterparts).

    At the risk of making Drupal development dangerously user-friendly, may I see a patch to add this check to the status report? Something like:

      $requirements = array();
      $storages = array_merge(module_implements('field_storage_info'), module_implements('field_storage_info_alter'));
      $storages = array_combine($storages, $storages);
      $fields = array_merge(module_implements('field_info'), module_implements('field_info_alter'));
      $fields = array_combine($fields, $fields);
      $entities = array_merge(module_implements('entity_info'), module_implements('entity_info_alter'));
      $entities = array_combine($entities, $entities);
      foreach (array_intersect_key($storages, $fields) as $module) {
        $requirements[] = array(
          'title' => t('Module %module should not define both a storage_info and a field_info hook.', array('%module' => $module)),
          'severity' => REQUIREMENT_WARNING,
        );
      }
      foreach (array_intersect_key($fields, $entities) as $module) {
        $requirements[] = array(
          'title' => t('Module %module should not define both a field_info and an entity_info hook.', array('%module' => $module)),
          'severity' => REQUIREMENT_WARNING,
        );
      }
      foreach (array_intersect_key($entities, $storages) as $module) {
        $requirements[] = array(
          'title' => t('Module %module should not define both an entity_info and a storage_info hook.', array('%module' => $module)),
          'severity' => REQUIREMENT_WARNING,
        );
      }
      return $requirements;

    I was just thinking the other day (#1268974-22: For disabled node types with 'base' != 'node_content', _node_types_build() does not return data about the type.) about how to migrate toward cleaner uninstallation without breaking all sorts of existing modules. I'd love to see different uninstall policies that one could implement (e.g. hook_full_uninstall(), hook_metadata_uninstall(), etc.) with hook_uninstall() marked for deprecation. I liked the idea because the UI could be extended for finer administrative control and for certain sites I might like to filter out modules that don't implement hook_full_uninstall(), for instance.

    I went a little ways this direction by writing a tiny braindead module that shows you the orphaned fields that still reside in your database: http://drupal.org/sandbox/Mile23/1171894

    Subscribing

    How about the following compromise:

    We allow modules to be disabled, but we do not allow update.php to run in this state. The admin must 'shit or get off the pot' before running updates. This way, we get rid of the need to run updates for disabled modules. Thats the biggest problem here IMO.

    Temporary disabling of modules for debugging purposes is still allowed.

    We allow modules to be disabled, but we do not allow update.php to run in this state.

    So in order to upgrade from D7 to D8, or even from 8.1 to 8.2, it will be necessary to first remove all non-core modules and purge all associated data from the database.

    This is a lovely idea which I am sure will quickly gain widespread community support.

    I'm assuming that major version upgrades are handled by #1052692: New import API for major version upgrades (was migrate module). You mocked that approach in #3 and then seemed to support it later. Minor upgrades would require you to enable or uninstall only those modules which are disabled. There is no distinction between core and contrib there. Minor version upgrades do not require that you disable all contrib modules. See UPGRADE.txt

    catch points out a scenario where you have 3 disabled modules and want to uninstall one but can't properly cleanup the disabled module. This suggests that uninstall is another operation which can't be performed with additional modules in a disabled state.

    @pillarsdotnet - I think you have some good ideas but your sarcasm is very hard to work with. Please try to keep collaborative and positive.

    You mocked that approach in #3

    Sorry; I did correct myself. When upgrading from d6 to d7, I wound up manually importing the data anyway. As I said in #3 and you appeared to confirm in #15, that's probably the way to go. Newbies and managers who are considering Drupal will be disappointed to hear that they will have to start virtually from scratch at every major version upgrade, but it's probably better to admit failure in the beginning than to apologize for it later.

    Minor version upgrades do not require that you disable all contrib modules.

    Okay, noted; thanks.

    I think you have some good ideas but your sarcasm is very hard to work with.

    I suspect that you didn't find any humor in my dangerously user-friendly comment, either. But seriously, if I took the time to format and submit such a patch, would I be wasting my time? Or do such checks belong in Coder rather than system_status()?

    At any rate, we need to provide some kind of feedback when people break our implicit don't do that rules.

    "How about the following compromise: We allow modules to be disabled, but we do not allow update.php to run in this state."

    Is that a compromise? Between what and what? There are a number of problems, and they do not represent any kind of dichotomy for which there is a compromise. There are only solutions.

    Some of these problems are: The update/upgrade problem you mention, the fact that your site breaks if you uninstall a needed field or entity/node type, and orphaned data left behind because there's no D in Drupal CRUD.

    The obvious solution is to require dependencies between fields, entities, and the modules that use them, and require any module-generated content type (entity, field) to demolish its own data if Drupal isn't going to do it.

    For Drupal updates, disallowing update.php if there are disabled modules isn't such a bad idea. However, even uninstalling these modules will in many cases leave behind orphaned data in the database, leaving behind a trail of error-generating tables in an undefined update state.

    So please tell pillarsdotnet that it's OK to write a real patch. :-)

    The compromise is between #0 which says 'No more disabled modules' and webchick's 'We need to briefly disable code for debug purposes and keep data around in the meanwhile'.

    Yes, there are a lot of related issues here. I won't change the title of this issue because I smile every time I see it. But perhaps we could narrow the scope to 'disallow update.php when any module is disabled'. Thats a nice win, and pretty easily accomplished. IMO that does not have to wait until #1052692: New import API for major version upgrades (was migrate module) which probably won't land until we are much closer to release.

    As for your last request - anyone is welcome to write any patch they wish. system_requirements() checks do seem like a (small) piece of the solution. Now that i downscoped this issue, this isn't the best place IMO.

    So please tell pillarsdotnet that it's OK to write a real patch. :-)

    Posted: #1288276: The status page should warn when two different types of field hooks are implemented by the same module.

    I've opened #1330472: clean up entity-related data on module uninstall.

    Short version: The only solution I came up with requires us to enforce *no* other modules are disabled when an entity-providing module is uninstalled.

    #23/26:

    Minor upgrades would require you to enable or uninstall only those modules which are disabled.

    So, um ... how could this possibly work for a multisite installation where you have multiple sites running off a superset of shared modules, but each site only uses some subset of those? Enabling all modules for a given site could be extremely problematic.

    If a module has never been installed on a site there is no problem. The issue here is about (dropping support for) enabling a module, disabling it, then leaving it in that state for any period if time and expecting everything to magically work which is what core attempts now. Just having a module in the file system doesn't count as installed.

    Gotcha, thanks, that's clearer. The docs would need to make that distinction clear I think.

    Perhaps if modules can still be disabled temporarily yet remain visible (ie not just temporarily removing the module folder) there needs to be some explicit distinction on the module admin page between a module that is available but not installed vs one that is installed but disabled. Seems like a 2-checkbox job, that?

    If the module is in the filesystem but not enabled or installed, it will show up as an unchecked listing on the modules page.

    If the module is in the filesystem, was enabled at some point, is now disabled, but was not uninstalled, it will show up in the Uninstall tab of the modules page.

    Note the awkwardness of that last sentence, and behold the UX problem for disabled modules. :-)

    Yes, exactly, a big fat UX problem. Hiding a crucial part of the current module status in the Uninstall tab also strikes me as dangerous, not just the fact that there is a third "disabled" state.

    Devil's advocate: why is there a separate Uninstall tab anyway, if simply disabling a module can be potentially as dangerous as uninstalling it?

    Disabling the module isn't that dangerous any more, since it will warn you that you can't uninstall it before its content is gone.

    Modules should probably look more like themes, which can be enabled/disabled and default/not default.

    Disabled modules are broken, and it's time we gave up on them.
    The bandaids we put on Field API are not nearly enough, and there will always be more issues as modules become more complex.

    So I agree with catch. Let's only have installed / uninstalled.
    Of course, there are things to consider:

    1) Updating. There is already wide consensus for moving major core updates (between branches, such as D7 -> D8) to a Migrate-like solution.
    This means installing D8 & contrib modules, then pointing towards the old site to get the data, and convert it into a new suitable form.
    We already force contrib to write update code, this is just a new and less problematic way of doing it. And "disabled" doesn't matter in that case.

    2) Debugging
    We can all agree that it is sometimes useful to disable a module (when it can be disabled, for example a module that doesn't provide entity types or field types).
    I think the solution for that is the ability to prevent certain hooks from running.
    Basically, we'd have something like a special callback in module_hook() (I know, Inception) that would allow a module to say "nothing to see here, move along".
    Then you have something like #1387776: Move some rebuilds outside of Drupal in core or contrib, a script that scans the files, builds a list of all hooks, and allows the user to say
    "Disable just this hook." or "Disable all hooks of this module" or "Disable all hooks of this type (form_alter)" or "Disable all non-core hooks".
    The possibilities are very interesting, and far wider then what we have currently. I've also added a comment to the issue I linked with further thoughts on what is essentially a hook registry.

    Ugh, there is no way in heck we could possibly explain to 99% of Drupal's users what to do with checkboxes about "do and do not run X hook." :(

    99% of the users won't care about debugging at all.
    What I'm talking about is a developer's tool.
    Plus, it's not like our current modules page, or the disabled / uninstalled duality isn't pure, non-functional hell (uninstall is still pretty much impossible in D7 for ordinary users because of Field API and that comes down to "disabled' again).
    Our modules are barely disable-able as it is. Try telling a user to disable Commerce, or anything more complex than Aggregator.

    But let's give users a thought...
    We could have some simple version that allows the user to disable all hooks for a certain module (a column in the modules page or whatever),
    but I'm worried that we'd be back to the same level of breakage again.

    Plus, when users do debugging, they often do it after someone else told them to (a maintainer on the queue, someone on support). And in that case, having the ability to disable a certain hook would allow very precise investigation (I know that VBO can be affected by hook_form_alter, don't really care about hook_init, etc).

    EDIT: Expanded my points a bit.

    We could even have the ability to mark certain hooks as not possible to disable (hook_entity_info(), hook_field_info()).
    So that way even if the user clicks "disable", that leaves a few hooks alive, so less chance of breakage.

    Not sure if any of this makes sense, just trying to offer a way to slay the dragon.

    All hooks should go through module_implements() (not the case now).

    devel module (or even system module if bash the crocodile debugging is such a core feature) can implement hook_module_implements_alter()), there is a list of checkboxes for each module and clicking one + saving disables all their hooks from running. While any module is disabled, a hook_requirements() or similar screams bloody murder at the admin until they enable a module again. It would be possible to do this for individual hooks as well but we don't have to, to keep the same level of UI for this as there is now.

    That's easy enough to solve at least to the level of UI-only debugging easiness we have now, compared to the various issues listed here which are impossible to solve with the ability to indefinitely disable modules and expect to switch them on again months or even years later as if nothing had happened, or alternatively uninstall them after all. These have caused actual user data loss in the wild let's not forget. I can't believe this issue has been stalled since June on this.

    We came up with a couple easy first steps with decent UX in IRC:

    1. Do not let update.php be run when there are disabled modules. Inform the user that the modules should be either enabled or uninstalled before proceeding, with links.
    2. Add a warning to the status report when there are disabled modules, with a list, a caveat that leaving modules disabled may result in bad things, and links to enable or uninstall.

    Also, have a list of certain hooks that must always run, for starters hook_entity_info() and hook_field_info().
    So even if a user disables a module for debugging, parts important for the rest of the system continue running.
    And then we can kill the whole concept of "activate / inactive" fields.

    I have a slightly different perspective on this issue. I don't think the problem is with the concept of disabled modules itself, but rather with the specific way we've chosen to define them in Drupal. So before we throw out the concept entirely (or add warnings to the UI about the peculiar consequences of our current definition), maybe we should rethink the definition?

    In Drupal right now a "disabled module" means two totally different things:

    1. The module's functionality disappears from the user interface.
    2. The module's functionality disappears from the API (the code isn't loaded, its hook implementations aren't invoked, etc).

    The only problems I'm aware of are with #2, not #1. So, strawman proposal: Is it possible to get rid of #2 only? In other words, when a module is "disabled", maybe its code should still be there running in the background and managing whatever data it still needs to manage. But it would be responsible for staying out of the UI.

    The naive way to do that would just be to continue invoking all hooks, but the module itself uses statements like "if module_exists('me')" to avoid running the UI-related code that shouldn't be run. That's not great DX, though, and to be robust we probably need the module invoking the hook to make the decision sometimes also. So we'd have to think a lot more about the consequences of this, and whether it would require a cleaner separation between hooks that affect the UI and hooks that don't, etc. But I think it's worth thinking about.

    One obvious consequence would be performance. Under this plan, disabling a module wouldn't improve the performance of your site anywhere near as much as it does now, since most of the code would still be running. But I think that's fine. The best practice could be "to improve performance, uninstall all modules you aren't using." That's already the best practice now anyway to some extent (since uninstalling removes cruft from the variables table that gets loaded on every page, etc). It would just become more of a best practice going forward.

    Big points to David for creativity. I guess we would rename 'disabled' state to 'hidden'. I'll mull it for a bit.

    #42 doesn't make sense to me really. What happens if you "hide" a node access module? Its interface goes away, but it continues to affect access control? Am I misunderstanding? How would you "hide" access control without turning it off?

    Node access would definitely be something that should turn off. The goal would be that to a user of the site, the module must appear like it's not there and therefore has no effect.

    The things that have to stay would really be mostly housekeeping hooks. E.g., if your hook_node_delete() implementation removes stuff from your module's database table every time a node is deleted, that needs to keep running so that your data is always up to date for the next time the module gets turned back on.

    "Hidden" may or may not be the right term... it's tricky.

    So yeah, the way I described it originally (turning off the user interface vs. turning off the API) may have been a bit simplistic. It's not quite along those dividing lines really.

    I'd like to suggest a new (half-serious) name for this concept: "hibernation"

    When a module is put into hibernation, it only does the minimum things necessary to stay alive, so that when it wakes up later it will be happy and functional. (By contrast, what we have now is more like "zombie modules". When we disable a module, we kill it almost entirely, but then expect it will be fine when we bring it back to life later... Instead, it just feasts on our brains.)

    So, does this concept of "hibernated" modules makes sense? I think it could really work, but I'm a little worried about whether the DX tradeoff is worth it. No matter how it's implemented, it will necessarily add more things for module developers to think about. And if they don't do it right, we'll have lots of "I turned this module off but parts of it are still on" bug reports.

    Well, the real question is how to mark the hooks that are "essential" and must always run. Approaching it from this angle since that's the shortest list of hooks.
    hook_hook_info()?
    Core should do it for itself (as I said, hook_entity_info(), hook_entity_info_alter(), hook_field_info(), etc), and then contrib can do it for itself (commerce hooks for bundles, etc).

    As far as I'm concerned, if we switch to installed / uninstalled (therefore removing the "enabled" word, and going directly from installed to uninstalled without the zombie state in between), we can keep the "disabled" term using the new concept. Or does it have too much baggage?

    I'm really not convinced by this 'partially disabled' notion. It won't help with things like debugging, since parts of the module still run.

    Having encountered a problem with Commerce and fields from disabled modules yesterday, I was thinking about this problem as I wonder whether we can't take an idea from Views: the 'broken' handler. When I build a View and add a plugin or a handler that's from a contrib module, and later disable that module, the view still broadly works, but the specific handler is marked as being broken. I can remove it, or re-enable the module and in both cases it's ok.

    The main problems with disabled modules are that there are things in the database that need code to work, and the code is now absent. These are mainly: entity types, node types, field types, field widgets, and field formatters. Could we provide a 'broken' type or handler for each of these to step in when the expected code is missing?

    I'm thinking these would do things like:

    - for node types, show the node in strikethrough on the node admin page, and prevent it being edited
    - for field components, show the field as 'disabled' on entity display and forms

    @#48 by joachim:

    Could we provide a 'broken' type or handler for each of these to step in when the expe ted code is missing?

    That would be a major paradigm shift. Traditionally, Drupal handles buggy code by failing spectacularly rather than gracefully. Core developers see this as an advantage for several reasons:

    1. Crashing with an error message strongly discourages developers from ignoring the problem.
    2. It takes less code to crash than to gracefully recover.
    3. Error-checking and handling would unnecessarily slow down the bug-free code portions.

    > Traditionally, Drupal handles buggy code by failing spectacularly rather than gracefully

    A 'broken' handler doesn't mean buggy code. It means a fallback component that does pretty much nothing apart from tell the user that the actually required component is missing.

    re: 'broken' handler : that works well for views, because a view is only runtime code, that doesn't carry db data.

    These are mainly: entity types, node types, field types, field widgets, and field formatters. Could we provide a 'broken' type or handler for each of these to step in when the expected code is missing?

    "field widget" and "field formatter" are a non issue : missing widgets and formatter types are already replaced by the 'default' widget or formatter at runtime.
    For "field type", the problem is that, unlike a View, a field has to do housekeeping on actual db data.
    If a node is deleted while a field type is 'absent/disabled', there's just no way we can reliably clean the corresponding field data, and a genereic 'broken' handler won't help here.
    Same kind of argument applies to "missing entity type" , "missing node type".

    So could we disallow deleting of entities that are in that state?

    I don't think we're expecting a site with disabled modules to be fully operational, just not keeling over and usable for debugging and upgrades.

    So could we disallow deleting of entities that are in that state?

    This has been debated at length in #943772: field_delete_field() and others fail for inactive fields (that ended up doing this opposite : prevent disabling field types in use).
    Preventing deletion of stuff that include disabled field types is just not trackable - includes preventing node deletion, node type deletion...

    There was an interesting suggestion in #16 that no one followed up on:

    3) Modules should be able to block their own disabling and uninstallation: hook_can_disable(), hook_can_uninstall(). These hooks would live in the install file, and be called while creating the disable/uninstall module forms, not during the act. These hooks would return messages explaining to the user what needs to happen before the module can be disabled or uninstalled (or TRUE, because it can be disabled/uninstalled). A 'why can't I uninstall this?' link would be added to the form, similar to what Coder does with the 'code review' link, and following this link would show the explanation.

    This is something that would be extremely useful. It doesn't solve the whole problem but at least it provides a way for a module to communicate that it shouldn't be disabled. And that takes care of another consideration -- some modules CAN safely be disabled. So modules that should not be disabled invoke hook_can_disable() to refuse to allow that to happen, other modules do not and users can still disable them.

    Also I like the creativity have having a module still available while disabled but just hidden from the user in some way, but I can think of hundreds of ways the code would have to be tweaked to make sure it behaves correctly both in the enabled and disabled state, and I have no confidence I would even know how to ensure it works correctly both ways, even with a good faith effort to do so. If we think things are broken now, I can't even imagine how much more broken it would be when module maintainers try to write code that should work both ways, but doesn't.

    I fully agree with both Karen's points in #54.

    - "Modules should be able to block their own disabling and uninstallation" is already possibly with hook_system_info_alter(), that's what we did in #943772: field_delete_field() and others fail for inactive fields. Official hook_can_disable() / hook_can_uninstall() hooks would ease the process and make it an official pattern.
    [edit: we have an opportunity to introduce a hook named _disableable, let's not miss it :-p]

    - The 'active but hidden' approach sounds like an implementation/debugging hell in practice. Too many chances this is done wrong.

    > Preventing deletion of stuff that include disabled field types is just not trackable - includes preventing node deletion, node type deletion...

    It seems that the main problem is that the arrival of FieldAPI has changed the way dependent and required modules interact with one another: it's turned it upside down in a way.

    In D6 days, module B would depend on module A, and use hook_form_alter to change its form. Module A is completely ignorant of what module B is doing, and if B goes away A can keep on working as normal. There is even then a slight problem of data not being handled (eg disable taxonomy module, then delete a node with tags) but it's just database cruft and nothing you'd notice.

    D7 fields turns this on its head by making module A go and ask for its dependents with field_attach_form().

    Given we've already made that change, I don't see why we can't also require modules that use FieldAPI to first ask field_attach_is_my_form_ok() and block operations such as deletion if it gets back a FALSE.

    - "Modules should be able to block their own disabling and uninstallation" is already possibly with hook_system_info_alter(), that's what we did in #943772: field_delete_field() and others fail for inactive fields. Official hook_can_disable() / hook_can_uninstall() hooks would ease the process and make it an official pattern.
    [edit: we have an opportunity to introduce a hook named _disableable, let's not miss it :-p]

    What we did was a mistake that solved nothing. It basically tells users "You know what? Forget it, you can never uninstall".
    It was a (bad) bandaid for D7 and not something that should be thought about for D8.
    That code created circular dependencies for Commerce and required an alter hook that basically undid everything core did for Commerce modules, allowing modules to just be disabled (and then on uninstall disabled fields would get re-enabled, which would delete them but keep them in the db. So you have junk in the database but at least Commerce was uninstalled and can be installed again). See #1403972: Make uninstallation possible again if you want the full story.

    I've spent countless hours just getting something as large as Commerce to actually be uninstallable.
    And that is after helping the unfortunate #943772: field_delete_field() and others fail for inactive fields get written.

    I feel like we've lost focus here, entertaining irrelevant ideas.
    David gave us a good summary in #42.

    Here's a plan:
    1) The "disabled' concept gets removed from Field API and all other parts of core apart from the status column in {system} and a link on the Modules page to flip that. This means, for example, that modules get uninstalled while still being enabled. This is what catch originally proposed.
    2) We then, for debugging purposes, add a new "hibernation / disabled" (reuse the old word or not, I don't care) concept that disables all non-essential hooks.
    This requires hooks to be sorted into two groups. It is done by core (and contrib modules) by declaring which hooks are essential, through
    hook_hook_info() for example. That list is consulted by module_implements(), so if a module has status = 0 and the hook is not in the essential list, don't run it.

    So if a module goes into hibernation, it still provides the data structures it needs, it just disappears from the menu, the alter hooks, etc.
    The list of essential hooks should be fairly small once we stop misusing hooks in core (so field types become plugins, for example. And plugins are always provided, they are in the essential category).
    The end result of this is that a module can be enabled / disabled, and installed / uninstalled. These two are not connected. No more disable -> pray -> uninstall -> curse.

    I could even imagine the whole hibernation concept being provided by contrib (Devel). Core just needs to make sure it uses module_implements() always.

    If we only want disabled/hibernated for debugging purposes only, I don't see what's wrong with #10, if the 'essential hook' is the one that's hosing your site then you want disabling the module to switch everything off - the main thing is having it as a completely separate state to installed/uninstalled which bojanz's post nicely summarized.

    #54 won't solve the problem we have, as still you can disable modules and enable them afterwards. Consider module B storing information for an entity provided by module A. If module B is uninstalled while module A is disabled, the entity type of module A is not there and thus the data module B stores related to the entity type of module A won't be removed during uninstall - it is going to remain in the database, forever.

    That said, #10 sounds like a good solution to me.

    I don't understand why core does anything with disabled modules at all? To me, "disabled" means "completely ignore this bit of code but don't remove the data because I might still need it". If you disable a module that holds data and has interactions with other modules and continue to run the site that way for a while and the data gets out of synch, that seems more like user error than anything. I can see providing a warning on disable that you may have problems when you re-enable but, beyond that, why should core care? We can't keep users from doing stupid things in every case. If we try, they just get more creatively stupid. :)

    So my proposal is:

    • Active - Module is turned on and running normally.
    • Disabled - Module is completely ignored by Drupal as though it didn't exist. If you continue running in this state to the point that your data is hosed, it's your own dang fault. This state should be used primarily for debugging or turning off modules that aren't always needed and are safe to have disabled for long periods of time.
    • Uninstalled - Drupal removes all data belonging to the module and then ignores it.

    This doesn't address the "disable on major upgrade" bit but I think the Migrate option is a better route for that, anyway.

    Michelle

    We've been talking on IRC and I thought of an idea:

    1) Get rid of disable.

    2) Add a hook or something to let modules define what sorts of uninstall they support.

    Options are:

    1. Uninstall totally
    2. Uninstall but save configuration
    3. Uninstall but save all data

    #1 would be the normal uninstall we have now. #2 would save information along with uninstalling. If the module supports saving it would also support importing. When the module is installed again, it has the option of importing the saved off data. It's up to the module to know how to handle this data, determine if it's out of synch, stale, whatever. Core shouldn't need to worry about that (except for core modules).

    This is also very similar to the whole config in code / migrate instead of update idea and likely could use much, if not all, of the same code. So, basically, when you uninstall you do a module dump similar to if you are doing a major upgrade. That way you can turn off modules temporarily and just suck the data back in when you turn them on or use that to do an upgrade.

    That would get rid of the whole disabled modules problem completely because it only allows uninstall but it also opens the door for uninstall not necessarily meaning losing all your data.

    Michelle

    [Edit: just a typo]

    So to sum it up, it would be "put all of the module's data into a hibernation in a cave that nobody else goes into".

    There's no reason we can't call this 'disabled' in the UI, as suggested in #57.

    > and likely could use much, if not all, of the same code

    Yup, but this is something that's broken on D7 too. Or is it too late to do anything to help D7 at all?

    > Core shouldn't need to worry about that (except for core modules).

    Be nice to provide an API though. Something akin to cache_set/cache_get buckets but without any cron clean-up obviously.

    Issue tags:+Usability

    I wanted to chime in a bit, because this has a deep impact on the UX. We have been working on a module page redesign and did explore how we could allow for uninstall to have more prominence.

    I would say that the current model is not so broken, other than uninstall basically not doing much. The idea of disabling a module, and uninstalling when you really want to remove the data, to me feels solid.

    I am deeply troubled with the proposal, to basically have two states - that are to be communicated on the module page. I would prefer a workflow, that handles data removal after on/off of a module. Even though, that is still difficult from a UX perspective, it at least doesn't require two different operations/models to be explained.

    I read over in the redesign modules page that there was "building consensus" for removing the disabled state of modules. I would like to state categorically that this is a terrible, awful, no good, very bad idea that I will fight with my very last breath. For an end user's sake, there *has* to be a state called "disabled." What actually happens when that state is achieved is up to core developers, of course, but you cannot possibly explain to an end user "enabled, uninstalled, or uninstalled, with caveats."

    Before we all panic, I think we actually have an idea that's viable for everyone if we bring together the different threads:

    1. there is still a 'disabled' state, caused by unticking a module's ticky box
    2. disabling a module causes all its data to go into hibernation in a bucket.
    3. re-enabling / waking up a module requires that module to re-introduce its hibernated data into the DB in a way it knows best
    4. uninstalling a module wipes its data entirely. You can only uninstall disabled modules -- BUT! this is not because of the architecture, it's a designed UX pathway to guide the user.

    @webchick
    The point of that comment and remark was to get UX people involved while there's still no final plan or even a first patch, because UX is a big factor in how this is implemented.
    Still, I fail to see how installed / uninstalled are such an unnatural concept. The fact that there's a middle step for uninstalling is never something I found clear.

    We've just had a huge IRC discussion (catch, Bojhan, Michelle, merlinofchaos)
    Bojhan has the opinion that the flow should be disabled -> uninstalled, even if the "disabled' step is not needed or might not mean anything.
    It is still up for discussion whether that should be one step (disable & uninstall) or two. It's a matter of how the modules page is (re)designed.

    However, we still agree that only enabled modules can be uninstalled. So catch had the idea of silently re-enabling the module just prior to uninstall.
    The icky part here is hook_enable / hook_disable (and hook_modules_enabled / hook_modules_disabled). Those hooks will fire like crazy.
    catch and I agreed that it might not be a bad idea to just kill them. They are only misused in contrib anyway.

    So, plan of acton could be:
    1) Make disabling non-destructive, the idea I mentioned previously
    2) Remove hook_enable / hook_disable (and hook_modules_enabled / hook_modules_disabled)
    3) Kill the active / storage_active Field statuses. Since hook_field_info() runs for disabled modules, the field type always exists. Same for storage
    4) Make uninstall enable the module before continuing.
    I would also like batch api / queue api powered field deletion when the field type module is being uninstalled.

    What I dream of is being able to go to the Modules page, check 10 checkboxes, and click uninstall. I get a progress bar, the deed gets done, I rest.
    No more reloading the modules page 20 times, running cron, seeing what depends on what.
    Bojhan can modify that plan to satisfy the UX gods, but the point still stands.

    There would be side effects to that plan: disabling the PHP module would still keep the filter it provides, making it kinda useless.

    [EDIT: Added the plan of action]

    Of course, the problem with partial disabling where some hooks run is exactly that; some hooks run.
    So people would probably argue that hook_entity_load( ) should always run, and if there's a bug there, it will stay present even if the module is disabled.
    I don't see any way of going around that. The fact is that disabled modules are a fundamental problem at the heart of Drupal, and fixing that while keeping everyone happy will lead to a few WTFs, even if it succeeds.

    We need all disabled modules to be active to be able to properly uninstall. We cannot fake "disabled", because this would lead even more WTFs (why does it still break my site??). ouch.

    Still, silently re-enabling them for uninstall seems to be the only way out.

    re @bojanz #66:

    Kill the active / storage_active Field statuses. Since hook_field_info() runs for disabled modules

    The issue is not whether hook_field_info() will run for disabled modules. It is that hook_field_delete() must be reachable when the field data is purged - which can take an arbitrary amount of time.

    > It is that hook_field_delete() must be reachable when the field data is purged - which can take an arbitrary amount of time.

    We have a queue system in core already. Why not use that?

    Situation: module A is deleting entities, which affect a field marked as inactive and belonging to disabled module B.
    Stick an operation in the queue that says 'Hey module B, when you wake up out of hibernation (if ever), you need to perform this task'.

    Stick an operation in the queue that says 'Hey module B, when you wake up out of hibernation (if ever), you need to perform this task'

    "this task" being "do your job purging those N field values, that belonged to nodes that got deleted 5 weeks ago".
    That means we're able to preserve the actual field values somewhere and hand them out later on at a time when the nodes themselves (and possibly the node type itself) are long gone.

    The logic for maintaining that 'in between' state is really not something I'm willing to write or maintain within Field API. Just too much hair pulling and possible race conditions.
    I don't even think that the part that does "on field delete, extract the field values out of the main storage and push them somewhere out of the way but available for values purging at an arbitrary later date" is doable with Mongo field storage.

    We cannot fake "disabled", because this would lead even more WTFs (why does it still break my site??). ouch.

    Well, it's either remove the disabled state (something which webchick said she is explicitly against) or soften the problem by making the state less "disabled" :) It's a compromise I'm willing to make.

    The issue is not whether hook_field_info() will run for disabled modules. It is that hook_field_delete() must be reachable when the field data is purged - which can take an arbitrary amount of time.

    True. As I said in #66:

    I would also like batch api / queue api powered field deletion when the field type module is being uninstalled.

    This would solve it nicely.

    I would also like batch api / queue api powered field deletion when the field type module is being uninstalled. - This would solve it nicely

    Yes, it would be a better DX to haver on top of the current D7 state (field type module un-disableable until there are field data) - a D7 patch is completely possible

    but -

    No, it doesn't solve the fact that hook_field_delete() needs to be present (and therefore the module enabled) during all the time data is being purged.

    Well, we already said that we want to make uninstall work on enabled modules. That's the basic point of this issue.
    So purging happens at the beginning of uninstall, the hook is still present.

    Here's an idea which won't address everything but which I think could be useful for some of the problems we have here:

    Introduce the concept of a clean-up jobs queue, and the concept of the site being in a 'dirty' state while clean-up jobs remain outstanding.

    - any module may add jobs to the clean-up queue
    - cron runs will gradually empty the clean-up queue
    - the clean-up queue can be completely dealt with in a batch operation, similar to going to the updates page.
    - while the site is dirty, certain tasks may not be performed. These would include disabling/hibernating further modules, or enabling new modules, or certain entity operations. The admin UI would explain why these actions can't be carried, with a link to a help page and to the page to run the clean-up batch.

    In essence, this is a generalization of the situation we currently have where deleting things leaves the modules page telling you that you still have dependencies, which then silently vanish on cron runs.

    So for example, deleting an entity with inactive fields logs a clean-up job for deleting the row for (entity_type, id) in that field's table.

    to follow David_Rothstein I also have another funny one :-)
    #1458338: Media file widget causes unrecoverable error after disabling and re-enabling media module

    I think it marks out, what webchick and catch are saying in #4 and #7. I think both are right. I can agree with webchick that we should keep in the options for disabling a module since we shouldn't force users to reinstall or move folders for a short -off- time, but as catch also says: the only data we can guarantee atm after re-enabling as -kept-, is configuration.

    xjm: thanks for pointing me to this issue. But I would like to vote for keeping focus on the issue title to find a better solution for disabled modules, instead of discussing module uninstallation and reinstallation as an option for this. Why not adopting the method how configuration is kept, even if it would be for keeping at least module depending fields and its recognition afterwards? I dunno if possible, didn't took a look into it yet. ... Just another thought ...

    Title:Disabled modules are broken in every possible wayEnsure that disabled modules maintain data integrity
    Issue tags:-Usability

    Okay, time to get this moving again.

    Talked this over with merlinofchaos (and later chx as well) at DrupalCon, and we narrowed down the fix a bit, concentrating on the important point.
    This is a strictly code change, with no UI changes.

    The crux of the matter is what David said in #42, quoting:

    In Drupal right now a "disabled module" means two totally different things:

    The module's functionality disappears from the user interface.
    The module's functionality disappears from the API (the code isn't loaded, its hook implementations aren't invoked, etc).
    The only problems I'm aware of are with #2, not #1. So, strawman proposal: Is it possible to get rid of #2 only? In other words, when a module is "disabled", maybe its code should still be there running in the background and managing whatever data it still needs to manage. But it would be responsible for staying out of the UI.

    The key concept here is Data integrity. Right now, if you disable a module, all hell breaks loose because data integrity isn't maintained, field types and entity types disappear, etc.

    The proposed fix is this:
    1) All installed modules get loaded / included at bootstrap, not just enabled ones.
    2) Modules declare hooks that are essential to maintaining data integrity, by using hook_hook_info(). These hooks are hook_field_info(), hook_entity_info(), hook_entity_load(), etc. Basically, anything relating to CRUD and providing "building blocks".
    3) When invoking a hook (module_hook() / module_implements()), hooks are invoked in enabled modules. Also if the hook is declared as "essential", it is invoked in disabled modules as well.
    So, when a module is disabled, its hook_form_alter, hook_menu, hook_views_data and other "UI level" hooks don't fire, it disappears from the UI.
    However, if it implements some of the essential hooks, then those hooks still run.

    I have a half-broken patch prepared (as always, the installer is the one creating a problem. Sad, sad code).
    Gonna hack on it some more tonight, then post it.
    The issue we need to tackle first is: #1503224: Cleanup module_list(). Those cleanups are necessary if we want to do this correctly.
    A followup issue is #1503314: Remove the concept of active / inactive (field types, storage) from Field API (since it would no longer be necessary to maintain the active status, all fields and storage engines would be active).

    Removing the usability tag since there are no UX changes here (although I'd love to see us separate enabled / disabled and installed / uninstalled as concepts in the UI, eventually).

    Issue summary:View changes

    Updated issue summary.

    Status:Active» Needs review
    StatusFileSize
    new15.9 KB
    FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1199946-fix-disabled-modules.patch. Unable to apply patch. See the log in the details link for more information.
    [ View ]

    Rolled against #1503224: Cleanup module_list().
    Still broken (install fails).

    The "essential" parameter can be called something else.
    Needs some code in entity_hook_info() that registers entity type specific hooks (such as hook_node_load()) -> Can we call hook_entity_info() from hook_hook_info()?
    Also remember that some field hooks actually function as callbacks (hook_field_load, hook_field_insert, etc), so they are always called when the module is included, no need to declare them. Hoping that whole mess gets fixed when Field API moves to plugins.

    In a plugin world, I see us declaring certain plugin types as essential. So a field type would continue to exist, but a field widget doesn't need to.

    Status:Needs review» Needs work

    "Field API --> Plugins" is in progress, relying on the current (in-progress too...) state of the Plugins API.
    In that approach, it will all depend on the "Plugins discovery mechanism" whether Plugin Implementation classes (i.e. field type handlers) provided by disabled modules can be located, instanciated and have their methods invoked.

    That discovery method (who holds a registry of available plugins and how is this list collected) is very much in discussion right now - CMI, info() hooks, class annotations... See #1497366: Introduce Plugin System to Core, more specifically comments #5, #7, #12.

    Also, think the recent "PSR-0 loader for module-provided classes" registers namespaces on disabled modules right now.

    The proposed fix is this:
    1) All installed modules get loaded / included at bootstrap, not just enabled ones.
    ...

    As it is now, when I have an installed (as in placed & extracted in the file structure - not necessarily enabled) module that is known to cause issues, I simply leave it disabled in the server. Even if I can't get to the modules page to disable it, I've learned to do it straight from the db and the system table (to get around WSODs for example). I don't have to worry as to which part of the module was broken - be it the UI or API code it now simply doesn't load at all. The reason I leave it disabled is because I want to be notified of any future version updates and try to see if the problem I was having is still there simply by upgrading and (re)enabling the module.

    What does this proposed change here mean for user like me that are in this habit of keeping their problematic modules disabled, but still under /sites/all/modules? Do I have to completely remove them (as in also delete them from the file system) in order to be sure that it's not them causing any other issues?

    There's also the category of modules that serve for a single purpose (migration, import, troubleshooting etc) and that it is recommended to disable them when not needed. Until now, we used to simply disable them but still keep them around just in case we need them again. Will we now have to also make sure we deleting the actual files from the server too?

    > Do I have to completely remove them (as in also delete them from the file system) in order to be sure that it's not them causing any other issues?

    That means that either:

    a) you have to run the uninstall procedure on them. Your data is lost.

    b) you just move them out of the way and hope for the best. So our troubleshooting procedure now involves manually moving files, and we've reinvented 'disabling' a module to be more complicated and just as bad.

    Status:Needs work» Needs review
    StatusFileSize
    new20.69 KB
    FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1199946-fix-disabled-modules-alt.patch. Unable to apply patch. See the log in the details link for more information.
    [ View ]

    @yched
    Yes, I'm aware of that discussion. Doesn't affect this issue much. I'm personally for using annotations. Extending that to hooks would allow explicit registration and metadata above the hook itself, which would be awesome.

    @klonos, joachim
    The major implication is that the module file is always included, and present in the memory of each request. So you can no longer gain a performance boost by just disabling a module. Generally, it would behave the same as before, most hooks don't run, so most problems would disappear by disabling the module. There's always a slight chance that the bug is actually in one of the essential hooks (entity_info, or whatever), so there is a possibility that disabling wouldn't solve it, won't deny it. Still, it's really small.

    That said, I've been looking into solving that as well.
    Here's an alternative patch. It introduces a "hibernation" state ({system}.status == 2). When a module is being disabled, we check if it implements one of the essential hooks, and if it does, we put it into hibernation, which works as I described above (module file included, only essential hooks run). If it doesn't implement any essential hooks, it gets completely disabled and isn't included, same as before.
    Compared to my previous patch, this one actually works in my testing.

    So, I now need reviews (about everything from docs to key naming, approach taken, etc).
    1) Which approach should I continue? Hibernation seems like a less invasive change. Wondering about the terminology, and whether it is good enough.
    2) If I continue with hibernation, there's a matter of:

    +  // Certain hooks should be invoked in hibernated modules, but shouldn't
    +  // trigger hibernation themselves.
    +  // @todo Make this generic? Another key in hook_hook_info()?
    +  $ignore = array(
    +    'schema',
    +  );

    I also need help on getting #1503224: Cleanup module_list() to pass tests.

    You could also mark them as uninstalled (i.e. status = -1) in the database without actually running the uninstall hooks. I think that would achieve pretty much the same thing as disabled modules do now (the difference would be that the module's update functions don't run but that's probably more in line with what you want anyway).

    That kind of thing would not be "officially" supported any more - since as we know, for a significant number of modules the concept is broken anyway. However, I could imagine a Drush command that does it? .... So people in the know could still do old-style disabling of modules for debugging purposes, if they're willing to assume the risk.

    Oops, crosspost.

    Interesting, need to think about that some-modules-can-be-disabled-and-some-can-only-be-hibernated approach!

    Status:Needs review» Needs work

    I can't actually do a patch review, but the logic behind the "hibernation"/"-1 status" seems the best approach to me. Thanx for coming up with it ;)

    Issue summary:View changes

    Update summary

    Status:Needs work» Needs review

    ...sorry for being the reason to bury this for so long. So many x-posts :/

    While you guys are speaking about data integrity, you may want to have a look at #911352: Document that foreign keys may not be used by all DB drivers.

    #773828: Upgrade of disabled modules fails seems like a sub-issue or follow-up to this, so I've demoted it.

    As you can see, this issue won't ever happen, so there's no point in demoting other issues for it.

    Now that #1503224: Cleanup module_list() was fixed and in, I think the next thing... is for someone to layout some detailed next steps to provide some direction.

    Countless of issues are currently bumping into the problem space of disabled modules. This needs to end.

    Thinking through the problem space based on D8's architecture leaves me with only one possible conclusion that is a radical change compared to now. Instead of duct-taping the current bogus architecture and making it even more complex, we need to revise the concept entirely. I did not want to hi-jack this issue with my proposal, so I've created a new issue:

    #1883658: Remove the concept of disabled modules

    Feedback welcome. (Again, please over there, so in case that issue won't fix, we can return to here.)

    Status:Needs review» Postponed

    postponing this while we give that a go.

    Title:Ensure that disabled modules maintain data integrityDisabled modules are broken beyond repair
    Priority:Major» Critical
    Status:Postponed» Active

    Marked #1883658: Remove the concept of disabled modules as duplicate, re-titling this one to describe the problem rather than any proposed solution.

    Quoting fago from the other issue since that sums up my current position (which is the same as 18 months ago):

    Yes, let's face it: Disabling modules is broken beyond repair. I don't see it helpful for troubleshooting to offer a feature that might break your site's data integrity. That's not helpful, so let's remove it.

    People arguing for disabled modules please read #1017672: D6 to D7 update process permanently deletes comment bodies and other data, and throws fatal SQL errors and #895014: All fields of a node type are lost on module disable if you haven't already, this isn't a theoretical issue.

    Answering webchick from the other issue:

    Look at it this way; we wouldn't be finding all of these issues with disabled modules if normal people didn't have a desire to disable modules, and do it often, whatever their reasons.

    There's two reasons why modules can end up disabled and cause problems, which have nothing to do with how much people love the feature or not:

    1. Upgrade instructions explicitly tell people to disable modules, they don't have a choice or they're "doing it wrong", see previous comment for the kinds of fun that causes.

    2. The uninstall tab is relatively obscure and hard to find (and not even an option for modules that don't define hook_uninstall() which I bet many field/plugin-defining modules don't since they're not maintaining their own schema), so plenty of sites end up with a bunch of 'disabled' modules that have long since been removed from the code base.

    I'm with @Jose Reyero in the other thread. Data integrity checks like #1451072: Deleting a comment author while the Comment module is disabled leads to an EntityMalformedException error after it's reenabled should be handled in hook_enable() / hook_disable(). We could also go with hook_info changes that flag data integrity issues in system_modules_disabled() so that we can give better context to end users. For instance, "Hey, you're about to delete Commerce X, doing so will delete the following data....".

    But running data "integrity" code in the background is a misnomer. If I don't want the functionality, I don't want its data. (Remember, there are cases where we disable and remove modules because they cause database bloat and kill performance.) We just need to be more clear from an API standpoint how to guard against integrity violations and what a module author's responsibilities are in terms of:

    * Preventing data integrity failure.
    * Rebuilding data when necessary.
    * Warning the end user about potential data loss.

    In the node access world, we've had to deal with this problem forever, and it is the module's duty to make one's data enabled, disabled, and rebuildable.

    Plugin dependencies are a separate issue, and, presumably introduce hard dependencies in module .info files.

    Fixing the upgrade process is also a separate issue.

    If you want disabled modules, then you expressively opt out of staging. Which means both content and configuration.

    The two facilities are fundamentally incompatible. In pretty much the same way they're incompatible with updates.

    Truth is, by disabling a module, you're enabling plethora of code consisting of dirty hacks that are currently scattered across the entire system, which normally does not run. The majority of that code involves brutal stop-gap fixes, because the system cannot reasonably know how to resolve conflicts when there's data for functionality that does not exist.

    In an as abstract and modular system as Drupal is today, disabling a module factually means to uninstall it step by step under the hood, since all data that isn't exclusively owned by the module needs to go through APIs of other modules, and those modules have to make hard decisions in order to keep the system running.

    Unless you record and replay every single change that happens to your site, it is impossible to replay the exact history that would be required to maintain data integrity of disabled modules. The more data changes, the more unrecoverably broken is the data of disabled modules. That's why disabled modules represent data loss. If you don't lose it immediately, then you lose it over time.

    The problem with all of this is that we're putting this logically and architecturally broken facility in front of innocent regular users. Worse, we even try to "support" it — even though there's factually nothing that can be supported.

    As mentioned in #1883658-8: Remove the concept of disabled modules, there's nothing wrong with Devel (or even a devel-alike module in core) allowing you to remove a module from the system without uninstalling it. A matter of minutes to implement. But it needs to be crystal clear that you're effectively hacking the system and that there's absolutely no guarantee that the configuration and data of the removed module will remain to exist or be functional.

    The problem is that essentially this hack is the built-in, default user interface for modules in core today and worse, we're even forcing users to use the facility, and worst, we make it look like as if it was a safe and sane thing to do.

    The problem is that even the transitional disable » uninstall state in itself breaks APIs across the board, and consequently, data integrity in other, enabled modules.

    Disabled modules worked well in a time when every module was an atomic, isolated isle, mostly doing its own thing, with its own data, and its own functionality. Today, the world of Drupal modules is too abstract, too modular, and too integrated, in order for that concept to work.

    It's five minutes to midnight to get rid of it.

    At the risk of sounding like a marketing droid, if the other big systems in our market space let you disable things and we remove that capability, that's not going to look good. "Pfft, why can the WP developers figure this out but those oh-so-smart Drupal people can't?" (You know that's not a fair comparison, and I know that's not a fair comparison, but that doesn't mean the comparison won't get made.)

    Is that even the case though? Last time I looked at WP (admittedly some time ago), various widgets required hard-coding function calls into the template. You can't disable that without hacking the code out of your template again.

    Also like sun says (and apparently me in July 2011 in this very issue) we can still provide the facility for this, just hidden away in devel with a warning instead of baked into the entire system.

    With the approach proposed by @sun in #1883658: Remove the concept of disabled modules can we get any replacement for the troubleshooting ability we lose? Everytime a non-developer opens an a bug report I can't reproduce, I ask him to disable contrib modules one at a time until she figures out which one is the cuplrit. This won't work with the new proposal.

    > Unless you record and replay every single change that happens to your site

    I still like the sound of this. We already record actions for future playback with the core job queue. This would just need to be able to handle invoking a given module and hook.

    @plach I think your choices would be: ask the person to make a backup, and then A) uninstall the contrib modules or B) ask them to install devel and use it to disable modules. I'm guessing we would put up a good doc page on how and why to do that.

    For PR: I wonder if we could rename "uninstall" to "disable" so we would still have disabling... and then call removing the code either uninstalling or removing the module.

    Sounds like we might have a concrete sub task: do a survey of a couple other systems and see what terms they use, and what they mean.

    Issue summary:View changes

    Updated issue summary.

    Issue summary:View changes

    Merged issue summary from http://drupal.org/node/1883658

    Issue summary:View changes

    Updated issue summary.

    Issue summary:View changes

    Updated issue summary.

    @plach I think your choices would be: ask the person to make a backup, and then A) uninstall the contrib modules or B) ask them to install devel and use it to disable modules. I'm guessing we would put up a good doc page on how and why to do that.

    Ok, this would work for me.

    Category:task» bug

    I've revised, merged, re-structured, and clarified the issue summary, in the hope to clarify the overall problem space for everyone who hasn't been neck-deep involved in all the critical bugs of the past years, nor any of the major configuration system and plugin system issues that are completely stuck right now.

    The current issues are of primary concern here, as we're running into conflicts with disabled modules being officially supported, but it is factually impossible to support them, because finding a solution is identical to trying to solve the equation of 0x = 1. The only possible way would be to introduce tons of code that consists of lots of dirty conditional hacks that need to be re-executed at runtime, and which would not actually help or improve the situation of data integrity and data loss in the end - all it'd do is to prevent fatal errors.

    In short, we're trying to make sense and improve the new architectural features of D8, but the facility of disabled modules presents a major blocker in various issues. If we want the new D8 features to be feature-complete and architecturally sane, then we need to do something about this issue. And quickly, because time is running out.

    So far, pretty much all resolution proposals involved to replace the current notion with something else, because the current concept is utterly broken.

    Therefore, the most logical first step would be to remove the current facility. Second, reinvent and redesign it from scratch.

    At least that's what we'd normally do for any other facility in the system, if it would show the same track record of critical bugs and current unresolvable problems. It would especially make sense in this case, since neither the intended audience nor the actual use-cases of disabled modules is clearly defined, so all we're really doing currently is to duct-tape a preexisting and broken concept of which no one really knows why it exists in the way it exists, and everyone who wants to retain the current implementation blatantly ignores its brokenness. We can do better. :)

    This have been a pain for so long and I hoped we could fix this in D8. I see no other way of fixing this then ripping the functionality out of core. Let's get out of this mess.

    - Turning of the module will remove settings and data and clean up any run states.
    - Uninstall remove the modules file from local system. This might fail if webserver is missing access.

    Okay, I'm going to ask a question here.

    Known bad scenario:

    1. Disable module
    2. Make significant changes to entities on the site
    3. Re-enable module: stale data may be inconsistent, causing critical bug reports

    Proposed scenario:

    1. Export module data Back up entire site
    2. Uninstall module
    3. Make significant changes to entities on the site
    4. Re-install module
    5. Re-import module data. Restore entire site to backup at known good state

    At this point, the stale data may still be inconsistent with the new state of the site, depending on the nature of the changes made. My question is, will the data-import code for modules be better equipped to detect and repair inconsistencies that may have developed since it was exported, compared to what modules can do today with their existing data during hook_enable?

    If the answer is yes, then I am all in favor of the proposed changes. If the answer is no, then we have only moved and slightly obscured the problem, not solved it. Mind you, it might still be an improvement to obscure the problem if it cannot be fixed, but we should consider the cost vs. the benefit in that instance.

    Edit: Sorry, I missed an important part of the proposal. I am in favor of option A.

    @@ -0,0 +0,0 @@
    -If the answer is no, then we have only moved and slightly obscured the problem, not solved it.
    +If the answer is no, then we have removed the problem, and we don't try to solve it.  We purposively remove an architecturally flawed design.  We allow us to rethink and come up with new ideas and design proposals.

    And the answer is no. Let there be some Einsteins to think different, think big, and beyond imagination. :) As long as we meanwhile unblock progress for others and keep the system's architecture clean.

    Patch in #108 "Needs work": :)

    -If the answer is no, then we have only moved and slightly obscured the problem, not solved it.
    +If the answer is no, then we have removed the problem, and we don't try to solve it.  We purposively remove an architecturally flawed design.  We require the use of contrib tools to re-introduce the flawed concept at users' own risk.

    I think this is a plausible strategy, and probably an improvement over the current situation. The data import problem is going to remain a difficult problem that will be difficult to document and describe, but perhaps that is a discussion for a separate issue.

    Edit: My mistake. I am in agreement with #111.

    At the risk of duplicate content, I'm syndicating this once more for architectural review. Apparently, some people missed that the other issue was marked as duplicate.

    Removing a module is piece of cake: Remove its entry from the system.module:enabled coniguration. Flush all caches.

    Make it clear in that new facility's UI that this is a dangerous operation, which may involve data loss. Educate users that modules should only be removed temporarily, for a very short time, for the sake of debugging. Clarify that users might have to restore their site to a backup created before modules got removed.

    Actually, this seems like a pretty reasonable suggestion to me. I'm not sure we could safely expose it in the UI, but it is something that's very easy to write a handbook page for. (Yay CMI.) So I'd say documenting this troubleshooting technique clearly in the handbook would resolve my earlier concerns about removing the disabled state. I think proposed solution A in the current summary is the way to go.

    Assigned:Unassigned» Dries

    This is strategically important enough that we should run it past Dries.

    Issue summary:View changes

    Updated issue summary.

    By reading many of the comments in this case I have the impression that some are thinking that users are disabling/uninstall a module for no good reason. But in real world we must more expect that a module is totally broken piece of crab that has only or at least very many malfunctions and does not behave as intended. Additionally to this users like to get rid of this broken module and remove all data from Drupal to get back to a stable Drupal installation. This is why Drupal need to remove all data related to a module if it's uninstalled.

    I do not like to disable a module and keep it on my disk until the 8.x get's upgraded to 9.x. I just like to remove all this stuff from my disk, including everything saved in the database by this broken module.

    We have so many modules that are not properly maintained and so many people who cannot code, nor debug and so on. A module may discontinued because of security issues or lack of developers... I guess we have all of this, but maybe a proper uninstall.

    @hass, no-one's talking about removing the ability to uninstall modules, only the ability to leave them disabled without either unintalling or re-enabling (in fact we're not even talking about removing that necessarily, just making it much less prominent, slapping a warning on it that it's for temporary troubleshooting only, and not continuing to have runtime code attempt to support modules being in that state).

    It sound like we agree and might be at a point to start with a patch. Shall we try a patch for proposal A, or do we now want to continue discussing not if disabling is broken beyond compare, but now: what solution to pursue?

    Oh sorry. I missed that we are waiting for feedback from @Dries.

    Dries, ask bjaspan to recall his memories of the big discussion in Coppenhagen about upgrading disabled modules. When he chases you out of his office, that might be enough to convince you that disabled modules are a problem.

    I just now skimmed this entire thread, and what stands out to me is that the only identified use case for disabling a module while retaining its data and configuration (so that when the module is re-enabled, you don't need to redo all that data entry) is for temporary troubleshooting. And that for anything other than temporary troubleshooting, fully uninstalling the module is what the site administrator actually desires (or should desire), but doesn't always do, because it's an extra bunch of clicks with the current UI.

    If we believe this to be true, then I think the issue summary's option A is clearly the best choice.

    But I'm concerned that real site administrators are under-represented in this issue thread, and that we haven't considered real world use cases for when hibernating (#46) a module is strongly wanted. Without knowing what and how common those use cases are, it's hard (for me) to evaluate whether option B is better. Perhaps Dries magically has a feel for this, but in case not, how can we gather this information?

    In addition to troubleshooting/debugging, some use cases for temporarily disabling a module include:

    a) Suppressing errors during infrequent conditions unanticipated by the module (e.g., network communication problems for a module not built to handle those gracefully).

    b) Improving performance during a heavy traffic spike (e.g., disable comment.module or something like that to get load under control, but without deleting all existing comments).

    All things considered, I'm leaning towards option A (i.e. remove enable/disable). If we went with A, I believe we'd see (1) solutions that help people preserve their configuration files upon uninstallation and provide instant re-configuration upon installation, (2) maybe even a module-level kill-switch setting in a module itself, and (3) versioning of configuration. We could discuss how important something like (1) is and if it should be part of core.

    Option B (i.e. required hooks vs cosmetic hooks) is not a complete solution; it still can't guarantee data integrity. Plus, I think to a large extent we can already do some of this through permissions and existing configuration options. I feel option B is a non-starter.

    There may be an option C? The only time it may be safe to disable a module is when you are in maintenance mode and no content is created unexpectedly. Maybe you can only disable a module when in maintenance mode, and you can only get out of maintenance mode when no modules are disabled? It still offers no true data integrity, but it certainly provides a safer environment for people to disable modules. It would work for troubleshooting, but not for use cases (a) and (b). This solution isn't perfect either but may be worth exploring a bit more.

    Another data point: In some exciting client work this week, we had a site-killing "function not found" error that we were able to cure by disabling and re-enabling one of our custom modules. There may have been another way to do it, but disable/enable is one way out of an ugly circular dependency situation.

    Only allowing disabling of modules in maintenance mode sounds like an interesting possibility, since that's already the "stand back, I'm breaking things!" mode.

    OK so to get started on this, I think the following are relatively uncontroversial at the moment?

    1. At the API level, allow a module to be directly uninstalled (i.e. no intermediary disabled state).

    2. Build a separate UI for disabling and enabling modules - we can then make the decision about whether this is possible outside maintenance mode (I quite like this idea since it enforces the warning).

    3. Remove the ability to disable modules by unchecking them on the admin/modules page - we still might want to leave uninstall in a tab for now.

    There are a lot of use cases for a site administrator to disable a module (and not just temporarily). Also, most modules actually can be safely disabled and turned on again at an arbitrary later time. The bug being discussed in this issue is not a widespread problem that many modules face; it's only a serious bug because when it does happen, the consequences can be catastrophic.

    Given that, removing the functionality altogether seems like an unfortunate solution, even as an "intermediary" step.

    If we don't want to fix this directly (and I don't really understand the specific criticisms of option B that a couple people mentioned above, but I do think option B is complex and possibly not worth the effort) then it seems we should look at how to work around it. Making the UI for uninstalling modules more prominent and the UI for disabling modules less prominent definitely sounds like a good idea to me, but simply slapping a warning on all modules that you shouldn't disable them except temporarily is crying wolf (and likely a warning that people will ignore exactly when they shouldn't).

    Earlier I think someone suggested just making it a flag in the .info file or whatever, to allow a module to declare that it is not safe to disable. This does put some burden on module developer, but not really a whole lot, and it can be addressed with documentation. I don't think most modules would need this flag anyway.

    So what I would think might work is:

    1. When you uncheck a checkbox on the modules page and submit it, you get a confirmation form.
    2. The default behavior would be to uninstall the module (hence the need for the confirmation; we have that now). However, there would be an option on the form to disable the module instead.
    3. If the module is flagged as not possible to disable, core would prevent you from selecting that option. But it would be implemented in such a way that a module like Devel could alter the form and expose it again with a warning about only doing it temporarily.

    Hopefully this is an achievable (if not perfect) solution, with some usability wins in that it makes it easier for administrators to uninstall a module rather than simply disable it (which in most cases is probably what they wanted anyway).

    2. The uninstall tab is relatively obscure and hard to find (and not even an option for modules that don't define hook_uninstall() which I bet many field/plugin-defining modules don't since they're not maintaining their own schema), so plenty of sites end up with a bunch of 'disabled' modules that have long since been removed from the code base.

    Actually as of Drupal 7, that's no longer true anymore; as of #151452: Uninstalling modules does not follow dependencies and can lead to WSOD any module can be uninstalled via the user interface. But it is definitely hard to find the tab and a pain to do, so most people in my experience don't bother doing it.

    The bug being discussed in this issue is not a widespread problem that many modules face; it's only a serious bug because when it does happen, the consequences can be catastrophic.

    I've come up with the following kinds of "data" that modules need stored, along with a representative module for each one. Of course, some modules store multiple kinds of data.

    1. permissions: contextual.module
    2. own config: action.module
    3. settings within or referenced from another module's config: php.module
    4. entity type: comment.module
    5. entity/field storage: field_sql_storage.module
    6. field type: email.module
    7. field: forum.module
    8. field-like data that ideally should be converted to a real field for D8: contact.module
    9. non-config, non-entity data: dblog.module

    @David: for which of these kinds of data/modules do you consider disabling to be catastrophic vs. not?

    If we went with A, I believe we'd see (1) solutions that help people preserve their configuration files upon uninstallation and provide instant re-configuration upon installation

    I wonder if contrib (e.g., http://drupal.org/project/deploy or something like it) could even do this for entity data, given the serialization/deserialization work being added to D8. That work has been primarily motivated by web services, but no reason contrib can't leverage that to serialize/deserialize to/from files.

    I already started writing a module that will export entities to serialized files, then re-import them into a fresh site. I'll be using this for a demo of replicating an entire drupal site, content and all, into a freshly installed Drupal instance. So yeah, there's a lot of potential there honestly.

    Another example from [#1822048:1], note that in Views' case it goes to great lengths to handle the case of disabled modules, since a view has myriad dependencies across multiple modules which currently may disappear at any time.

    A view contains a lot of different fields, which contains to several modules
    Due to a site-update some of these modules get disabled, so the handlers doesn't exist anymore
    In previous versions the actual executed view on the livepage didn't failed but just made these specific things hidden but sure you should really take care about that. Let's assume you have some access checking, so the user could see content which never was supposed to be seen.

    If the access plugin work gets in (i.e. access to blocks and potentially router paths governed by configurable sets of plugins which can be provided by multiple modules), then that's going to be a much more widespread case.

    In previous versions the actual executed view on the livepage didn't failed but just made these specific things hidden but sure you should really take care about that. Let's assume you have some access checking, so the user could see content which never was supposed to be seen.

    Rules in 7.x has the same problem to solve. What I did there was maintaining a list of modules the configuration depends on and one of the module gets disabled, a) the "plugin" gets skipped when dealing with rule and b) the rule is marked as broken in the interface + skipped from evaluations + a warning message is shown. When the module is re-enabled, the rule is re-enabled and the admin notified with a message.

    Imo, being more conservative with checks here is a good thing. In fact, I even ran into the situation on a client site where views cache got broken and so a view was executed without an (possibly important) filter! We should better fail pro-actively.

    Still, I'm not sure how this relates to the "disabled modules" feature and so to this issue. For this problem it does not matter whether the module is just disabled or uninstalled?

    I think that the question of

    How to deal with references to plugins of modules that suddenly disappear?

    is a non-obvious, secondary goal of the proposal here. Essentially, the hope is:

    1. All system components should be able to fully rely on the current list of installed/enabled modules.
    2. Things always exist if they ought to exist. An intermediate state is not supported.
    3. If a module gets uninstalled, we know exactly what to do; namely: Remove all references to the module and its functionality and its data everywhere.

    Or in other words:

    1. There is no more intermediate state of "disabled", for which additional business logic would have to figure out (somehow) whether referenced (plugin) functionality can be safely omitted, or whether the entire requested operation needs to be canceled and the code must error-out (somehow — all we're really doing there is to move the problem one layer up and leave it to the caller to figure out how to deal with the unreal error, which inherently breaks data integrity on even more layers...).
    2. If you remove functionality without properly uninstalling it, you deserve a sea of errors, because installed things are expected to exist.
    3. We turn the current tristate of 1/0/unknown into a simple and always clear 1/0 boolean. This means, for the first time in history, we can and actually have to introduce a new process to properly transition from 1 to 0 — i.e., truly removing all references to a module's functionality everywhere, and properly uninstalling the module's data (by passing the data through the module's APIs). That will cause us some hard headaches, but a proper destruction process is long overdue. Unlike the previous "unknown" state, all other system components know exactly what they need to do upon destruction.

    sun: I don't think that removing the "disabled but not purged" state will do anything to make it easier to "purge" data. If you remove a views argument plugin module, you still need to either

    1) Magically edit every view to remove the reference to that plugin, possibly causing it to break in exciting and possibly data-exposure-security-issue-causing ways.
    2) Gracefully handle in Views the possibility of "OMG where did that plugin go?"

    And read "Field", "Rule", "text format", or various other systems above as needed. Just saying "We don't support disable but not purge" doesn't make "purge" any easier.

    One advantage for the purge situation is that the module is still enabled when you uninstall it, so plugins can still be discovered, info hooks are still executed etc. Right now when a module is uninstalled, Views (or whichever module consuming plugins) doesn't have a decent way to know which plugins are provided by the modules being uninstalled since it's not really supposed to load that dead code and execute it itself.

    Thats a pretty major improvement. I had not thought of the benefits that uninstall will see. Lets proceed.

    Also when disabled modules are uninstalled it can get a lot more complicated than that, the above was only the most simple example:

    For example module x provides a plugin used by a View, then Views module is disabled, then module x is uninstalled. hook_modules_uninstalled() should still run for Views module, but then Views module can't even rely on its own API/Services whatever being available at that point.

    Bundle-providing modules are required to run field_attach_delete() bundle in hook_uninstall() for their bundles, but if field module is optional + disabled then that function isn't available.

    I think #123 is fairly uncontroversial, and I would perhaps add to it:

    4. Do not allow modules to be uninstalled or update.php to be run while any modules are disabled (#21).

    5. Further discuss whether to allow some modules to flag themselves as "safe to disable" (#124), and what such a flag should allow.

    @David: what do you think? With these 5 steps, do we still leave open the possibility to have your concerns addressed?

    Perhaps we also should keep a track of module that just got removed without getting uninstalled, the user being stupid and deleted the module directory, and lists these as broken modules? Then recommend the user to restore the module and to uninstall it properly or be ready to face a sea of pain.

    Assigned:Dries» Unassigned

    Doesn't need to be on Dries' plate currently.

    Perhaps we also should keep a track of module that just got removed without getting uninstalled, the user being stupid and deleted the module directory, and lists these as broken modules?

    This would be anything in the system config but not in the file system, should be very doable to present that. At the moment it causes quite serious issues leaving a site in that state, i.e. #1081266: Avoid re-scanning module directory when multiple modules are missing.

    @David: for which of these kinds of data/modules do you consider disabling to be catastrophic vs. not?

    No hard and fast rule, just any module which depends on responding to another module's events and can't (or isn't willing to) deal with the possibility that it failed to respond to some, cannot be safely disabled. Basically, you just have to look at your code and say "what would happen if these hooks don't get invoked for a few months"? Comment module, for example, probably shouldn't ever be disabled (at least currently) not because it defines an entity type, but rather because the entity type it defines is heavily tied to nodes and the code isn't really written to deal with the possibility that the comment and node records are out of sync.

    Core modules tend to be complicated and to depend on each other a lot and so I would expect them to be more likely than average to have these problems.

    For modules like Field, Views, etc. (which store data provided by other modules and don't want those other modules to ever be disabled either), this could be achieved with documentation, or perhaps hook_system_info_alter() to edit the flag and prevent all of them from being disabled at once.

    @David: what do you think? With these 5 steps, do we still leave open the possibility to have your concerns addressed?

    Somewhat, though I don't understand the idea of having two separate UIs... this would involve adding yet another independent page to the admin interface which also lists modules similar to how admin/modules already does? That's why I suggested just doing it as a checkbox on the confirmation form (seemed a lot simpler).

    From my point of view, it's way simplier than all that.

    If data of disabled module is prone to corruption, module should clearly notify about that and suggest deinstallation of data.
    If freshly enabled module finds it's old data to be corrupted, it should repair it. If repair is impossible, ask admin to uninstall old data. If it's unsafe, ask if admin wants to try repair or not.
    If module fails to do that, it should be considered a bug in that very module.

    No work on architecture needed, maybe except making sure that needed hooks are there.

    Seriously, if I disable a module all I really want is it's hooks not firing, none of them. If module have UI that makes sence to be turned off when the hooks are there, UI should be a submodule (see views approach). And if you will remove true module disabling, ppl will rename / copy out module's code to stop it from working when they need so. Way less safe. Way less convenient. So removing "disable module" option from Drupal provides none at all improvements of data integrity, just encourages risky workarounds.

    Long story short:
    1) Disabling modules works pretty OK
    2) If module can't take care for it's data, if it can't it's module's bug
    3) Be careful not to encourage dangerous hacks

    I know little about Drupal 8 and just skimmed through this topic, but maybe some of my suggestions will be of use (hopefully I'm not just repeating what has already been brought up).

    It seems like there are 2 main concerns here:
    1) Disabled modules are broken beyond repair.
    2) Disabling modules is a useful feature and should not be completely removed especially since no. 1 does not apply to every module.

    This leads me to think that every module should deal with the problem themselves while core provides the necessary means to do so. The way I see it handled roughly is that disabling and uninstalling are split apart: you can do either separately from the other. Every module that can handle being disabled (either no chances of any issues arising or they can be fixed when the module gets re-enabled via hook_enable for example) can be disabled. Modules that would break one way or another could tell Drupal that they cannot be disabled at all (only uninstalled). Maybe there could also be a possibility for module X to tell that modules Y and Z must be disabled or uninstalled before this module can be disabled.

    Modules could also block certain tasks from being performed when disabled if necessary (forcing users to either uninstall or enable them before proceeding). Since code of disabled modules shouldn't be run there could be a possibility to mark this in the .info file. Another approach could be to to introduce additional hooks and/or a "modulename.disabled" file for code that will be executed in disabled state. This file should be exclusive for the disabled state when no other module files are included so that a module can perform the minimum tasks that are needed for it to not break. Being exclusive it might be possible to duplicate necessary hook implementations there to just handle disabled state.

    Might be too early or the wrong place for this, but in case it's not and the suggestion or something similar goes through it might be nice to have the module page reorganized as well:
    *) 3 separate tabs: first for enabled, second for disabled and third for not installed modules.
    *) every tab has only the relevant buttons (disable/uninstall, enable/uninstall, enable) with empty checkboxes by default that allow the user to select the modules the action should be performed on.
    *) confirmation page for the action would show the necessary messages (for example "module X will not be disabled, because [some message the module itself provides]" or "disabling module Z requires modules Y and Z to be disabled as well - do you wish to continue").

    Issue summary:View changes

    Updated issue summary.

    Wouldn't it be enough to just make hook_disable() required, make Drupal to flatly refuse enabling anything not implementing it, and report bugs every time when re-enabled module finds it's data corrupted?

    The 'disabled' module state makes #1872876: Turn role permission assignments into configuration. more or less impossible.

    @#142 Why? And how moving module's code outside Drupal's directory tree would help? Because that will be happening if it will be the only way to temporarily disable module without deleting it's data.

    API docs for hook_disable() does not mention anything about interacting with permissions. API docs for hook_permissions() contain no indication about hook_uninstall(). So, why permissions set by modules inactive at the moment should be anything more than permission system's own problem? As I see it, it should simply keep the data until module is reactivated or uninstalled, that's all.

    @Molot.

    When a module is uninstalled, Drupal tries to remove its permissions.

    When you uninstall a module it's already disabled.

    Code for disabled modules is not supposed to ever be run (except for hook_uninstall() and hook_schema() which are in the .install file), that's the whole point of disabling them.

    Even if you could run hook_permission(), permissions can be defined dynamically based on the presence of other modules which might or might not also be disabled so the list might not match up.

    Because you can't run hook_permission() for the module, there's no way to figure out which permissions it defines when you uninstall it.

    This leaves the permissions system with two choices:

    1. Leave cruft around that can never be cleaned up.

    2. Add hacks (which is what is currently there) to record the module that provides a permission. Except that doesn't really work either, for example #607238: Permissions are assumed to be unique among modules, but uniqueness is not enforced.

    If you read the issues linked at the summary of this post, you'll see many, many other similar cases where data is stored which references something provided by a module, and it either goes stale while that module is disabled, or can't be cleaned up when the module is removed. These have variously added a tonne of complexity to core which barely anyone understands the full implications of, as well as real user-facing bugs like severe data-loss.

    The outcome of this issue as I understand it won't be that disabled modules gets completely removed, but that there is a dedicated UI for temporarily disabling a module for debugging, and that core will neither allow disabled modules to be uninstalled, nor guarantee data integrity for them - but you'll still be able to debug by process of elimination when you need to.

    Issue summary:View changes

    Adding another issue reference.

    What I see and read is that disabling modules works as it's supposed to, leaving it's data available for re-enabling, and that's permission system what fails, as it looks like it was never designed to have modules disappear, no matter permanently or not. Permissions was designed to stay forever once assigned, it seems, so why blame any other part of Drupal?

    In ideal world people will never need to uninstall or disable module. In real world sometimes it's needed, even on production, for many purposes, like new versions introducing incompatibilities or cookie law that just got interpreted in quite not expected way. Data like permissions should not be destroyed, and disabling will happen, one way or another. So core should just give modules tools to deal with it, like function to check what other modules are using or was using given permission and way to sign-out from permission usage on uninstall. If I need to disable Google Analytics for a week or two, last thing I want is to get rid of all my settings and configure which roles to track and which roles don't and who can choose all over again only because I needed to wait for code to meet legal requirements. I believe I'm not alone in that.

    last thing I want is to get rid of all my settings and configure which roles to track and which roles don't and who can choose all over again

    CMI solves that for you. Your config is in your VCS.
    Then "saving past, currently unused, config items just in case you need to use them again later" is a feature of your VCS, just like for your typical code, it shouldn't be the burden of Drupal and its runtime code and concepts to support that feature.

    Well, I think that's only true for a minority of sites. Most people use a system like Drupal because they want to make changes via the user interface (on their production site)... and they don't have a full staging+production setup to be able to push configuration changes via a VCS. I mean, you can put it all in code in Drupal 7 too (via the Features module) but if you look at the stats only about 15% of sites use that module.

    Also, this is especially true for something like a Google Analytics settings page brought up in the above example, because it's more "end user focused" than "developer focused" (but still equally painful to have to redo from scratch).

    We discussed this (again) at DrupalCon Portland and I believe we should remove the disabled functionality. I believe catch and Alex agree with that position.

    For now I also believe we don't need a semi-hidden UI in core; it could be a contributed module. We should look at the use cases for having such a UI and discuss whether there are more elegant solutions for them (e.g. better use of permissions).

    We can choose to bring back the disabled functionality if we can code up a viable technical solution that doesn't risk destroying people's data.

    OK, we have our orders. The most recent patch introduces a hibernate state which Dries does not want (for now). So, the patch before that is the one that needs rerolling - #79. In addition, we should edit the upgrade code and instructions so that:

    1. We refuse to upgrade if a site has any disabled modules. All modules must be active or uninstalled
    2. Edit the upgrade instructions and remove the bit about disabling all modules before upgrade. Tell folks that they need to enable/uninstall any disabled modules per above. I think it is now safe to upgrade your Drupal install in one go - there is no need to do core alone as a first step.

    To me, this plan still sounds like using a bulldozer to weed your garden.

    There are plenty of modules (I would argue a majority) where disabling causes no harm, and it doesn't make sense to prevent them from being disabled. The Google Analytics example mentioned above is a good one. People should be able to turn that off and then turn it on again a few weeks later without having to completely reconfigure it; that's basic website functionality.

    Any thoughts on #124? That comment got a fair amount of IRC love after it was posted :) But there hasn't been much discussion here.

    Nothing stops module devs from implementing a switch for toggling some feature on/off. ;)

    Nothing prevents them from writing their own CMS either... but they don't :)

    I think it would be poor usability and poor developer experience to ask modules to do this on their own, rather than providing a clean, consistent way to do it (like we have now).

    There are a lot of situations in CMI where we can say "90% of the time this will work, but in the other 10% your data is hosed and your site is rendered unusable" and in every one of those situations we have erred on the side of preventing data loss. We shouldn't be getting into a situation where we have to explain to users what situations are and aren't safe to perform certain actions. There are ways this can be implemented in contrib where the risks are up to the user installing the module, and there are more developer-centric answers like removing the module's entry in the modules config file.

    However in user-facing core I think it is prudent to always err on the side of data safety and (as much as possible) absolute clarity about what is happening when an action is performed. #124 is an improvement I suppose, but it also puts the burden on the module developer to specify when their module is safe to be disabled, and I wonder if most module developers even understand this complex situation at all. I sure didn't until well into the implementation of CMI.

    Nothing stops module devs from implementing a switch for toggling some feature on/off. ;)

    Well, at least, Drupal should list all modules implementing a switch to toggle some features or all that the module dev implemented.

    Modules not implementing the swith cannot be disabled (checkbox checked and disabled), but still be visible on the list.

    So why not keep the same module list that we use currently to enable/disable modules? Only module implementing their own switch would have the checkbox clickable.

    The UI could be confusing if the same checkbox does disable/uninstall a module.

    @xmacinfo: Consider the CDN module. It requires a special Amazon-supplied domain name so that it can alter the URLs for images and other assets. It also has a 'Disable' setting so you can turn off the CDN without disabling the module. This is good design.

    Requiring that you disable a module in order to make it behave differently is bad design.

    Hopefully D8 will bring the possibility of doing something like using drush to change the settings in container variables, akin to Symfony's console, or Mac OS X 'defaults' command. This way you would be able to change settings however you need them during development, and to script more complex settings changes.

    > We shouldn't be getting into a situation where we have to explain to users what situations are and aren't safe to perform certain actions

    Why don't we say that if a module implements hook_disable() then if can be disabled -- and ONLY if it implements it.

    That way the Google Analytics case can be covered.

    @Mile23 If all module have a disable switch like CDN, why not move that switch to the main modules list page?

    1. We should encourage developers to add a switch.
    2. We should provide a single location to list all switches. And dependencies.
    3. Why not the use the modules list page we currently have to display the new switches column?

    The modules page currently has a way to link to the module's configuration page, set in the .info file: http://drupal.org/node/542202

    configure = path/to/configure/page

    Heads up, we started working on this yesterday and will be all day. We created a helper issue though to reduce the noise, see #2003966: Helper issue for #1199946 : Disabled modules are broken beyond repair

    Issue summary also needs an extreme update of course.

    Issue summary:View changes

    Adding #607238 to summary.

    However in user-facing core I think it is prudent to always err on the side of data safety and (as much as possible) absolute clarity about what is happening when an action is performed.

    Agreed, and that is the reason for the proposed confirmation form. It would be displayed in all cases (uninstalling or disabling), and I definitely think we should be clear in the wording that turning off a module is potentially destructive, regardless of whether Drupal is going out of its way to destroy the data for you (uninstall) or not (disable).

    As an example, I suggest looking at the Synaptic Package Manager in Ubuntu. When you remove a package from your system in Ubuntu, you have two choices: "Mark for Removal" and "Mark for Complete Removal". In both cases, you get a confirmation form, and in the first case the confirmation message lists your package under a heading that says "To be removed", whereas in the second it's listed under "To be completely removed (including configuration files)". So it tells you what it's doing and how the options differ and makes both scary-enough sounding that you think before confirming, and on top of that it doesn't make any promises. But neither does it force you to destroy things that you have no intention of destroying (which is what Drupal would do if the option to disable a module went away).

    Conceptually, that is what Drupal needs to communicate also.

    #124 is an improvement I suppose, but it also puts the burden on the module developer to specify when their module is safe to be disabled, and I wonder if most module developers even understand this complex situation at all. I sure didn't until well into the implementation of CMI.

    Personally I'd be fine with leaving the flag out for starters (and continue to let any module be disabled), especially if we can come up with a good, generic confirmation message.

    In practice, the modules that are actually the most dangerous to disable aren't ones that people are too likely to want to disable for a month at a time anyway (too fundamental to the structure of a typical site).

    I think removing the ability to disable modules in core is not a bad thing. I wouldn't mind a suitably cautionary interface either, but I don't think it is required for typical site building.

    Some modules _probably_ are safe to disable - but in my experience issues crop up in the places you least expect them - look at the issues in the "History" section of the summary, and ask yourself how many we would have been able to predict and suppress disabling for those module(s) in advance.

    That said, enabling this in contrib is still an important goal, I think. I notice the patch in the helper issue is completely removing/merging the enable and install hooks as well as the disable and uninstall hooks - this strikes me as unwise and unnecessary, since it means contrib modules that provide/add a "disabling" UI will have to "work out themselves" a new (potentially "shared" between projects) hook name for disable/enable. In addition, each contrib module that thinks it can support disabling will need to implement that hook and call it directly from it's own install/uninstall hooks. It seems simpler all round to keep the hooks separate, and in core we simply always call hook_disable prior to hook_uninstall and the same for install/enable (basically just 2 more lines of code). We keep the API slightly more fine grained than we strictly need for the core UI, but make life much easier for contrib authors.

    Just to raise awareness of the issue, there'll still need to be a way of uninstalling modules via update scripts, any insights onto how this should be handled should be shared in #2001310: Disallow firing hooks during update. Thanks :)

    Issue summary:View changes

    Updated issue summary.

    I just heard about this issue today, so sorry if I am late to the party here, but I think that in spite of the existing problems mentioned in this issue, removing the ability to disable a module and retain the data should not even be brought to the table. Here's why.

    Drupal core is alright, but what makes Drupal awesome is the fact that we can add any number of contributed or custom modules. The quality of those modules, or the ability of the modules to handle any specific use case, is always undetermined. Quite frequently, things go wrong.

    A common practice to figure out which module is causing the problem is to turn it off. And by "turn it off" I mean MAKE THE CODE NOT RUN. I don't mean "disable the user interface", or "delete all the content governed by this module" or "revert all configuration". I just mean, make the code not run.

    If we have modules delete all their content and wipe out all their configuration while people are debugging, they are going to be pissed.

    As long as we allow people to add contributed code (or custom code) to their Drupal sites, we have to provide a way to toggle things on and off, without destroying all the data that has been collected.

    There will still be the ability to disable modules, although it won't be the UI in core. You will (as far as I know) still be able to remove modules temporarily by commenting them out of the appropriate YAML file, or by implementing hook_module_implements_alter() (removing entries from the list) or some other methods. I am certain that there will be contrib modules supporting this action before D8 is even released, but because of the *unsolveable* data integrity issues it shouldn't be core's problem.

    Jen brings up some really good points, despite heyrocker's counterpoints in #164, IMO.

    I realize this is a complicated issue. But, asking a newbie to manually edit YAML files or implement hook_module_implements_alter() or install yet another contrib module is the wrong direction for UX.

    @arlinsandbulte:
    What @heyrocker means is that this feature would be provided by a contrib module (could very well be devel.module, for example)

    #164 solution is to create a contrib module in order to be able to disable contrib modules.

    I'd also like to point out that we had this entire discussion at DrupalCon with Dries, all the points of view in this thread were represented, and Dries made the decision that we should drop the functionality from the UI. So I don't really know what we're still arguing about here.

    I am currently working on the patch to solve this. Actually, we are not only removing this functionality from the UI but instead removing it from core _entirely_. However, I will try to provide the amount of pluggability needed to re-establish this functionality in contrib.

    But as @heyrocker already pointed out: The decision has been made and for the various reasons mentioned here this is a necessary step. No point in bikeshedding this any further.

    I'm going to move discussion about implementation over to #2003966: Helper issue for #1199946 : Disabled modules are broken beyond repair

    ... The decision has been made ... No point in bikeshedding this any further.

    Still consider this UX...

    D7:
    - Something wrong with the site.
    - Troubleshoot by disabling contrib modules.
    ...

    D8
    - Something wrong with the site.
    - Install a contrib module (if the problem above with the site does not prevent you from doing so)
    - Enable the module (if the problem above with the site does not prevent you from doing so).
    - Troubleshoot by disabling other contrib modules.
    ...

    Unless I'm mistaken and the UX in D8 is something different, this is UX degradation IMO.

    - Troubleshoot by disabling contrib modules.
    BOOM.

    That's usually what happens.
    "troubleshoot by disabling contrib modules" has broken more sites than it has fixed (at least if you used Commerce, Og, or any more complex suite of modules) and I can't wait for the day everyone forgets that trick.

    "troubleshoot by disabling contrib modules" has broken more sites than it has fixed.

    You only say that because when this process causes grief, people complain and report issues. Each time this process saves the day nobody reports anything. So, I believe that your comment above is wrong - this process has done way more good than harm (though, we'll first have to remove it in order to see this).

    Disabling modules is actually a very powerfull feature, actually no I'm wrong, disabling module is something absolutely normal to do: you can do it for many reasons and the most important one is you disable a module when you want to disable a module. A framework such as Drupal where you cannot disable a module would be just a huge what the fuck, seriously.

    @bojanz: For a lot of Drupal users, sitebuilders in particular, troubleshooting by disabling modules is one of the few ways to do it.

    What I have noticed has been getting very little thought in this issue is all those users who do not share the same level of understanding of the Drupal architecture, have the same developing and debugging skills and so on.

    Building a website in Drupal is not done in code any more - it is done by configuring functionality in the UI. The fact that every user has easy access to the source code has blurred this understanding for many. Imagine if Drupal and contribs where distributed as binaries instead.

    Personally I am horrified about the outcome of the discussion in this issue and the implications this will have for the UX for practically everyone who will build, own and maintain a site in Drupal 8.

    @heyrocker: Just because Dries made a decision (#168) , that doesn't mean he made the right decision! In this case I sense it was more a convenient than rational decision. A decision that will not delay the release of Drupal 8. That for me is a scary decision.

    Different approach

    I took a look at this a few months ago and thought about it from a different angle. One of the problems here is that I as a user, especially sidebuilder, in many cases simply don't know where a specific configuration stems from. I can do an educated guess, but that requires tons and tons of knowledge and experience.

    That resulted in me writing up the Configuration control, UX & disable/uninstall (https://docs.google.com/document/d/1sgwOgcgcomopXBqfSxDfF8T6ErPpZfD1aH8r...) where this is addressed. As seen in the screenshots it would for example give visual clues directly in the UI about where a setting comes from.

    Its not the complete solution to this problem, but I am quite sure it has ideas that not only will give us back the control of the configuration, but also improve the UX for everyone, sitebuilders included, a lot. And, hopefully, also allow us to make this right.

    If you're still arguing to keep disabling modules as a primary feature in the modules interface, and that core should attempt to maintain perfect data integrity when they're re-enabled or uninstalled from a disabled state, please indicate whether you've read through all the issues linked in the summary.

    As someone who personally worked on several of these issues, including one that caused data loss for six months after the release of Drupal 7 (#1017672: D6 to D7 update process permanently deletes comment bodies and other data, and throws fatal SQL errors) and in such a way that those experiencing the data-loss often didn't realise until it was too late, it's frustrating seeing the same comments over and over again talking about how useful a debugging feature this is without addressing the thousands of hours and unknown numbers of broken sites that have been wasted trying to support all possible cases while modules are disabled.

    To repeat what's been said several times already on this issue, it'll still be possible to disable modules via some interface, likely provided by a contributed module, for debugging purposes only. What's being removed in this issue is 1. the ability to do so from the modules screen 2. the dozens if not hundreds of core workarounds that attempt to maintain data-integrity for disabled modules while they're disabled, up to and including disabling a module in Drupal 5 and expecting to be able to re-enable it fully working with everything intact after upgrading to Drupal 8 (yes we technically support doing this at the moment).

    @tsvenson the most problematic issues are not items of configuration provided by a module, it's items of configuration which relate to plugins provided via different modules, for example field types, formatters, widgets, views handlers, content types etc. etc.

    In the google doc, views is given as an example of a module that's not safe to disable. Actually it's usually fine to disable views itself - all your views will stop working but that's what you wanted right? What's tricky is when you disable a module that provides something that's included in a view, for example an argument handler. When you visit the view, it'll say 'broken handler' or similar, so updating, exporting etc. the view won't work as expected and the view itself may not render at all. Views can't know what the plugin does without it available, disabling the module disables the plugin, but when you re-enable the module you expect the plugin to be included in all your views, so it can't be removed from the configuration for the view while the module is disabled - leaving the View in this half-way state where it has something configured, doesn't know what it is, and can neither remove it entirely nor execute it.

    However the worst that happens with Views is that something doesn't render or you get some errors, with field types you can no longer safely load, update or delete entities with that field type assigned - you might lose data that should be there, or end up with stale data in the system that shouldn't be. Again, the issue summary has plenty of reading material on this.

    In this case I sense it was more a convenient than rational decision

    That's not fair to say. There are dozens of arguments and painful issues for removing this feature from core, and it's not that this discussion was lightly taken, nor that the discussion at Portland only took 5 minutes or so.

    Granted, I admit, I sometimes disable modules too, but I will not cry at all if this is not supported by core anymore - especially from a Field API co-maintainer perspective. I almost cried of relief and jumped like hell went this decision was made in Portland.

    Also, ModuleHandler is pluggable, so anyone can so *extremely* easily create a module in contrib that brings back this functionality. And see that queue explode. Someone just has todo it (and no, I'm not applying for it), we just don't want to maintain this in core during the D8 cycle and pretend we have a working solution.

    I agree there is tons of problems with disabling a module, I didn't read the whole thread but I read the issue summary, and there are some stuff that tickle me.

    Field types disappear; all field values get stale.
    For a start, for example field types: whenever a field with an unknow type is loaded, we'd better use the null object pattern and give a facade object that does nothing instead of doing nothing and expecting everything using it to explode. This actually would be a good way to solve this atomic problem.

    Considering Views, it always had implemented the null object pattern to for broken or missing handlers and plugins, and that's the best solution in order to avoid everything to explode: it just fallback almost nicely in most cases.

    Entity types disappear; all references are unresolvable, data is lost.
    Whenever an entity type disappear, it just diseappear, a non dependent module will never use it, a dependent module will be disabled along its dependency: once again there should not be any problem here. Any reference is supposed to live in data provided by a dependend module that should be disabled along the way. For modules that chooses unstrict dependencies, they should always provide a fallback mecanism, whether it's using a null object pattern or anything else.

    Plugin types disappear, and along with it, all plugin IDs; all unresolvable plugin references get orphaned.
    Plugin system before everything else should implement the null object pattern and smarter downgrade/fallback mecanisms, and modules using them (such as blocks for UI) should always fallback gracefully when building a UI (and throw real errors only in development/debug mode).

    There is a lot of real hard problems here, but I don't see any that can be solved by managing dependencies in a smarter way or by implementing lower level fallback mecanisms.

    The key concept here is data integrity.
    It always have been the problem, and not only in this particular case. When you uninstall a module you also cause data integrity problems, so hard to solve in some cases that some other CMS's and frameworks choose to never delete data because they still didnot solved that problem. They allow to disable modules anyway.

    Disabling a module is not something we can drop. Leaving the only choice of uninstalling is what will cause the most serious data integrity problems because it does not leave any other choice than dropping data if a module causes problems. It's not a "module" approach problem, but mostly a "project" approach problem: a site mostly explodes when the project has not been integrated well and dependencies where not correctly set in custom modules/features/profile.

    Building a website in Drupal is not done in code any more - it is done by configuring functionality in the UI
    When I read this statement, which is sadly true for a lot of people, I can only think that if everything happens magically behind the user's clicks, then it's Drupal's responsability to allow users to be able to disable a module by clicking it. If clicking the disable checkbox makes everything explode, then the magic that happens under is not reliable enough. Disable a module is not a problem, but a non reliable magical and complex module that violates encapsulation and spreads its dependencies along the user clicks really is. God only knows how much I hate Views but to be faire Views at least is a reliable module as long as it uses a fallback mecanism such as the null object pattern. If the Field API is not able to do that, then field API is not reliable enough.

    Please, allow users to disable modules in core, and fix the non reliable code enough instead: just making a few adjustments to plugin system and field API could already fix numerous related problems. Let's also think that disabling a module on a complex site is a huge decision to make but sometime, you desperatly need it. For example something that breaks on a production site may need to be temporarily disabled for the time you need to fix it: you cannot afford to loose data in this case, but you cannot afford to let your production going down either. Uninstalling a module in those conditions is not an option, disabling it knowing how it works is.

    Of course everything of this is an opinion, and I could be wrong all the way, nevertheless I think the solution you are proposing here is way too radical and is a huge fail for developer, integrator and user experiences.

    Well said @catch and @swentel.

    Please understand that, while this was definitely a hard decision, it is absolutely necessary to remove this feature from D8 for the reasons stated by @catch. Don't get me wrong, I would love for someone super smart show up in this issue with a solution that solves all our problems while keeping this feature, however many people have spent so much time already trying to come up with solutions to no avail that we simply don't have a choice but to remove this now.

    In this case I sense it was more a convenient than rational decision.

    That's really not fair. We are at 178 comments already and many people with deep understanding of this problem and profound technical background came here and provided feedback. We have to guarantee data integrity. With this feature in core, we can't. Period.

    I don't understand the argument that is constantly made in this issue that disabling a module is such a nice debugging feature and, if we don't have it, people will lose their data due to uninstallation being the only choice left. This is not Drupals fault. It's your workflow that is seriously broken if you rely on this. You always back up your database before playing around with your modules, upgrading stuff or uninstalling/installing a module. Period.

    Please, allow users to disable modules in core, and fix the non reliable code enough instead: just making a few adjustments to plugin system and field API could already fix numerous related problems.

    There is simply NO way to fix all these problems on the side of each module. Every module that implements or provides a hook would have to cover all scenarios that could potentially occur. With the vast amount of contrib modules out there and the overlap and integration that all those modules share with each other this is simply impossible.

    For a start, for example field types: whenever a field with an unknow type is loaded, we'd better use the null object pattern and give a facade object that does nothing instead of doing nothing and expecting everything using it to explode. This actually would be a good way to solve this atomic problem.

    It' s not just loading though is it. Say the field is an entity reference, and the module you disabled has code to update field values when referenced entities are deleted. If you disable the module, this code will not run. So either you end up with stale data, or you have to write a bunch more code to do a comparison against all existing entities when the module is re-enabled (which would be a very expensive operation on a site with lots of content). And that's a simple example.

    It' s not just loading though is it. Say the field is an entity reference, and the module you disabled has code to update field values when referenced entities are deleted. If you disable the module, this code will not run. So either you end up with stale data, or you have to write a bunch more code to do a comparison against all existing entities when the module is re-enabled (which would be a very expensive operation on a site with lots of content). And that's a simple example.

    Actually that's a problem that lies way beyond the disable feature, it's also true when you uninstall. A module such as entity reference actually creates implicit loose dependencies, and it is it's responsability to be able to get over this scenario. If the referenced entity does not exist anymore, it has the responsability to either raise an exception or fallback silently on "there is nothing in here" or in a null object if the field is required. The scenario where the entity reference field would react on update or delete won't happen anyway since the module is disabled and update/delete operations are therefore not possible anymore, so it's quite a valid scenario for this code not to run. If the module is disabled, the data is not lost, and those hooks must absolutely not run. If I reenable the module later, data will be there and there's nothing that should have changed. There is no code to run on enable since there is no possible way the entities have been droppped since the entity type has been disabled and its storage/controller instances along the way.

    Anyway I'll leave this issue to the core maintainers and developers since they are the one to maintain the software, but I really think that all the raised problems here may happen in a lot of other cases: this data consistency problem does not only happen on disable, it will also probably happen arbitrarily due to core bugs or poorly coded custom code. Even if you choose to remove the disable feature, you'll need to fix most of those issues anyway. I think that modules such as entity reference needs to be extra careful and extra reliable regarding all those issues independently of this current discussion thread's problem.

    Please provide a suggestion for how to fix this 'simple' problem then... At the very moment when you disable a module, all the data that would otherwise potentially be touched by said module is now "stuck in time". No hooks fire for that module and it has no chance to interact with any of the data that is being processed, which it would otherwise have touched in some way or another. Now, when re-enabling that module, there is a good chance that you already have broken data. There is no way that every module out there can cater for all the eventualities that might occur. There is an astronomical number of potential interactions between modules that you can't possibly predict. You can't seriously expect every module to gracefully fall back and magically resolve all those scenarios. So, what you would need is something that allows you to go back in time and fire all the hooks, events, etc on all the data that passed by in the meantime. Now, please elaborate how you want to fix that issue.

    At the very moment when you disable a module, all the data that would otherwise potentially be touched by said module is now "stuck in time". No hooks fire for that module and it has no chance to interact with any of the data that is being processed, which it would otherwise have touched in some way or another.

    That is actually what means "disable a module". Stalling its data is the very essence of disabling it. It's up to the site maintainer to know what (s)he does. [EDIT: that is exactly that would happen if you enable the module late in the site lifetime].

    Now, when re-enabling that module, there is a good chance that you already have broken data.

    I wouldn't say "good", I'd just say there is a chance, yes. That's up to the site maintainer to know what (s)he does.

    There is no way that every module out there can cater for all the eventualities that might occur

    A module has only two eventualities to consider: is my data valid? Yes, continue. No? Wait and see, but do not make the UI going WSOD. That is all.

    There is an astronomical number of potential interactions between modules that you can't possibly predict.

    That's definitely true, indeed. But if the site happens to have an astronomical number of interactions, I'd say you're stuck into extreme complexity and your problem is way higher than this issue.

    --

    If data is broken after all that, I'd just tell the developer to fix it, if possible. And it's better to break a few pieces of data than an entire site. Drupal is a CMS, not some kind of higly critical information container: if it was, we'd just use the relational database in one unified coherent fashion instead of making pretty much everything loosly coopled and independently backend-able.

    Actually that's a problem that lies way beyond the disable feature, it's also true when you uninstall.

    It is at the moment, it won't be once the issue is fixed.

    Right now uninstall hooks don't have access to anything in .module because that code is not supposed to run, because the module is disabled, making clean-up very difficult. Once modules move immediately from installed to uninstalled, the uninstall hooks have full access to the module API (as does anything operating on hook_modules_uninstalled()) so it's possible to reliably clean up.

    Uninstall is not so simple, it's a problem on more CMS's than Drupal, I'll expect a lot to fail in correctly uninstalling even you make the whole API simpler to use. Think of field types for example: the module using the field is not the one I'm uninstalling, problem remains the same, data consistency is also fucked up by the fact that if my field was required, or if references still exists in configuration, poorly tied reference fields and such you will experience stalled data and broken references problems too.

    There is nothing to prevent the field API from removing field data when the field type does not exist anymore: it should be able in all cases since the storage backend is not provided by the type, but by core or potentially yet another module. Even if the field can't run any hooks, it can successfully remove all associated data. Drupal 7 can't because it will fail on non existing field types and wrong definitions, but from the field name name it is able to retrieve the database table name and drop it. Simple exemple, and from what I can see in Drupal 8, it seems that there are huge efforts being made for separating storage and business maters, which means that if you don't know the business, you still can delete stuff.

    EDIT: What I mean in all this is that stupid business code may and will exist, data problem will come along, no matter how hard you try to fix the disable/uninstall problem: those problems exist because the dependencies are not strict and explicit, and the code is not robust enough.

    EDIT: Anyway no hard feelings, just a simple disagreement, I said what I wanted, I leave you to the implementation now. Thanks for listening and answering.

    For what it is worth, I would like to address the UX of removing the disabling of modules. Since I started following this thread and came to an understanding of what can go wrong with disabled modules, I have completely stopped re-enabling modules in D7. When I have a situation that requires disabling modules for debugging purposes, I do so on a copy of my website. Once I have decided that I am done with the test, I simply sql-sync the live site back over the dev copy, and all of my temporarily-disabled modules are enabled again.

    This workflow may in fact be somewhat harder for novice users; however, we would not reasonably expect to advocate disabling modules on a production site just to avoid a site copy, would we? There will definitely be a period of education required here, but once it is over, I don't think that we will find ourselves much worse off than we were when we could disable modules -- and we've also sidestepped some very nasty problems.

    The point above about disabling modules such as google analytics for a longer term (e.g. not just for debugging) is valid; however, I think that we can address the issue of reconfiguration using CMI features. I definitely believe the time has come to let go of disabling modules, and turn our attention toward fixing functionality gaps that arise thereby via other means.

    @tsvenson Making a final decision after discussions spanning multiple years and hundreds of comments over dozens of issues is hardly what I would call "convenient". I also don't think it was a decision made with an eye towards the Drupal 8 release cycle, this topic never came up actually. It was a decision made to keep people's data safe, because this is the only way to reliably do so, as has been demonstrated in this thread and others multiple times. To say that such a major decision was made for marketing, just so we can hit a schedule, is insulting.

    I will now be unfollowing this issue and move to where actual work is being done.

    It was a decision made to keep people's data safe, because this is the only way to reliably do so, as has been demonstrated in this thread and others multiple times.

    I've been doing some research and I'm going to have more comments (of a technical nature) when I have some time later, but on a non-technical level, this is what I still don't understand. How is forcing people to delete their data (i.e., only giving them an option to uninstall when they'd prefer to disable) at all related to "keeping people's data safe"???

    At best it can be said that this would result in Drupal being more clear about what's happening to your data, and that's a good thing. But there are many other ways that have been discussed above that would achieve that clarity that don't involve removing the concept of disabled modules altogether.

    Each module will now need to provide a way to disable itself. If all modules are providing a clean way to disable themself, Drupal core might display a clean module list where modules could be disabled.

    In a way, that would duplicate the current module list, but instead of core disabling a module, disabling a module would provided by the module itself.

    How is forcing people to delete their data (i.e., only giving them an option to uninstall when they'd prefer to disable) at all related to "keeping people's data safe"???

    Because it will remove a whole category of issues like #1017672: D6 to D7 update process permanently deletes comment bodies and other data, and throws fatal SQL errors and the complex and fragile workarounds that have been added to gaffer tape around them.

    Also it's not forcing people. Assuming a contrib modules provides this option, it's forcing people to go an extra few steps before they can disable a module (or to a different admin screen if we end up putting one into core, I'd not be opposed to an issue to put a UI back in on a separate screen, once the modules UI and other band aids are cleaned up here), and preventing them from doing disable -> uninstall.

    But there are many other ways that have been discussed above that would achieve that clarity that don't involve removing the concept of disabled modules altogether.

    None of those have been viable. The closest was for every hook/plugin implementer (or consumer) to somehow indicate whether any particular module is safe to disable or not, then prevent disabling modules for those - which still prevents those modules being disabled for debugging purposes (which a dedicated debug UI wouldn't) and remains extremely fragile in that it depends on everyone developing modules that need to indicate one way or the other understanding all the nuances of this issue (which is clearly not the case).

    Yikes, its practically impossible to keep up with the flow of comments here today :)

    @catch "please indicate whether you've read through all the issues linked in the summary."

    Sorry, but, bluntly, that sounds to me that I have to qualify to have the right to give any input/feedback by first spending x amount of hours, in this case probably several days, to read through all those issues and probably do testing to understand everything perfectly from a technical point of view.

    It also means that the majority of users affected by this change will be excluded from having an opinion. That simply because they don't have the coding skills and experience to understand what is going on under the hood.

    In my opinion we need to do exactly the opposite. We need to give more respect to users based on why they have chosen to use Drupal and for the skills and experiences they have and can contribute from.

    @swentel @heyrocker: Sorry for coming out so blunt about the decision. Hands up, I completely admit that I screwed that one up with the wrong choice of word.

    Only thing I can say to defend myself is that it stems from my frustration about that I strongly believe that we aren't taking UX design/strategy seriously enough and including early enough in the development process.

    Many have given good arguments about why this is needed due to data losses and other thing. However, that is also sort of defining a point of origin to use for coming up with the solution. Which in this case is to remove the ability to disable modules to prevent these data losses.

    I am far more interested to better understand how we got to this "mess". What could we have done differently and so on. This issue is now two years old and I see little thought has been given to:

    • Sites built with Drupal is a process of configuring functionality in the UI
    • Why is configuration and data getting this messed up?
    • Could it be possible to better isolate site features to prevent these things happening?
    • How are users (sitebuilders) working with modules?
    • How are they trying to find out what causing problems with their sites?

    That is just a few basic UX questions that could have been asked and used to find ways to avoid this happening.

    Particularly as we know that no real usable site will ever be built with only Drupal Core. No, instead we know that a large percentage of Drupal sites have lots, often over 100, contributed/custom modules.

    We also know that those sites will need not only maintenance but also changed and new features. New modules will be available that will be replacing modules currently used.

    I don't see this as uncommon scenarios.

    I see the as typical scenarios in both developing the first version of a site and the continued improvement of it.

    Thus, the conclusion is that modules needs to be disabled/re-enabled not only for debugging purposes but also as a natural process of the evolution of the site.

    This also demonstrates that the Drupal community need to take UX more seriously and try harder to make it an integral part of the development process. In fact, it is so important that it often needs to be addressed before the first line of code is written.

    Sorry, but, bluntly, that sounds to me that I have to qualify to have the right to give any input/feedback by first spending x amount of hours, in this case probably several days, to read through all those issues and probably do testing to understand everything perfectly from a technical point of view.

    If you want your comments to be given weight, then doing background reading, especially when it's so prominently available, is usually a first step. Pounard found time to post very lengthy comments on this issue, while in those comments admitting he'd not found time to read it. Similarly you found time to post two paragraphs about not wanting to read the other threads. I don't understand that approach at all.

    It's a complex topic and the majority of the posts in favour of keeping the feature boil down to "no I want to keep it, make it work", which Drupal core has failed to do for at least 6 years or so and all the people who tried to make it work have been burned out by it.

    . This issue is now two years old and I see little thought has been given to:

    ...
    Why is configuration and data getting this messed up?
    Could it be possible to better isolate site features to prevent these things happening?

    No hundreds of hours of thought have been given to those topics, much of it documented in the linked issues which you've just refused to read, and which led to this issue being opened.

    #943772: field_delete_field() and others fail for inactive fields has plenty of discussion on exactly that topic for example.

    [off topic warning]

    @pounard

    Building a website in Drupal is not done in code any more - it is done by configuring functionality in the UI
    When I read this statement, which is sadly true for a lot of people, I can only think that if everything happens magically behind the user's clicks, then it's Drupal's responsability to allow users to be able to disable a module by clicking it.

    Let me elaborate a bit on what I mean with that. But, first let me be clear about that I am not saying that to diminish the code developer, far from it.

    I see it like this:

    • You write code to extend Drupal's, in core, contrib or custom, capability to build sites with.
    • You build sites by configuring the core, contrib and custom module functionality using the UI

    This configuration is done primarily by the sitebuilder role. However this role and its needs is somewhat neglected by the community. This is done by trying to solve site users needs directly in code and without providing the needed configuration options in the UI for the sitebuilder to be able to properly tailor the available functionality for the specific sites needs.

    From my point of view: Drupal has enormous potential, but a lot of this potential is hidden away in code because the needed configuration in the UI is lacking.

    Please also see my The Drupal Site Building Experience post where I go much more into detail about this, including presenting a simplified role map about the communication needs and also illustrating just how central the sitebuilder role really is.

    @fubhy

    I don't understand the argument that is constantly made in this issue that disabling a module is such a nice debugging feature

    Well, I could counter by saying I have since long lost count about how many times I have seen project maintainers recommend users to disable this or that module to see if that fixes things.

    Since that recommendation is made by those who built it, it is quite understandable why that argument is given here.

    I'm unfollowing as well. I honestly swear on my heart that I will won't fix every issue in the D8 cycle pointing to this one in case disabled modules stays in core and an issue arises with fields, active, disabled or dead sites because cron doesn't run anymore.

    Again: someone just create a contrib module and take it there. If one can fix it nicely, bring it back in D9, for now, this feature has to go.

    @catch

    Similarly you found time to post two paragraphs about not wanting to read the other threads. I don't understand that approach at all.

    That is conveniently taking a quote out of context mate. Particularly as you referred to my GDoc I wrote back in January in your previous comment...

    We also know that those sites will need not only maintenance but also changed and new features. New modules will be available that will be replacing modules currently used....Thus, the conclusion is that modules needs to be disabled/re-enabled not only for debugging purposes but also as a natural process of the evolution of the site.

    I don't see how that conclusion follows from the premise. If you are installing a new module that you intend to replace a prior module because it implements what you want better (by whatever your definition of "better" is), then you can be in one of the following situations:

    - The new module provides an automated upgrade path from the old module. Yay: run it, verify it worked, then uninstall the old module.
    - You have to port your content/configuration manually. Oh well, do it while both modules are enabled, then uninstall the old module after you're done.
    - There's no content or configuration managed by the old module that is useful to you now that you decided you want to switch to the new module. Ok, uninstall the old module first. Then install the new module, and configure it how you want.

    What is the situation in which you want to disable the old module, but keep its data around, invisible in the UI, slowly atrophying, and then re-enable it in a potentially broken state?

    Particularly as we know that no real usable site will ever be built with only Drupal Core. No, instead we know that a large percentage of Drupal sites have lots, often over 100, contributed/custom modules.

    If this is true, then why is it bad for the ability to disable (without uninstalling) a module to also be a contrib module? Contrib is a great place to innovate and refine things that are currently unstable and experimental. And as has been said in this issue, that's the state that disabling without uninstalling modules is in right now, and has been for years.

    I thought that going to Symfony we'd be able to manage a much more flexible and modular environment, while this issue is proving the exact opposite: we're stuck into Drupalisms to the bones and totally unable to replace components or write reliable code. The only solution this issue has found is that removing features while telling to ourselves it's fine because "contrib will do it". I'm opposed to this kind of speach, it's a very dangerous idea to think contrib will solve everyting, it's contrib which actually created this delicate situation where disabling a module is impossible, because all those modules such as CCK or Views works upon a contract based on loose and implicit dependencies without solving the reliability issues (partially false because Views actually solve some of them a in a good way); This thread is just wrongly legitimating that fact. The final point of all this is that Drupal won't ever be stable until there is no feature left in it if we continue this path. Sorry, but I don't get the whole argument, even after re-reading once again everything. If you wan't data to be consistent, stop relying on contrib and make the whole more robust, disabling or not modules will never ever fix that. I'd be sad but not mad if I can't disable modules, but I'd be terribly disapointed if I heard one more time "but contrib will do for you": it's giving away responsabilities, it's false, and it's leaving a very delicate topic to potentially unstable and unmaintained code. I do not like this, some people might like this idea, and it's fine that everyone has its opinion, but I won't install a module just because I need to disable a module: it's too big of a joke.

    Excuse me a moment for being blunt, but...

    Some of the top minds in Drupal have been trying to figure out how to make disabled modules work but not work at the same time without losing data or making every contrib module implement 4 extra pathways "just in case" for over a year. They've failed. The decision to remove a feature like this is not taken lightly, and I'm sure Dries didn't take it lightly either. It's done. It's unfortunate, it's sad, but it's inevitable at this point.

    If you don't like it, come up with a patch that makes disabling a module not lead to code bloat and data loss. Can you? Please do! If you can, then you're smarter than catch, heyrocker, sun, and a few others combined. Congratulations.

    If not, then yelling at people who have been trying to make it work "well try harder!" is not going to accomplish anything except making people upset (yourself included). Because really, that's what you're doing.

    I don't like this decision either, but I understand it, and trust those involved know what they're doing. You should too.

    cf: http://www.garfieldtech.com/blog/experts-opinions

    @pounard

    (partially false because Views actually solve some of them a in a good way)

    I think it is important to look at how Views have been able to solve some of these problems. Could it be because it in many ways simply bypasses large parts of the Drupalism and as a result creates views that is quite autonomous?

    If so, that probably also gives a clue to why Views is not only able to function as good as it is, but also allowed the View developers to provide a really great UX for it.

    Can we please stop with "Views handles it fine, why can't each subsystem just do the same" ?
    That's pretty naïve, and a bit insulting.

    A view is "just" configuration for how to do a task at runtime. If some parts of the view definition are missing, then some or all of the expected behaviour just doesn't work. A block or a page won't behave as it did, that's what disabling a module does, fine.

    Field API stores and maintains user data. Huge difference. It *does* go to *great lengths* to account for the hairpulling case of "how do I limit the wreck when I'm supposed to maintain data on which I have no information whatsoever because the field type has gone, and nodes keep being created, edited, deleted".
    The amount of code and complexity added to support that is just absolutely insane, and adds an enormous technical debt whenever we need to maintain / refactor the system. "Field API to CMI" took 18 months, and that's not a minor part of it.

    The truth is that when a plugin in use goes missing, there's no "one and single good approach" for how to handle it. It depends on the plugin type, it depends on what that "thing" was supposed to be doing (a cache backend ? a text filter ? a rule condition ? an image effect ?...).

    When a field type goes missing, we can do pretty much nothing with the data, but we currently go through hell to *try* to preserve it (and there's no way to catch all the things and race conditions that can happen)
    When a formatter or widget goes missing, no biggie, we don't break and silently fall back to the "default formatter" or "default widget" - because we learned the hard way in the CCK years that, yes, widgets / formatters can go missing, and that baking a notion of "default widget / formatter" in the API would be a good idea.

    There is no simple good answer. Each plugin type needs to find how it can do its best for its own case. And that's a complex question. For each plugin type. In core, and in contrib. Took us 5 years in CCK / Field API. Took views a couple years too. How many other "plugin types" in contrib even care about the question ?

    And that's only about missing plugins, not even the CMI aspects...

    @xmacinfo:

    Each module will now need to provide a way to disable itself. If all modules are providing a clean way to disable themself, Drupal core might display a clean module list where modules could be disabled.

    That sounds sane to me.

    This will result in a lot of "provide a way to disable the module" requests in issue queues, but maybe that's not a bad thing. For modules that can be safely disabled, the maintainers will add that ability. For modules that can't, they won't. The problems will arise for poorly-written or poorly-maintained modules where somebody unwisely adds a "disable" switch without thinking it through and careful testing - but at least that is no longer core's problem (and I agree it shouldn't be).

    I don't see any reason why the "clean module list" you mention should not simply be an "enabled" checkbox column in the main module list - any contrib module providing the ability to disable other modules can alter the form to add a checkbox for modules declaring themselves "clean" (gulp). If this proves workable in the long run, perhaps it will make it back into core.

    I don't see any reason why the "clean module list" you mention should not simply be an "enabled" checkbox column in the main module list - any contrib module providing the ability to disable other modules can alter the form to add a checkbox for modules declaring themselves "clean" (gulp). If this proves workable in the long run, perhaps it will make it back into core.

    This is exactly what I have in mind.

    How is it "not core's problem" that all kinds of people will be accidentally deleting their data all the time?

    This issue is not a big enough deal (we've had it forever) to be creating this huge of a data-loss problem as part of a supposed solution. This kind of thing is going to make people stop using Drupal. Accidentally delete your field data once, for 10,000 records, and you're gone.

    Also @crell, I also really don't like the "we've already decided on this so we should stop talking about it" perspective. I know that in a court of law people can only be tried for the same crime once, but in the world of software development, this is often how bad decisions are made. We should be allowed to change our minds, especially in light of different opinions (consensus of 3 can be different than consensus of 30), new information, differing use cases, and the like. This kind of change is a huge deal, and it deserves a second, third, or fourth opinion.

    We have some problems with the system, yes, but there have to be different solutions. Here are some things @Steven suggested we try the last time we had this argument: https://drupal.org/node/194310#comment-644570 We haven't done most of these things yet. Can we start here at least?

    dreditor bug :(

    If you don't like it, come up with a patch that makes disabling a module not lead to code bloat and data loss. Can you? Please do! If you can, then you're smarter than catch, heyrocker, sun, and a few others combined. Congratulations.
    I'm sorry my comment was a bit harsh. I didn't meant any harm or insult.

    --

    I think there is a poneyload of cases where disabling a module is safe, for example overlay, pathauto, transliteration (in some cases), all *_ui modules, context, admin, admin_menu, book, legal, services links, google analytics, views in some cases, wysiwyg, better formats, recaptcha, *captch in fact, spam, mollom, search, search api, redirect 403, navigation 404, devel, profiling, fivestar, imce, backup and migrate, advanced help, help, shortcut, global redirect, authentication provider modules, services in some cases, etc...

    Considering this, I seriously think that being able to disable all those modules is a very precious feature. I heard when it was told upper that those modules could implement their own mecanism for disabling their features, but I do not think that this will be a reliable solution: someone scripting its Drupal site needs to have a unified way to do this, and leaving this to contrib will give you one different way per module to disable a feature, which will make the life of integrators and admins impossible. We actually implement mecanism such as this on custom projects in order to ensure that our features can be disabled easily on a production site if something goes wrong at delivery: the problem with this method is that every module will provide variables for this, you need to introspect the code to find it, all developers not always share the same naming conventions when the project development and maintenance goes up to more than a year, etc... This actually cause trouble.

    There is some common ground that can be found between the pro-disable (people who think that breaking everything is admin responsability) and the anti-disable (people who think it's broken anyway): for example, a very very simple solution, but which may have some drawbacks: set a key in the module.info.yml file that says "can be disabled" (for example the "required" key would have been very good for this if it wasn't used for core modules): then you could run the hook_enable() and hook_disable() on those modules when disabling them, and ignore those hooks for "unsafe" modules (and just do not let people disable the "unsafe" modules).

    Drupal 7 attemps to do dynamically in some way by preventing modules providing fields to be disabled when they are used, it's a good start. We could go further and let the developer choose if the module is safe to disable or not just by adding the "required" (or whatever other name) to true in the info file.

    This could actually also help building a simpler UI for enabling/disabling safe features (a simple table screen with a lot less modules than in the "Extend" screen) and actually gives to core one of the features module's feature and make a lot of people happy.

    So no I'm not smarter than all those people, but I think there is other paths to explore than just removing this, because I think it goes against industrialisation.

    We should be allowed to change our minds, especially in light of different opinions (consensus of 3 can be different than consensus of 30), new information, differing use cases, and the like.

    That's exactly what's happened here though. We tried to support this for somewhere between 6-12 years, failed abysmally, came up with another idea.

    Immediately going back to "no, you need to support it for another 6-12 years" is not allowing for change. Not speaking for others who worked on this in the past, but I'm not going to spend a single minute on supporting disabled modules any longer in core, except the effort required to remove it. It's my time and I've wasted enough hours of it, unpaid, trying to get Drupal 7 to the point where it didn't completely destroy people's sites. swentel said a similar thing. Like Crell said 'work harder' is not on.

    @pounard most of the modules you listed can either easily be reconfigured or allow ctools/features export/import of configuration, something that's going to be more baked in and less hacky with CMI. i.e. they're safe to disable at the moment because there is no meaningful data to lose.

    However there's two on your list which likely are not safe to disable. Book module stores references between nodes which will be stale on re-enabling, legal module or one of the very similar ones uses a node for storage of terms and conditions which could just be completely missing on re-enabling. Which again shows how hard it is to predict what's safe and what isn't.

    Status:Active» Needs work

    Maybe getting to the state with disabled modules removed brings us in a better place for exploring alternatives.

    Status:Needs work» Active

    Yes... Working on the patch.

    @jenlampton, #206:

    How is it "not core's problem" ...? ...This kind of thing is going to make people stop using Drupal. ...

    +1million!

    Count me in as one of the people really scared by this decision. Now, I won't stop using Drupal, but I'll definitely "play it safe" from now on. If you don't care about loosing most of the community testers like me out there, then please go ahead with this.

    If this is not core's problem, then it's equally not my problem that core will need people testing/reviewing. If you believe that only developers' time is precious and that they cannot and should not be spending time helping people with issues caused by testing their code, then please also respect my belief that developers should find other suckers to test their code. Simple as that.-

    Again: count me out of testing contrib or core modules if I'm not offered an easy way to enable/disable them at will and if developers are not willing to fix broken things I find during testing (this includes during enabling and disabling modules). Why should I respect their time wasted in coding if they do not respect my time wasted in testing.

    PS/disclaimer: Although I was following this issue for some time now, I was also really scared to express my opinion (take a look at all the "If you're not an expert, don't speak" comments above) and frankly I'm still scared of all the flames this post of mine might cause. This is far from the Drupal community I know and believe am part of all these years.

    How is it "not core's problem" that all kinds of people will be accidentally deleting their data all the time?

    Data loss due to uninstalling a module is a problem with your deployment workflow (or the non-existance of one). One does not simply uninstall a module on a production site, nor should one disable a module on a production site. That's why you have development and testing environments. That's why you have database backups.

    PS/disclaimer: Although I was following this issue for some time now, I was also really scared to express my opinion (take a look at all the "If you're not an expert, don't speak" comments above) and frankly I'm still scared of all the flames this post of mine might cause. This is far from the Drupal community I know and believe am part of all these years.

    No one ever said that. The only thing people asked for was to read the issue properly including the explanations for why this decision was made. You don't have to read or understand the code behind this to understand the implications of this broken feature. There are a x-dozen comments on this issue already that explained, in a detailed manner, how this whole thing is broken and why it has to be removed.

    Count me in as one of the people really scared by this decision. Now, I won't stop using Drupal, but I'll definitely "play it safe" from now on. If you don't care about loosing most of the community testers like me out there, then please go ahead with this.

    If this is not core's problem, then it's equally not my problem that core will need people testing/reviewing. If you believe that only developers' time is precious and that they cannot and should not be spending time helping people with issues caused by testing their code, then please also respect my belief that developers should find other suckers to test their code. Simple as that.-

    Again: count me out of testing contrib or core modules if I'm not offered an easy way to enable/disable them at will and if developers are not willing to fix broken things I find during testing (this includes during enabling and disabling modules). Why should I respect their time wasted in coding if they do not respect my time wasted in testing.

    Right, go ahead and 'threaten' to not test anymore, that's the spirit ... *sigh*

    Let me get this straight one last time: A _whole bunch_ of the _smartest_ core developers in this community tried, in a combined effort, to find a solution to the problems related to this 'feature'. We failed. And I would even go as far as to say that, right now, it's simply impossible to provide a unified feature that supports disabling _all_ modules without scraping your data. In order to provide a solid core for Drupal we have to think about worst case scenarios and well, those are MUCH worse in case of having a feature for disabling modules than the UX implications of removing that feature.

    At this point it sure feels like everyone that does not support or understand the necessity of this feature removal simply cries in outrage as if we, maliciously, stole the baby's lollipop.

    I am unfollowing this issue too now. It's simply too frustrating and time-consuming. I believe we did try very hard to justify this issue before the community. I am sorry if we failed with that too. Will come back with the patch once ready.

    For what it worth, I understand the expressed fear. I'm on this side too. It happens oftenly enough that a dormant bug triggers on a production site, or the client needs to disable a specific feature for a certain amount of time, or stuff like that. We always test the disable first on a development environment, we have workflows, but it happens that the targetted delivery on the production site is to disable a module, that will need to be enabled again later. That's why I can't stand this thread.

    On the other hand, fubhy and alexpott told to me on IRC that there the goal is to cleanup core and make the install and enable process cleaner: in order to achieve that, if I understand it well, removing the feature is necessary in order to explore new ways to implement it later in a followup. That's a fair point. Now, from my perspective, I don't agree with this at all, Drupal 8 is supposed to reach stable by the 4th quarter of this year which means that if this patch is this being held longer than one or two months in the issue queue, the followups will never happen, and I seriously doubt than any followup will land at all considering how controversial is this topic.

    That's why I'm still definitely opposed to such change and I'd really like to see a wiser and less extensive compromise happen. For example, I'm not opposed at all, in fact I'm pro with fubhy and alexpott about the fact that module handling needs serious cleanup, especially dependencies management, but I think this can be done without removing totally the enable/disable feature.

    Its sad that this had to turn out to yet another role war in the community. It only creates a hostile atmosphere between us, despite we all share the same passion and goal to make Drupal better.

    But, this is still going to be a real problem for many users and I definitely share the fear with @jenlampton that Drupal stand a good risk of losing a lot of users, both existing and potential new. Not just because of this individual issue, but because there are so many UX related issues with Drupal that is not dealt with in an optimal way.

    @fubhy:

    A _whole bunch_ of the _smartest_ core developers in this community tried, in a combined effort, to find a solution to the problems related to this 'feature'.

    And where were the UX designers and roles such as sitebuilders in this?

    I have the outermost respect for a lot of Drupal developers, core developers in particular, and the coding skills you all have. However I don't see the same respect for the skills and experiences in the other direction. For the soon 5 years I have been a member of this amazing community little progress has been made in improving the development process.

    Despite that virtually everyone knows about the problems with the Drupal learning curve, our usability and other UX problems, in the end code rules...

    Every day that passes I am only getting more and more convinced that we are heading in the direction I wrote about a few months ago in my Don’t Make Drupal the New ‘Microsoft Office’! post.

    The politics in this issue only reaffirms that. Makes me really sad.

    Separate modules for functionality and UI

    Both in core and with many contrib modules it is quite common to separate the functionality and the config UI. Field UI is one of those. This was presented to the community as a good thing as it allowed us to turn on&off the configuration UI at will. Something especially good for production sites.

    That's just another thing we users have been told is good about being able to disable modules...

    @tsvenson, I fail to see any kind of logic in your comment.

    A _whole bunch_ of the _smartest_ core developers in this community tried, in a combined effort, to find a solution to the problems related to this 'feature'.

    And where where the UX designers and roles such as sitebuilders in this?

    In order to work on the UX or site-builder experience for "something", we still need to be able to do that "something" technically, no? The problem we have here is that we couldn't.

    Both in core and with many contrib modules it is quite common to separate the functionality and the config UI. Field UI is one of those. This was presented to the community as a good thing as it allowed us to turn on&off the configuration UI at will. Something especially good for production sites.

    That's just another thing we users have been told is good about being able to disable modules...

    That has nothing to do with the enable/disable functionality, it's purely install/uninstall which is not going anywhere.

    We are throwing around the term 'data' a lot here in regards to data integrity.
    And I think we need to be careful about what we mean by 'data.' In the posts above, we are mixing differnt 'data' types and not fully understanding what is meant by 'data' in each case.

    In Drupal, 'data' can mean 3 different things:
    1.) Configuration Data
    2.) Content Data
    3.) Relationship Data

    (1) Configuration Data is being pretty well fixed by CMI (not an easy task - kudos to the CMI team). It is now easier than ever to save, backup, & restore configuration data with new new features introduced as part of CMI.
    Suggestion: When disabling/uninstalling a module, a warning or option to save config data should be presented to the user during the uninstall process.

    (2) Content Data is largely atomic, but not entirely easy to save, backup, & restore. Up til now, Backup & Migrate and/or the Feeds module were used for content backup & restoration (as well as migration). But, the process is far from user friendly.
    Perhaps a better method should be included in core to backup and restore content (easier said than done, I think).

    (3) Relationship Data is probably the hardest to grok & the hardest to handle in terms of this issue. And relationships might be considered Configuration, but I don't think so. Relationships can be made between similar Content Data types (node-node) or different data types (node-user) or those relationships can be made on the entity level (I think).

    I think one of the factors that has fired up this entire discussion is the oft-quoted drupal phrase:
    "[during upgrades] we may break your code, but we keep your data safe."
    That statements depends on how you interpret 'data' and 'safe.'

    I understand this is a really complex issue that has probably stemmed from code debt starting all the way back in Drupal's roots.
    I appreciate all the thought and effort that went into deciding to "Remove the ability to disable modules." I know it is not an easy decision, and I am sorry for questioning that decision. But, that is ultimately what makes Drupal Drupal. The ability for ANYONE to provide input and contribute in any way they can, even if that is only to help steer and not do the real coding.

    In this case, my feeling is that we are swatting a fly with a sledge hammer.

    Disclaimer:
    I'm really not very fond of removing the disable functionality from core. But I can see the serious problems with the current implementation: I see the inconsistencies and workarounds, I see the many issues and I see the impossibility to ensure data integrity in all, untestable and unforeseeable scenarios.
    And I understand that these problems are not results of "Drupalisms" but of the immensely flexible yet complex interfaces of our framework, where many if not most modules of our ecosystem are highly interacting with each other and not just simple plug-ins like in Wordpress. This is both our strength and weakness.

    Where we are:
    But while OOP adoption, improved testing, and the integration of some widespreach contrib modules into core may help us make Drupal 8's APIs less error-prone, there is always a rest risk. Some of the most experienced (and most burdened!) core developers said they don't want to keep bearing these situations and all the work they're leading to. I very much respect that, even though this doesn't have to be the last word. But I agree that this means the starting point of renewed efforts needs to be the removal of the functionality.

    Now if we still want to reintroduce something like disabling modules, what we can do is collect and aggregate these data-integrity related situations by type, and then try to find solutions for the most frequent types. The question can't be whether it is possible to ensure data integrity on disabling modules, but in which cases it is and to which extent and to which price.

    The fact that our most experienced developers are so much burdened and overloaded leads to the question how small the bottleneck has become, and how we can widen it again. But that's another issue.

    Proposal:
    That being said, I can only agree to the other commenters that it is really hard to read all comments plus the linked issues, not necessarily because the pro- and con-arguments are so complex, but because they're reiterated back and forth and mostly only touched-upon but not run through from A to Z.
    The fact that all of this happens in a mostly very emotional tone where both sides feel offended, is very understandable but really might end in a "role-war".

    So at this point, more discussion doesn't seem to make the points clearer, yet hardens the front lines. And as every possible role conflict is involved here, it is a sore point that can't simply be put to silence either.
    Therefore I believe we should for now shift our efforts away from discussion in order to collectively summarize, sort and put together all relevant aspects into an unexcited, prosaic and largely objective writeup, possibly replacing and improving the current issue summary, which currently fails to reflect the whole thread of discussion.
    As soon as all alternatives and arguments are before us (and newcomers popping in), I believe we are in a better situation to turn back to a respectful and effective evaluation and discussion on The Best Way™.

    [edited: "Where we are" heading, respect; removal as starting point; bottleneck question]

    Don't we have 2 clear paths here?: modules that deal with data, and modules than do not deal with data. If the module do not deal with data, you can be disabled, if you do deal with data, you should delete it all before you can be disabled. This means that the user will always be responsible for their data, and that Drupal will not allow them to accidentally delete their data or allowed it to become stale, as they would have to consciously delete it, back it up, or whatever else, before disabling the module.

    oops

    The issue summary is pretty good as is. "I don't like the conclusion so let's talk more" is how work doesn't get done.

    @tsvenson:

    And where were the UX designers and roles such as sitebuilders in this?

    I refer you once again to: http://www.garfieldtech.com/blog/experts-opinions

    To be blunt, UX designers were not consulted on the question "is it possible to support disabled modules without losing data" because A UX designer has nothing particularly to offer that discussion. Just as if you're discussing the color pallet for a new core theme, please ignore me (almost) entirely. I have nothing particularly to offer that discussion. (Sure, there's plenty of in-between cases where both devs and UX folks are relevant and should be consulted. This isn't one of them.)

    "do we want to support disabled modules?" Sure, everyone has an opinion there, and I think we all want to. But that's not the important question. The important question is "can we", and the answer to that is "so far, looks like no". "Might we be able to see a better partial solution after we rip out the unsupportable crap we've got now?" Maybe. There's only one way to find out.

    Is removing disabled modules a usability regression? Yeah, probably. Is there an alternative? No, not really. Does it suck? Yeah, quite a bit.

    Will whining that "we're making Drupal harder" change that it's not possible to support disabled modules in their current form, or that everyone who's tried is now firmly convinced we need to rip it out for our own sanity, *and* most have now unfollowed this issue because of the unproductive whining that it contains? No.

    I really, really hate the "talk is silver, code is gold" mantra, but right now it's quite applicable.

    count me out of testing contrib or core modules if I'm not offered an easy way to enable/disable them at will

    @klonos: I value all the testing you've done on Drupal and your generally thoughtful and insightful comments on issues. I'm confused about this one though. Drupal core will still provide an easy UI to uninstall a module (given #219, perhaps that hasn't been made clear enough and is causing some of the frustration?). For purposes of testing, what's the inconvenience in that? Also, a module like devel.module could provide for a non-immediately-destructive disable, if that turns out to make things easier for testers. The flaw with it would be the same as the flaw that's in D7 core already: that over time, you will eventually lose parts of that data, and your site will start having data integrity bugs popping up. Which I assume is not that big a deal for testing purposes, because I guess I'm assuming that you'd be doing this testing on a local copy of a site, not directly on your production site.

    I'd be terribly disapointed if I heard one more time "but contrib will do for you": it's giving away responsabilities, it's false, and it's leaving a very delicate topic to potentially unstable and unmaintained code

    The code in core has turned out to be unstable. I'm not claiming contrib will make it more stable. I'm claiming that given that it's unstable already, and there's no viable patches yet submitted that make it stable, that contrib is the better place for that unstable code to reside in. If you're using a devel.module feature, you have a different expectation from it than when it's a core feature. And in this case, that's the expectation we want people to have, because it aligns with reality. But also, sometimes, really amazing solutions to seemingly intractable problems do emerge from contrib. We can't count on it, but it does happen sometimes. And if a production-site-worthy disable implementation is figured out in D8 contrib, then I'm sure lots of people here would support it getting added into D9 core.

    @effulgentia:
    Mostly ACK.
    Just think that this issue isn't compelling enough in convincing many more Drupal users to come that this has been the only and right solution. I can respect the decision and live with using devel, but if we don't take serious the existing complaints, we'll reap much more complaints at a later point in D8's development process.

    @Crell:

    The issue summary is pretty good as is.

    For a controversial issue: nope. The summary offers Proposed solution A and a "mostly obsolete" Proposed solution B, as if there was only one option, leaving out alternatives that have in fact been proposed here such as:

    • Allow some modules to be disabled, others not.
    • Add more "bandaids" as FieldAPI does.
    • Provide a generic mechanism to disable the UI only.
    • Make disabling a module a bit less convenient, so the user notices

    There are arguments to dismiss them all, but at least we need to mention alternatives that have been brought up.
    But fine, no tag needed to work on the issue summary.

    Sure, there's plenty of in-between cases where both devs and UX folks are relevant and should be consulted. This isn't one of them.

    Not correct and not respectful to UX people. UX is much more than the theme's color palette. Removing user-facing functionality which has been around ever since and that is at least frequently used by lots of people, is very much a UX issue. If devs put a veto on fixing more issues with disabling, and there is no convincing alternative, okay, that might be the result. But it's not like this fundamentally was a core devs' only issue. Nevermind.

    "I don't like the conclusion so let's talk more" is how work doesn't get done.

    Paraphrasing whom? Work is currently being done on #2003966: Helper issue for #1199946 : Disabled modules are broken beyond repair, so followup discussion can continue here.
    But okay. If there should be no more talking here, then we can stop talking here and close this issue as soon as the helper issue is committed. Everything else will then be discussed (and ideally coded) in followup issues. Don't know if that's better, but maybe it is.
    Unfollowing, too.

    @Crell:

    To be blunt, UX designers were not consulted on the question "is it possible to support disabled modules without losing data" because A UX designer has nothing particularly to offer that discussion. Just as if you're discussing the color pallet for a new core theme, please ignore me (almost) entirely.

    Seriously Larry, are you actually saying that your understanding of what UX design is that it limited to discussing color pallets and maybe other forms of UI prettyfications?

    Even if you believe it is more, that paragraph alone gives me complete clarity about why things are as bad as they are.

    I highly recommend you to spend a few hours to read up on what UX really is. This article from Computerworld: Tech hotshots: The rise of the UX expert is an excellent start and should only take you a few minutes.

    Here is a quite from it:

    In design parlance, the user interface (UI) is what the user sees; the user experience (UX) is how the application behaves.

    tsvenson: My batchelor's degree is in Human-Computer Interaction. I know full well that design and UX involves far more than picking colors. I very specifically called that out as an extreme end of the spectrum (in the part of that paragraph you didn't quote). *Most* decisions should involve input from both well-informed developers and well-informed UX folks to varying degrees. But not all.

    Take for example a solid, first-rate designer like Mark Boulton, or UX expert like Leisa Reichelt. Both are highly skilled and good at what they do and Drupal is unquestionably better for their involvement. And what value do they offer to the question "is it technologically feasible to offer disabled module functionality?" Absolutely none. "Would it be helpful to users?" Sure! But that's not the question on the table.

    My point is simply that your line about "were UX designers consulted before making this decision?" is completely irrelevant, and belies a lack of understanding of the question at hand. "How useful is this" is completely irrelevant in every conceivable way if the answer to "can it be done" seems to be "no".

    I return again to my original point: Many brave and wise developers have spent hundreds of hours trying to make this functionality work in modern Drupal. They've failed, and Dries has agreed that we should cut our losses. "Try harder" is an insulting statement to make.

    See comments #99, #120, and #130 where I also argued to keep "disabled". But I trust that if catch, sun, swentel, and heyrocker (and others) can't get it to work, it's not going to work in Drupal 8. Better to pull it out than to leave a system with major data loss bugs in place.

    Might it be useful to have the proposed solutions updated to indicate intended / expected results should a module's files be deleted prior to uninstalling?

    Some semi-random thoughts (admittedly haven't read the full backscroll in detail, so maybe some are redundant):

    First, the install/enable (and uninstall/disable) terminology has always been confusing. The admin UI presents enable/disable as the primary actions to perform on modules, with Uninstall on a separate tab most people never look at. We've got the set of four hooks which module maintainers should be familiar with, but I suspect many are confused by the distinctions and don't always know what should go into hook_install() vs. hook_enable(). The concept of "install" as distinct from "enable" doesn't even exist in the UI - site admins have no idea that the effect of turning on the Enable box and clicking Save configuration differs according to the module's previous enablement state. A minor WTF this proposal should clear up, compared to the data integrity issues, but a WTF nonetheless. Collapsing to a single on/off switch for modules is definitely a positive thing in my mind, but we should make sure the UI language and API language match. As proposed in the summary the API keeps the install language in the API - I think the enable/disable language is clearer so would prefer that the surviving hooks be named hook_enable and hook_disable, but if we must go the hook_install route then the UI and documentation should talk of installing/uninstalling modules.

    So, as to the fundamental issue, which is the desire to temporarily turn off a module with the ability to turn it back on with the configuration and data it had previously... Rather than the old model of leaving that stuff sitting around the Drupal environment, what if we addressed this with a backup/restore model, with the modules taking full responsibility for the backup and restore? Modules maintaining configuration and/or data would implement one or both pairs of hook_backup_configuration()/hook_restore_configuration() and hook_backup_data()/hook_restore_data(). Assuming we're using enable/disable language, when the admin requests disabling a module, if one or both backup hooks exists, they'll be prompted "Backup @modulename configuration?" and/or "Backup @modulename data?", and the hook(s) invoked for any Yes answers, with the answers remembered. All data/config for the module would be removed from the active environment either way.

    When the admin asks to enable a module which was previously disabled, and we recorded that backups were taken, we would prompt whether they want to restore the config/data. The prompt might have some generic text about the risk of data integrity issues (and maybe let the module insert any specific caveats). If Yes, call the module's hooks to perform the restoration.

    The basic data integrity issue still remains - at restore time the system may have evolved out from under the state that existed at the time of the backup - but I think this approach helps mitigate it:

    1. It would no longer be the default that the data would sit around in the Drupal environment rotting - it will be explicitly set aside, so within the environment integrity is maintained.
    2. Today, WTFs can arise if a module is left disabled for a while and re-enabled where the site admin didn't want the old data, but never did an uninstall. The explicit restore request on re-enablement addresses this.
    3. Module authors today rarely think of taking any actions in hook_enable() to maintain/restore data integrity after a period of disablement - this approach forces consideration of "What does the environment I'm restoring into look like? What assumptions can and can't I make about its state compared to when I did the backup?".

    The basic support for this in core would be fairly straight-forward, I think - but there would have to be work to implement the backup/restore hooks for core modules. I think it should be pretty simple for core to provide backup/restore services for configuration. How/where to backup data, however, would be the main sticking point...

    Thoughts?

    Pancho, I agree. The problem with the current issue summary is that it has some vague generalizations, plus a huge list of links. The links are great but most people aren't going to have time to read through and understand them to meaningfully evaulate the pros/cons and consequences here. They need to be grouped into categories and summarized.

    I wrote above - "I've been doing some research and I'm going to have more comments (of a technical nature) when I have some time later" - basically I'm trying to read through, understand, and summarize the actual technical issues. It looks like it's going to take a lot longer than I hoped though, even though I was already familiar with many of them. But it will happen at some point.

    I think UX experts can absolutely contribute to this issue by evaluating the technical reality and helping to determine how severe the consequences are to real users, and whether (from a user perspective) it really is "broken beyond repair" or to what extent some of our problems come from poor expectations in the user interface. For example, there have been a number of proposals in this issue about ways we could present this concept differently in the interface, including Michelle's in #61, my comparison to the Synaptic Package Manager in Ubuntu in #160, and parts of mikeryan's suggestion in #227. To use the Ubuntu example again, I can happily use the admin interface on my Ubuntu laptop to remove the PHP package but leave behind its "configuration files" (after going through a confirmation form where I affirm that I really want to remove it). If I leave it off for 6 years and then try to turn it on again, it will probably try to use my old php.ini file, and it's unlikely that every single part of it will work the same way it used to. I may lose some of my previous configuration, but that doesn't mean the whole concept is flawed and I should have deleted it in the first place. I'm not sure most humans really expect computers and other machines to work that way even; if you leave your car parked in your garage for two years and then go out and start it afterwards, there's a good chance it will stall. The second law of thermodynamics exists. Life still goes on :)

    To summarize quickly what's wrong with the issue summary (maybe someone will be inspired to improve these parts in the meantime) it basically lists a few general arguments:

    1. The key concept here is data integrity.
      All data that isn't exclusively owned by a disabled module needs to go through APIs of other modules, and those modules have to make hard decisions in order to keep the system running.

      If you disable a module, data integrity is no longer maintained; i.e., disabling a module means to uninstall it step by step under the hood:
      Field types disappear; all field values get stale.
      Entity types disappear; all references are unresolvable, data is lost.
      Plugin types disappear, and along with it, all plugin IDs; all unresolvable plugin references get orphaned.

      "Data integrity" is a complicated problem and it's certainly not limited to disabled modules, is it? And orphaned data is not a huge problem in a world where disk space is cheap, is it? (Plus if you really want to clean up the orphaned data, you can uninstall.)

    2. Configuration, content, and data of disabled modules cannot be staged. The system is unable to know how to deal with the data, since it is impossible to serialize/export and import it.

      It's not clear why you'd want to stage content and configuration owned by a disabled module in the first place?

    3. The current transitional disable » uninstall state in itself means that the API of a module is not invoked when its data is deleted, which consequently means that data integrity gets broken in enabled modules.

      That's definitely a problem, but it's an argument for removing the "disable » uninstall" transition, not for removing the whole concept of disabled modules.

    4. Users are currently required to use the dangerous facility of disabling modules and the system makes it look as if it was a safe and sane thing to do.

      That's an argument for improving the user interface, as has been discussed above.

    Some of my questions above are rhetorical rather than actual, but hopefully it illustrates why the issue summary doesn't really justify the proposed change.

    I've spent the entire afternoon reading every comment in this issue and read enough of every linked/related issue to get a decent grasp of those issues. I have to say I'm pretty mortified.

    I hope that my comments aren't disregarded (or seen as in anyway disrepectful) because of my lack of experience/expertise given I'm not a UX professional nor am I a developer. I am probably the typical Drupal user/sitebuilder. I'm a designer with some basic PHP knowledge and have used Drupal for around 2 years now on a daily basis and have completed around 20 or so projects of varying complexity.

    I'm hoping that my perspective may be helpful in bringing together a clearer understanding of how to reach a consensus about this issue as I think it may be a case of conflicting priorities which is causing this issue to be a bit polarising.

    How I have understood/used this feature

    And the module system in general
    In my experience and knowledge it is perfectly fine to disable a module whenever and enable it again at will. When I install a module I will be told if I have missing dependencies and I am told if there are dependent fields when I try to disable a module which has fields is use. That indicates to me that the module system has checks and measures in place to prevent or warn me from making decisions which could damage my site and as a Drupal user I know that key warnings are presented appropriately (when running update.php or a backup_migrate for example).

    So how could unchecking a box which presents no warnings potentially result in possible data loss?

    In my opinion the only solution is to remove the disable module functionality from the core.

    Why:

    • I chose Drupal for: Security, Stability and Productivity/Features in that order. To me a piece of code in the core means it is stable, secure and most importantly safe. To find out today that I may have some serious data integrity issues on my live sites and those in development because I have used a function which by all accounts is presented to me as a safe, ordinary feature, is really painful.
    • The less experience with Drupal you have the more modules you will install, try out and replace while looking to achieve a desired result.
    • Currently there is nothing in the UI which indicates to a user that they need to, or should uninstall a module.

    As many have mentioned quite a few times in previous comments, this really is not debatable. The only way forward I see with this feature is if there are some significant changes are made to how this feature is presented to users and there is quite a substantial effort is made to document the risks/issue.

    If this feature is available it should be in a contrib module.
    Personally I have never found a bug in the core (excluding IE9's Helvetica bug). I've barely even searched the core issue queue because I know, and it always turns out, to be a contrib module which is causing a problem. The documentation and as a user getting to know Drupal, it is very clear that contributed modules come with the possibility of risks while the core is stable and secure due the the many eyes, years of development etc.

    Many key development and production features and functionality which almost every Drupal site need are contrib and I don't understand why this couldn't be the case here.

    The UX nightmare (if removed)
    By sharing my perspective and my use I was hoping to create an understanding that this issue has been created by the current poor UX/UI as much as contrib modules have snowballed this to a critical state. It's really a catch 22. Either the current UX/UI issues are resolved so this feature is used more thoughtfully by users or another UX problem is created. A new UX problem is easier to resolve than a UX problem with code instability as well.

    If the status quo is kept uninformed users will continue to create unstable sites and developers will still be haunted by issues reported that are caused by this problem. The worst case result being users will leave Drupal due to instability and developers will get burnt out.

    @Crell and others have made it quite clear this is not able to be resolved in code so ultimately the only way forward is to look at how to work with this feature removed. I don't see how any UX argument could be made against this as I can't see anything more negative to a user than a process which could break their site.

    To Summerise

    There are some incredibly valid arguments and use cases for keeping this feature/function however I believe at the end of the day it is a much better decision for the majority of (common) users, developers and better UX (and Drupal usability as a whole) to remove it.

    #215 Despite that virtually everyone knows about the problems with the Drupal learning curve, our usability and other UX problems, in the end code rules...

    This is so true and in part was what compelled to comment on this issue. As mentioned it's UX/UI which I believe creates a large part of the issue and like so many cases of bad UX/UI in Drupal. It is this determined focus on the code which takes focus away from the actual point - the user. In a comment above it was mentioned that it is sad that Drupal is not done in code anymore and references have also been made about UI being about 'magical buttons', I don't understand this perspective at all. Isn't the entire purpose of a CMS to provide an interface to build and maintain a website?

    Better magical buttons can only help Drupal and it's tireless developers/contributors. This issue is a fantastic example. I think D7 has only been mentioned once or twice in all 220+ comments on this issue and I can't find a D7 issue regarding this. So currently and in the future there are users unwittingly breaking things out there, none the wiser and contributors are trying to fix issues which are created by this when all that is needed is some sort of message, warning or something, anything to indicate to users that they should uninstall the module they have just disabled (or are about to disable).

    We are however told:

    Regularly review and install available updates to maintain a secure and current site. Always run the update script each time a module is updated.

    So if a user is looking for the right module for function X and they're first choice doesn't quite cut it, they will probably install an alternative, uncheck the unwanted module, check the new one, hit 'Save configuration' and then run update.php.

    Bugger.

    Title:Disabled modules are broken beyond repairDisabled modules are broken beyond repair so the "disable" functionality needs to be removed

    #227 -

    Modules maintaining configuration and/or data would implement one or both pairs of hook_backup_configuration()/hook_restore_configuration() and hook_backup_data()/hook_restore_data()

    The first hook sounds like the Features module + new CMI stuff to me. The second hooks just sound exactly like the existing hooks for enabling/disabling modules that *don't work* (hence this issue). You basically just said "rename some existing mechanisms to be more explicit for module developers" but this doesn't fix the underlying technical problems.

    It's all well and good to say a module "has to be responsible for its data" but here's just one simple hypothetical that illustrates how easy it is for this concept to run into troubles.

    Start with the Comment module.

    Each comment has a sequential comment ID and some other data associated with it.

    You have another module, the Domain module.

    Each domain has a sequential ID.

    Now say you make a module to integrate the two and have a table that lines comments up to domains based on their sequential IDs, you now have a list of what comments are for what domain, and it is pretty darn important that your list of comments against domains stays current or you're going to have content bleeding between domains.

    Now you disable any one of these three modules and perform some task that changes the sequential ID values for one of the still enabled modules, for example, upgrade Domain from D6 to D7 and have "domain 0" be re-numbered to some other ID.

    Now, re-enable the module that you had disabled. The external relationships with this module's data are now all broken and there's just no way for this module to compensate - the vital contextual information it could have used is gone and it missed its chance to react while it was disabled. Your site is now broken, and broken in a way that may go unnoticed for a while but then be fiendishly hard to debug and potentially even harder, or impossible to fix. Is this type of configuration of modules rare? not at all.

    These problems only get worse when you consider multiple related modules being disabled, re-enabled and upgraded at different times and in different orders. There're tougher issues to iron out than just sequential ID values not matching.

    You can't just "export" this stuff because as soon as you stop triggering a module's hooks responsible for keeping data in sync *it goes stale* equally fast whether it's in the database or in a .zip file on a backup drive in your garage. If it was just about straight export/import at the moment a module is enabled/disabled then we wouldn't have half as many problems as we do.

    Module maintainers can't even provide a "list of caveats" because it might be some other module's data (that the maintainer didn't write) that is now corrupted thanks to you disabling and re-enabling it. To even try to build such a list of caveats is again lying to ourselves by saying we can know something that we simply can't.

    Throwing a warning saying "oh, if you use this feature there might be some data integrity issues but maybe there isn't, but we don't know what they are and they're going to get steadily worse over time, or not" is not good enough, it's totally irresponsible, it's like putting a small warning label on draino and selling it alongside vegetables - I hope none of you would ever want to build features that are this unstable for a client!

    Saying "oh, well developers should be making backups, right?" also isn't good enough. The bugs are in the very fact that the database has stale data, you aren't rolling back to the backup you made before you re-enabled the module a few hours ago, you'd have to roll back to the backup you took before the module was disabled in the first place - which could be weeks or even months ago - it could be before the site was even launched if a bug from stale module data was introduced during debugging in the initial development phases!

    I think we all need to take a step back and remember that disabling modules is *not* vital to Drupal's operation or even one of Drupal's main selling points, it's a *convenience* feature for developers but it is very clear from this thread that it is more dangerous than useful and the overwhelming majority of users don't even realise that it's dangerous.

    As has been pointed out multiple times in this thread already, many contrib modules (including ctools that provides a great template for achieving this functionality alongside "exportables") have implemented their own mechanisms for disabling *components of their configuration* wherever it is robustly safe to do so, and this has proven to be a very effective compromise between the current broken core system and the obviously undesirable "everything always on all the time" alternative (see the above comments about how Views works). This would only become a more prominent and common design pattern if the crutch of the core "disable" was removed as it would force *every* contrib module developer to consider these issues as it relates to their module (which is a good thing if this maintainer is writing a module that implements its own database tables).

    +1 from me to ripping "disabled" out of core. Also a bonus +1 if we can uninstall a module's data and config separately while the module is still enabled (factory reset kind of thing).

    As pointed out eloquently in #230, one of Drupal's biggest *actual* selling points is reliability and stability. I truly believe that if "disabled modules" (disabled themes, too btw) was proposed as a new feature request today in the form it takes in HEAD currently, it would be marked as "won't fix" almost immediately.

    I think we're all pretty much agreed that disabling modules is a developer tool, to be used for debugging.

    So we should distinguish between:

    a) Uninstalling a module, which removes all its data.
    b) Disabling a module, which does not.

    Given disabling is only a developer tool, then it seems to me that a lot of the data integrity issues go away if we define 'disabling' as being an operation that puts the site into an unstable state, for development purposes only.

    So the problem of 'what happens if you create entities when some of the field types are missing' becomes irrelevant, because the answer to that problem is, 'you either can't do that, or you should expect it to blow up -- either way, you should be doing that on your development copy'.

    This is the workflow I'd envisage:

    1. disable one or more modules
    2. your site is now *forced* into maintenance mode. Obviously, for UX, we'd have to add a confirmation step for this, or move the module disabling UI away from the main module UI.
    3. the only way out of this state is:
    A. re-enable all of the disabled modules
    B. complete the uninstallation of all the disabled modules
    C. there is no option C.

    (Aside: on the matter of 'your views break if the modules that provide the plugins go away', surely that is just a true with a complete uninstall of the plugin module! So removing the disabling functionality doesn't make this problem go away, just makes it a little less likely it'll be encountered.)

    @dkre.one: thank you for a really great explanation from a non-guru site builder's perspective on why this is a worthwhile goal. I also think that the problems mentioned from having this functionality removed can be mitigated via some improvements to the UX and documentation.

    Given disabling is only a developer tool, then it seems to me that a lot of the data integrity issues go away if we define 'disabling' as being an operation that puts the site into an unstable state, for development purposes only.

    And is an excellent reason why this should be implemented by something in contrib (like the Devel module), not core.

    There is no other operation you can access through core's UI that will willingly put you in a position where you can end up destabilising your site. Risky operations like this are always pushed out to contrib.

    > There is no other operation you can access through core's UI that will willingly put you in a position where you can end up destabilising your site. Risky operations like this are always pushed out to contrib.

    Well with that logic, core should drop the functionality that lets you enable any non-core module!

    Well with that logic, core should drop the functionality that lets you enable any non-core module!

    Please don't be facetioius about what I'm saying here. Extending Drupal through modules is a vital part of making sure that Drupal is a useful framework and is clearly more than just a "developer tool". Having a "disabled/uninstalled" split for modules is not vital and not safe.

    When core enables a module we don't *know* that it will destabilise the site, there are checks and measures in place to ensure a reasonable level of quality for at least all the modules on d.o and you can always uninstall the module, which *should* put your site back to whatever state it was in before the module was enabled.

    When core disables a module we *know* that any data associated with it will immediately start going stale. The only thing we don't know is how damaging this is to the integrity of the site.

    The logic behind the two situations is not equivalent simply because both operations entail some amount of risk and you know that's a ridiculous thing to say.

    Even with the proposed workflow in #232, there's nothing stopping a cron run or update script or similar doing just as much damage as if the site was not in maintenance mode. There should not be two options available, as re-enabling the modules is not really a safe option (and now we'd *really* be implying that it is), the only way out should be to complete the uninstallation of the disabled modules - at this point, why do we even have this workflow?

    Status:Active» Needs review
    StatusFileSize
    new178.54 KB
    PASSED: [[SimpleTest]]: [MySQL] 54,181 pass(es).
    [ View ]

    This patch does the following:

    • Deprecate module_enable/module_disable (Main reason for not entirely removing it: Drush still builds on it. Nothing in core still calls it though so we might just as well rip it out if everyone is fine with updating Drush again :) ).
    • Refactoring ModuleHandler::enable // ModuleHandler::disable // ModuleHandler::uninstall to just be ModuleHandler::install/uninstall. Disable + uninstall have been merged, enable has been renamed to install. Both have been cleaned up.
    • Added a hook_module_preuninstall() // hook_module_preinstall() to allow for proper clean-up of stale module date before actually uninstalling it.
    • Removing the 'disabled' module entry from state().
    • Move all invocations of module_enable/disable to module_install/uninstall.
    • Removing 'disable' module tests and replacing them with module uninstall tests where applicable.
    • Adding generic entity bundle purge code in entity_module_preuninstall().

    I created a meta issue as a follow-up for clean-ups and architectural fixes around ModuleHandler. Let's do the clean-ups (especially for ModuleHandler) as part of that. Yes, the code there currently breaks many rules but that was not introduced here and this issue is not about cleaning it up. We should get this in as quick as possible so we can start to work on things like proper field data purging and what to do with things like field_sync_status().

    Issue summary:View changes

    Updated issue summary.

    - There seems to be a removed comment test method that is supposed to be re-added as a separate class/in a separate class?
    - Maybe also install/uninstall comment.module in InstallUninstallTest, as it's an especially funny module?
    - module_install(array('system_test')); in VersionTest.php is not necessary, enabled in base test class.
    - Unecessary change in testModuleVersions()
    - $disabled_modules = array(); <--- Should be $installed_modules or $uninstallable_modules?

    As mentioned before, I do not see a solution for 7.x to 8.x upgrade yet, where we do need to handle the fact that a lot of modules might no longer exist, and the current instructions tell you to disable all modules. The update module handler helps, as we no longer invoke hooks on them, but it's still scary. We might not need to solve it here completely, but we probably need at least a plan, need to update UPGRADE.txt and possibly also a basic test that adds a non-existing/disabled module? In general, what happens with modules that were disabled in 7.x when you upgrade? What happens if you enable it afterwards in 8.x?

    The upgrade should refuse to proceed if you have disabled modules. You need to either enable them or uninstall them.

    StatusFileSize
    new178.51 KB
    FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1199946-240.patch. Unable to apply patch. See the log in the details link for more information.
    [ View ]
    new5.51 KB

    Fixing the things mentioned in #238

    StatusFileSize
    new178.52 KB
    PASSED: [[SimpleTest]]: [MySQL] 54,274 pass(es).
    [ View ]

    Patch did not apply anymore.

    Issue tags:+D8 upgrade path

    Tagging with upgrade path.

    Another related bug #2032369: _update_8003_field_create_field() relies on field schemas never changing and plugins being available - upgrade of disabled modules does not add namespace to discovery so all plugins are not accesible

    Issue summary:View changes

    Updated issue summary.

    Assigned:Unassigned» Pancho
    Status:Needs review» Needs work

    Uhh, the patch is really hard to review.
    But thank you for working on this huge patch! We most importantly want to get it committed very soon, before we need to reroll it again and again. Escpecially now that you managed it to pass all tests.
    Still I'm not sure we're already there.

    There are so many more occurrences of the word 'enable' or 'enabled' in code, including function names, parameters, config keys, internal variables and a huge load of comments. And while some are referring to themes or other things that can still be enabled, many are referring to modules.
    Same with 'disable' or 'disabled'. But attention: some of the remaining occurrences of 'disable/d' are pointing to code being superfluous now that we don't have the 'disabled state anymore. Good example is update_retrieve_dependencies(), which is "similar to module_invoke_all(), with the main difference that it does not require that a module be enabled to invoke its hook, only that it be installed." So we really need to look at them one by one, and not just find-and-replace.

    Also, the UI at least needs to be improved. No indication why so many checkboxes are locked, and on the Install tab the help text still says: "To uninstall a module, you must first disable it on the main Modules page."
    Then there are loads of inconsistencies like "You must enable the Devel module to install Devel node access."

    We might want to pick some of the low hanging fruit, ensuring everything is in an acceptable intermediate state and avoiding that some lines will never get fixed. So while we should defer a larger UI overhaul and some specific edge-cases to followup issues, we should also make sure the patch gets committed only after these followup issues are created.

    If you don't mind, I'm going to work on this tonight. I've already gone through a third of the files touched by this patch, and would go on with the remaining files plus potential repercussions on the rest of all core files. Hopefully I'll end up with a new patch passing tests just as well and with a list of followup issues top create.

    Thanks for looking at the patch @Pancho. I did not touch every occurrence of disable/enable, etc. in the documentation on purpose because that would've bloated the patch beyond the 500kb line. We don't want that. It's hard enough to review it as is. We can have a follow-up that purely touches documentation all over the place and fixes it. I made sure that I only touched documentation if I was editing the file/class/function/method anyways. I talked this through with both @alexpott and @catch and we agree that the documentation rewrites should be follow-up(s).

    The concerns you just raised about the disabled checkboxes are valid, but we should not derail this issue into a full UI rewrite either, especially since I already started re-factoring the UI in the WSCCI conversion issues (I worked on the install page, will work on the uninstall page after that's finished). And since those issues are critical, we wont miss them. Promised. Pinky swear.

    #1990544: Convert system_modules() to a Controller
    #1993202: Convert system_modules_uninstall() to a Controller

    Also, the UI at least needs to be improved. No indication why so many checkboxes are locked, and on the Install tab the help text still says: "To uninstall a module, you must first disable it on the main Modules page."
    Then there are loads of inconsistencies like "You must enable the Devel module to install Devel node access."

    Oh, right.. Yeah, that's one of the things I think we should fix right here. That makes sense... Will you do that?

    Status:Needs work» Needs review

    Let's leave this at NR though so others can review it too. We really need a few good reviews here.

    Issue summary:View changes

    reorder followups

    StatusFileSize
    new178.67 KB
    PASSED: [[SimpleTest]]: [MySQL] 54,821 pass(es).
    [ View ]

    Chasing head.

    Posted a follow-up issue for discussing the UI:

    #2035079: Figure out what to do with the install/uninstall modules page

    Issue summary:View changes

    Updated issue summary.

    Issue summary:View changes

    Updated issue summary.

    Self-review incoming :). I can fix these tomorrow/sunday or someone else can do it instead if you beat me to it.
    Note that all the things listed below were not introduced here but since we touch all these things we can just as well fix them right here too...

    +++ b/core/includes/install.core.incundefined
    @@ -992,7 +992,7 @@ function install_base_system(&$install_state) {
    +  module_install(array('user'), FALSE);
    @@ -2028,7 +2028,7 @@ function _install_module_batch($module, $module_name, &$context) {
    +  module_install(array($module), FALSE);
    @@ -2522,7 +2522,7 @@ function install_configure_form_submit($form, &$form_state) {
    +    module_install(array('file', 'update'), FALSE);

    All these could be moved to \Drupal::moduleHandler() calls directly I guess as we are changing them anyways.

    +++ b/core/lib/Drupal/Core/Extension/ModuleHandler.phpundefined
    @@ -719,185 +684,150 @@ public function enable($module_list, $enable_dependencies = TRUE) {
    +      \Drupal::service('kernel')->updateModules($module_filenames, $module_filenames);

    That should be $this->container->get('kernel')

    +++ b/core/lib/Drupal/Core/Extension/ModuleHandler.phpundefined
    @@ -719,185 +684,150 @@ public function enable($module_list, $enable_dependencies = TRUE) {
    +                  $factory = \Drupal::service($definition['factory_service']);

    $this->container->get($definition['factory_service']);

    +++ b/core/lib/Drupal/Core/Extension/UpdateModuleHandler.phpundefined
    @@ -70,16 +70,13 @@ public function enable($module_list, $enable_dependencies = TRUE) {
    +      $module_config = \Drupal::config('system.module');
    @@ -101,7 +98,7 @@ public function enable($module_list, $enable_dependencies = TRUE) {
    +        $config_storage = \Drupal::getContainer()->get('config.storage');

    Considering that we are touching it anyways we could just as well inject the config services there.

    +++ b/core/modules/system/lib/Drupal/system/Tests/Module/InstallUninstallTest.phpundefined
    @@ -0,0 +1,180 @@
    +        $this->assertText(t('hook_modules_installed fired for @module', array('@module' => $module_to_install)));
    ...
    +        $this->fail(t('Remaining modules could not be disabled.'));
    ...
    +    $this->assertText(t('The configuration options have been saved.'), 'Modules status has been updated.');
    ...
    +    $this->assertText(t('The selected modules have been uninstalled.'), 'Modules status has been updated.');
    ...
    +    $this->assertText(t('hook_modules_uninstalled fired for @module', array('@module' => $module)));

    There are a lot of tests that use t(). We did not introduce that here, but since we are touching/copy&pasting that stuff we could just remove those.

    THis seriously improves the maintainability of the module handler a lot! Great work.

    The only wrong line in the test is the following:

    +        $this->fail(t('Remaining modules could not be disabled.'));

    All other tests actually use t() to check the functionality, which is fine.
    +++ b/core/includes/module.incundefined
    @@ -233,6 +234,18 @@ function module_uninstall($module_list = array(), $uninstall_dependents = TRUE)
    + * @todo The only reason for not removing this right now is that Drush uses it.

    That is not a valid reason from my perspective. Drush should just adapt the new API of drupal 8.

    +++ b/core/lib/Drupal/Core/Extension/ModuleHandler.phpundefined
    @@ -529,43 +528,53 @@ protected function parseDependency($dependency) {
    -  public function enable($module_list, $enable_dependencies = TRUE) {
    ...
    +  public function install(array $module_list, $enable_dependencies = TRUE) {

    This patch would be so easier to review if there would have not been a rename from enable to install. Then the follow up would also be trivial to review.

    +++ b/core/lib/Drupal/Core/Extension/ModuleHandler.phpundefined
    @@ -529,43 +528,53 @@ protected function parseDependency($dependency) {
    +    if (array_diff_key($module_list, $module_data)) {
    +      // One or more of the given modules doesn't exist.
    +      return FALSE;
    +    }

    There could be a follow to discuss whether there should be an exception thrown so it is for example clear which modules could not be installed.

    +++ b/core/lib/Drupal/Core/Extension/ModuleHandler.phpundefined
    @@ -529,43 +528,53 @@ protected function parseDependency($dependency) {
    -      if (!$module_list) {
    -        // Nothing to do. All modules already enabled.
    -        return TRUE;
    -      }

    In the new code the hooks are executed even no modules are actually installed.

    +++ b/core/lib/Drupal/Core/Extension/ModuleHandler.phpundefined
    @@ -719,185 +684,150 @@ public function enable($module_list, $enable_dependencies = TRUE) {
    +  /**
    +   * Helper method for removing all cachebins registered by a given module.
    +   *
    +   * @param string $module
    +   *   The name of the module for which to remove all registered cache bins.
    ...
    +  protected function removeCacheBins($module) {

    Good idea to move this to a helper method.

    +++ /dev/nullundefined
    @@ -1,73 +0,0 @@
    -/**
    - * Tests enabling modules.
    - */
    -class ModuleEnable extends WebTestBase {
    ...
    -  public static function getInfo() {
    -    return array(
    -      'name' => 'Module enable',
    -      'description' => 'Tests enabling modules.',
    -      'group' => 'Module',
    -    );

    How annoying. Didn't git diff 8.x -M worked?

    That is not a valid reason from my perspective. Drush should just adapt the new API of drupal 8.

    Yeah, I will remove that as soon as this patch is RTBC so that people have an easier time testing this until then.

    How annoying. Didn't git diff 8.x -M worked?

    Nope, that test is horrible, absolutely horrible. And it's the longest running test I faced so far, I've nightmares from this test at this point. We changed too much in this file to make -M work. Anyways, I am not even sure why this test runs in the UI and not simply through direct invocation of the ModuleHandler. Once we cleaned up the ModuleHandler up and maybe introduced separate services for installation, discovery, etc. and turned the Modules/Themes/Extensions in general into proper classed objects we can potentially even turn this into a UnitTest, who knows. But we really should to be honest... These tests that constantly install/uninstall things take so incredibly long. It would definitely improve Issue Queue productivity if we can manage to increase the testing performance there.

    In the new code the hooks are executed even no modules are actually installed.

    No, the code never reaches that point if there are no modules to be installed/uninstalled.

    This patch would be so easier to review if there would have not been a rename from enable to install. Then the follow up would also be trivial to review.

    Agreed, we discussed this at length though and concluded that enable/disable is not the right term to use here as we are actually doing the full install/uninstall only. Keeping disable/enable would be confusing.

    adding tag.

    Assigned:Pancho» Unassigned
    StatusFileSize
    new5.88 KB
    new177.34 KB
    FAILED: [[SimpleTest]]: [MySQL] Drupal installation failed.
    [ View ]

    Fixing the stuff mentioned in #253 and #254

    Edit: Please disregard this patch... Touched ModuleHandler without actually injecting the stuff I changed there.

    StatusFileSize
    new2.27 KB
    new177.25 KB
    FAILED: [[SimpleTest]]: [MySQL] 54,855 pass(es), 108 fail(s), and 8 exception(s).
    [ View ]

    Re-uploading.

    Assigned:Unassigned» fubhy
    Status:Needs review» Needs work

    Need another re-roll due to #1990544: Convert system_modules() to a Controller getting committed (which is cool).

    Also going to fix the uninstall fails.

    Assigned:fubhy» Unassigned
    Status:Needs work» Needs review
    StatusFileSize
    new188.82 KB
    PASSED: [[SimpleTest]]: [MySQL] 54,979 pass(es).
    [ View ]

    Re-roll.

    First review, will go over it again once more tomorrow and also do manual testing. A couple of nitpicks, apart from the first one.

    +++ b/core/includes/module.incundefined
    @@ -201,29 +201,30 @@ function module_load_include($type, $module, $name = NULL) {
    + * @todo The only reason for not removing this right now is that Drush uses it.

    I agree with dawehner, drush needs to follow core, not the other way around.

    +++ b/core/modules/system/lib/Drupal/system/Tests/Module/InstallUninstallTest.phpundefined
    @@ -0,0 +1,180 @@
    +          // Add any potential dependency of this module to the list of modules we

    80 chars

    +++ b/core/modules/system/lib/Drupal/system/Tests/Module/InstallUninstallTest.phpundefined
    @@ -0,0 +1,180 @@
    +      // Check that each module is not yet enabled and does not have any

    'not yet installed'

    +++ b/core/modules/system/lib/Drupal/system/Tests/Module/InstallUninstallTest.phpundefined
    @@ -0,0 +1,180 @@
    +    // - That each module can be successfully enabled again after being

    should that be 'installed again' ?

    I agree with dawehner, drush needs to follow core, not the other way around.

    Agreed, but as I said, I will remove it once we are RTBC otherwise. That way, people can easily test this patch while still using "drush site-install".

    This is a massive API change. Given Dries' comment in #148 this change is approved.

    EDIT: for some reason I saw 3 instead of an 8.

    StatusFileSize
    new176.28 KB
    FAILED: [[SimpleTest]]: [MySQL] Drupal installation failed.
    [ View ]

    Patch #262 no longer apply, need a re-roll.

    Status:Needs review» Needs work
    StatusFileSize
    new187.28 KB
    FAILED: [[SimpleTest]]: [MySQL] 55,727 pass(es), 1 fail(s), and 0 exception(s).
    [ View ]

    A hopefully working reroll - with this patch drupal 8 install works for me.

    Status:Needs work» Needs review

    StatusFileSize
    new187.97 KB
    PASSED: [[SimpleTest]]: [MySQL] 56,060 pass(es).
    [ View ]
    new582 bytes

    Fixing Drupal\rest\Tests\NodeTest

    Status:Needs review» Needs work

    + * The tables declared by this hook will be automatically created when the

    trailing whitespace

    I'm going to complete the proposed review/analysis/summary (see #223, #228, #229) over the next several days.

    Admittedly have been putting that off because it's not very fun (and also due to lack of time) but it's important to get done and discuss this in a level way (and in a way that non-technical stakeholders can participate), given what's being proposed in the issue.

    I think we're all pretty much agreed that disabling modules is a developer tool, to be used for debugging.

    For what it's worth, I don't agree with that. (And neither do a number of other people if you read through the thread.) In fact, I'd almost go so far as to say the opposite... As a developer I debug on a local copy of the site whenever possible. So I don't care about uninstalling modules and destroying data (vs. disabling them and not destroying data), because it's just a throwaway copy. Not so for someone using the actual production website.

    Status:Needs work» Needs review
    StatusFileSize
    new187.74 KB
    PASSED: [[SimpleTest]]: [MySQL] 55,941 pass(es).
    [ View ]

    Chasing head and fixed whitespace issue from #272

    Issue tags:+beta blocker

    Tagging all critical D8 upgrade path issues as "beta blocker."

    If this patch get's committed I will remove all uninstall code from all modules I maintain just to prevent that the settings get deleted if the module just need to be disabled and nothing else. I don't care than about stale data in the database. Master no-brainer.

    If this patch get's committed I will remove all uninstall code from all modules I maintain just to prevent that the settings get deleted if the module just need to be disabled and nothing else. I don't care than about stale data in the database. Master no-brainer.

    No-brainer my ass. That won't work ;)

    You won't have a choice for many things. We are purging config, entities (including fields, etc.) for all modules when they are uninstalled. There is not much you can do to prevent that from happening. I am 100% sure though that there will be a contrib module that will bring this functionality (disabling) back (at your own risk).

    I've been scared by this issue since it has been posted. I really really really am against the removal of the enabling/disabling functionality. I want cruft in my database, if this is the price to pay. That's not a problem for me.

    But, if one day my website shows up an error I want to be able to quickly disable that module and see if that solves the problem. Sure, I could have two different environments: development and production. But reality is that people do not always have the ability to do so. Just think about these 3 scenarios:

    1. I go on holiday. And I don't bring my computer with me, but I have my iPhone for sure. A friend of mine (not a programmer, mainly a content editor) phones me saying "Ei, weird notices are showing up when I visit the homePage." Since I don't have my development environment, I decide to visit the website using my IOS device -- He's right, the xyz module is generating some weird notices. But, reality is that I can't know what's going on. I don't have time/resources to do so right now. But I need to get rid of those notices ASAP (is there anyone who doesn't care about SEO?) and I know that the xyz module can be disabled safely (it just adds some links here and there). So, long story short, I disable the module and clear the cache... That's it. Problem fixed for now, I'll get back to it as soon as I come back home.
    2. I have some feeds importer to import a lot of data into my website, but I use them only once/twice a month. Since I really care about my website performance: why should I keep the feeds module enabled when I can safely disable it when I don't need to import data?
    3. A website that I maintain with some friends has a custom module (written by myself). I'm the only programmer among the website maintainers -- everyone else just care about content. One day, one of the maintainers phones me saying "Ei, maybe I did a mistake while uploading an FTP folder. The custom module stopped working." Mmmmmm, let's see. I have the development environment, where the custom module works properly. But I want to be sure that he messed up only that module and nothing else. So I quickly disable the custom module (either using the UI or the "status" field of the modules table in the database) to check it out. Since the custom module was causing the problem, I simply re-upload it, re-enable it, clear cache and that's it.

    I'm not inventing anything: these scenarios are reality for me. IMO enabling/disabling a module is a critical feature in a CMS. I am really sad about this, but if the enabling/disabling module functionality gets removed I really have to reconsider wether Drupal fits my needs... That's it.

    You could tell me: "Backups exist. You can uninstall and then reinstall and restore the backup." And you would be right. But I don't have time to spend doing something that I don't need to, simply to check wether something is working or not and/or is causing a certain problem.

    Let's please not start this discussion again. At least not before someone fully reviewed the patch and it's RTBC and committed. We can then still talk about how we can solve this in contrib for D8. However, it's as simple as this: We can't fix broken contrib code in Drupal core. And we can't bend over backwards in Drupal core to even try to do that. It's simply overcomplicating and breaking stuff. That's it. Core should be as stable, clean and reliable as possible. If disable/enable stays in core, we are trading exactly that for a feature that is not even needed if you have a proper workflow.

    Also, everyone just keeps mentioning 'stale' data. This is not the true issue here. Noone cares (that much) about stale data. If it just sits there and doesn't touch or interfere with anything. The problem are unexpected values in previously stalled db/config entries or whatever. Things that simply didn't get a chance to update themselves as something else changed in the system which it would normally have reacted to. THIS is what is going to cause notices/exceptions on your site. And that's not something you can easily fix by changing a buggy line of code. Once your data is hosed, your data is hosed and you can just hope that your latest backup wasn't that long ago.

    So please, let's stop arguing now and instead get this patch reviewed and committed.

    > One day, one of the maintainers phones me saying "Ei, maybe I did a mistake while uploading an FTP folder. The custom module stopped working."

    Actually, that's an interesting point.

    If you accidentally delete a module folder (or .info file), on the next cache clear, doesn't Drupal disable the module automatically?

    Does that mean that accidentally deleting a file (or your filesystem having glitches) would then cause data loss?

    I have some feeds importer to import a lot of data into my website, but I use them only once/twice a month. Since I really care about my website performance: why should I keep the feeds module enabled when I can safely disable it when I don't need to import data?

    Feeds (in D7) has the ability to disable individual feeds that you aren't using and export feed configuration to code, meaning you could either disable the feeds OR uninstall the module with no consequenses. Bad example?

    If you accidentally delete a module folder (or .info file), on the next cache clear, doesn't Drupal disable the module automatically? Does that mean that accidentally deleting a file (or your filesystem having glitches) would then cause data loss?

    Well, if the module code is removed from the server, we obviously can't invoke its uninstallation code. So there'd be no immediate data loss, but there would be eventual data corruption. But this is a case of you being responsible for the problem by messing with the state of the server (just as if you starting deleting tables or fields from the database), which is different from data corruption caused by simply using a core-provided UI.

    why should I keep the feeds module enabled when I can safely disable it

    Because per #279 and the issue summary, you very likely can't safely disable it. Feeds is a pluggable system, so if you disable the module, then disable the modules providing some of the plugins, then re-enable the Feeds module, your existing feeds configurations might be broken in a way that maybe Feeds is able to recover from, maybe not. Per #281, exporting your Feeds configuration files, uninstalling the module, then when you need it again, re-installing the module and importing your configuration files is the safer workflow.

    Yeah, Feeds is a good example of a module that should actually work *better* after this patch if you have a half decent workflow.

    Not sure who in the world cares a enough about performance to be aggressively disabling UI modules but not exporting configuration and version control.

    Status:Needs review» Needs work

    I can for sure disable google_analytics, piwik, linkchecker without any harm. Allow modules that are able to assure data integrity to be disabled, please.

    It's my deccission to disable a broken feature temporarily to analyze it in production. Most issues go live without beeing noticed and will be seen by a mass of users very first. A WSOD must be disabled without data loss.

    A module that is missing hook_uninstall does not remove data as I know.

    Contrib is no option for this issue. This is a must function for core. Every field maintainer must expect inconsistent data. That's why we have empty() and isset() and others.

    Allow modules that are able to assure data integrity to be disabled, please.

    This is not possible unless the module (and it's maintainer) knows in advance how all modules that may be built by third parties either on d.o. or for custom projects that extend or list this module as a dependency will use the module's data. This is not possible. This argument has already been picked up, shown to be flawed and put down multiple times already.

    Contrib is no option for this issue. This is a must function for core. Every field maintainer must expect inconsistent data. That's why we have empty() and isset() and others.

    The issue goes beyond empty fields, what about changing/duplicate machine names, for example? You're not considering the full scope of the issue.

    A module that is missing hook_uninstall does not remove data as I know.

    This patch aims to change that.

    Pages