Module should not delete fields on uninstall

I would follow the example set by core. Don't do anything on uninstall. It looks like we have a todo that field.module will handle it for us automatically, but until then, we still shouldn't do it.

http://api.drupal.org/api/drupal/modules--field--field.module/function/f...

Well crap. comment_uninstall does delete its field. Hrm
http://api.drupal.org/api/drupal/modules--comment--comment.install/funct...

Disabling the module is just turning it off. Uninstalling means wiping it from existence. If I installed a module and then decided to uninstall it, I'd rather it didn't leave its fields clogging up my database. I think that's generally understood as the best practice. If other modules use fields that you define, they should declare your module as a dependency, so it won't be uninstalled.

The problem here is that this has never been resolved authoritatively by either CCK or core, even in D7.

So the very least we can do is pick a solution and give good comments for it, explaining the alternative.

@linclark, do I understand from the change of title that your patch in #5 is not to be pursued? Making this "needs work" then.

"The call to field_delete_field is the big problem right now. That is the one that deletes the field regardless of whether or not the user has attached it to another bundle using Field UI."

Yes, exactly. :-)

If you uninstall your module, you see this warning:

"The following modules will be completely uninstalled from your site, and all data from these modules will be lost!"

This leads me to believe that if I'm implementing hook_uninstall(), I can't let field_delete_instance() make the decision for me about fields I implemented, for the reason you just gave. Without cleaning up all the instances, there's a bunch of zombie data in the database with no module to look after it.

This problem exists for field_example, too. You can uninstall it but the data will live forever in your db.

An ideal core solution would be to have Field API flip a switch on module dependencies, disallowing the uninstall (or deactivation, for that matter) of any module that has instances in use. A quick test tells me this hasn't been implemented. And a best practice would be to disallow as many fields from being attached through Field UI as possible, but I'm coming up short finding out how to do that.

I have been fighting with the uninstall process of a module I'm writing.
I can't eliminate the instances from the database

it seems field_info_instances() doesn't return nothing after I disable the module.
I tried in the "execute php code" page from the devel module, I give

echo field_info_instances('entity type', 'entity');

getting "Array" in return, until I disable the module then nothing.
Is someone else having the same problem?

You may be interested in #943772: field_delete_field() and others fail for inactive fields.

thanks

With the devel PHP page, try print_r() instead of echo, like this:

print_r(field_info_instances('node', 'article'));

and/or:

print_r(field_info_instance('node', 'body', 'article'));

Echo just gives you 'Array' which isn't very helpful.

I made a little sandbox project to shed some light on orphaned field data. :-)

http://drupal.org/sandbox/Mile23/1171894

Wondering if #943772: field_delete_field() and others fail for inactive fields has relevance for this issue.

webchick sez it's fixed: http://drupal.org/node/943772#comment-4924298

But really that's just to start a new discussion in a new issue queue. I wonder which one.... Helpful list and meta-issue of what to do about disabled modules: #1199946: Disabled modules are broken beyond repair so the "disable" functionality needs to be removed

See #1204820: Entity providing modules must call field_attach_delete_bundle() in hook_uninstall().
In hook_uninstall(), you call field_attach_delete_bundle() for the bundle the module provided, and all the fields attached will get dropped.
This is the new consensus in core and contrib.

That issue is in EntityAPI issue queue, which isn't core, but yah. :-) The core issue is: #1115510: Entity providing modules must call field_attach_delete_bundle() in hook_uninstall().

It seems obvious to me that Drupal should do this instead of requiring it in hook_uninstall(), especially since all implementations will be less than trivial, exactly the same, and there's a high risk if someone does it wrong.

@Mile23:

If other modules use fields that you define, they should declare your module as a dependency, so it won't be uninstalled.

I don't know where the intricacies of this issue are concerned, but I completely agree with this.

Trying to fix this for #976022: field_data_media_gallery_media not removed on uninstall. How should this be handled in a module?

How about if some of the esteemed names who are following this issue chime in, please. :-)

Currently, Drupal won't let you uninstall modules which are responsible for existing field content. Perhaps the best practice is to not delete anything, and require the user to do all the deleting, via the Drupal dependency maze. As tedious as that can be. :-)

Same for Node API. Our module would depend on node, which can deal with orphaned node types from disabled and uninstalled modules, and force the dependency by disallowing disabling, if need be.

So the best practice would be:

1. Separate modules for each API (node, field) where possible, ideally with a 'core' module that supports both.
2. Leave content for the user to deal with, since they'll be able to do it in Drupal UI thanks to dependencies.
3. File core bug reports if Drupal can't deal with the dependencies you've created.

Any disagreement?

Solving two bugs with one patch: #1268112: Disabled Node Example module still permits construction of Node Example content

I'll mark this issue as fixed when the patch gets applied over there.

Committed a patch which doesn't perform any deletion at all, at #1268112: Disabled Node Example module still permits construction of Node Example content

See it here: http://drupalcode.org/project/examples.git/commitdiff/71dcc49705f5e838ff...

Well think this is the wrong approach as it is impossible to use the code unless you are already an expert now(and guess you don't need it then).

This is the scenario i think most beginners approach.

1 Activate module and see what is does.
2 Make own changes. See what happens.
3 When you got it all figured out how it works, start writing your own module or rename the module to your own module name.

As the second part is impossible to do as the fields and hooks are permanent and only invoked first time the module is installed i guess its not only me that will put down a lot of hours trying to figure out why their code don't work. And when we finally find out there is no other way then to manually remove every post/tables in the database to start testing how to build your module.

And maybe its just because I'm stupid but my codes long from perfect on first try so i have to manually remove all database post (don't even have any nodes or fields in the database), rewrite the code, try again and see if it works, remove everything again, try again and so on and so on.

So at least this module actually kind of makes so it takes me a lot longer to build my module and leave me to find other resources (and now i cant even remember the perfect wording i used on google to find the one page that actually had it as all other resources points here). So it do not feel like it is best practice to leave all fields and bundles intact after uninstalling the module.

But i really do appreciate all the help you give us! Your helps awesome!

Sharing fileds is not cool. You may think of some use case where is interesting. But generally is a bad practice. It raises problems when creating Features. It raises problems when installing-uninstalling-reinstalling modules. It incorporates complexity to the system (meaning Drupal core). More important, you should always be able to remove your module completely (entities, types, fileds, data...). If you want to keep your data, backup or migrate it. Cloning fileds would be a much more interesting strategy.

Examples is an awesome work BTW. Thanks!

Cheers
Alessandro Mascherpa

I am newly exploring this issue and have a clarifying question:

Is there a reason NOT to have field_delete_field($field) fail/abort deleting the field if there are any instances of that field (for any bundle)?

Note: Following this "garbage collection" approach would necessitate a best practice of always removing your field instances before attempting a field deletion. Alternately, an additional parameter could allow an optional bundle array to trigger instance and field deletion in a single pass ( field_delete_field($field, $bundles='') - again aborting deletion if any instances of the field remain).

@Strutsagget: The Examples project exists so you can have an example of how a given API should/could/might be implemented. You can change things as much as you want... You could have your own node module delete all the content when it gets uninstalled, etc.

There are a few different tools to help developers deal with these APIs. If you find yourself needing to reinstall a module all the time to test its installation behavior, you might check out drush. In fact, just get drush and learn it. :-) For instance: drush pm-uninstall [module] at the command line will uninstall the module, and then drush en [module] will set it back up.

@AlessMascherpa: Fields have two main architectural areas: The field is defined by a module, and the field is instantiated by the user (or the module). As far as Drupal is concerned, the module that defines the field is now a dependency of all content that uses instances of that field, and it won't let you disable or uninstall that module. This is why it's the best practice to separate fields out to their own module if possible.

@joseph.muennich: Look at the source for field_delete_field(): http://api.drupal.org/api/drupal/modules%21field%21field.crud.inc/functi...

It calls field_delete_instance() on all the field's instances, which marks them for deletion. Then you can call field_purge_batch().

Rundown here: http://api.drupal.org/api/drupal/modules%21field%21field.crud.inc/group/...

HTH.

Hi @Mile23,

Agree that a field declaration has to live in its own module. Like the entityreference filed does. No disagreement about that. Sharing fileds instances between entities bundles still feels like a bad practice (comment #29).

Using an entityreference field instance created by a module will crash (or be inconsistent) on reinstall if you don't delete the field instance or check if it exists before execute its creation, to take in to account this case. That's a way to increase system logic complexity without good reason. It does not make any sense BTW.

A better practice will be IMO to clone filed instancies instead of sharing them: http://drupal.org/project/field_tools This way you don't have to rely in the module developer following good practices. The system takes care about it for you. I understand that this is a system wide behavior, and not Examples module.
EDIT: I correct myself field_tools module referenced in prev. paragraf doesn't really clone a field instance. It eases the sharing of a filed instance in a bulky way :(

Cheers
Alessandro

Hi and thanx for your answere!

My problem aint with uninstallng/installing the module in general. That infect would be great id it worked and I do use drush on all our projects. The problem is that uninstalling and reinstalling does nothing as the creation of the field is just done the first time as the field is never uninstalled when you uninstall the module.