Properly deprecate the entity reference module

One important thing with module deprecation is we need to ensure the module is actually uninstalled before it's removed from the code base.

This could mean:

- Including an update with 9.0.0 that uninstalls the module, leaving just the .info.yml (but with installation prevented somehow) to allow that process to happen. Then physically removing the .info.yml in 10.x

- requiring manual uninstallation prior to a 9.x/10.x update

If modules are moving to contrib this isn't as important, but for actual removals we need to figure out how to manage it.

Probably better to add another update hook to uninstall module (the field_update_8002() already removes config

Reroll and removing usages, many tricky ones can be fixed by just adding it to \Drupal\Tests\DeprecatedModulesTestTrait, thanks to the work done in the block_place issue.

About the removal, not having the module around anymore results in some user notices, but nothing is broken.

On 8.9.x, do drush en -y entity_reference
switch to 9.0.x, rm -rf core/modules/entity_reference

Visit /update.php

Shows User warning: The following module is missing from the file system: entity_reference in drupal_get_filename() (line 188 of core/includes/bootstrap.inc), but you can run updates.

And at this point, the update system already purged entity_reference from core.extension. The reason that the user notice doesn't go away is because we still have the system.schema key value definition for it, so the update system tries to get its update functions.

What if we just add a system_update_900x() that manually cleans up that record (removing it from core.extension but not key_value is IMHO a bug anyway) if you really still have it enabled?

Too late to get this into 8.8.0, but even though we didn't enforce it so far and had a few left-over tests in core, I expect barely any site actually still has this module enabled, as this happened very early in the D8 cycle (8.0.0-rc1 seems to be the first tag to have the commit).

#7 looks RTBC to me.

Opened #3111645: Uninstall entity_reference module and prevent it being enabled again, remove deprecated code for 9.x to remove the code we're deprecating here, and properly prepare the module for actual removal.

I think it is unrelated test failures, so making it RTBC as per @catch's comment #12.

The test failures in #7 aren't unrelated, we need to add @group legacy to that test unfortunately.

For #15.

Is this what we need here? I am not exactly sure this is Novice :D At least I was not sure if I am doing this right.

Hm, that is not what we need.

We should only be deprecating any actual APIs or plugins here I think. And a hook_requirements() runtime warning if the module is enabled if we didn't already do that.

Copied over the requirements hook that we did in 9.0.x and removed the @trigger_error() from the .module file, I think that's what you meant by only deprecating actual API's here?

If we do that (which is IMHO what we should have done with block_place.module too), then I think we don't need to change DeprecatedModulesTestTrait, lets see.

Ok, green. Do we need some explicit BC tests here?

IMO we don't need explicit test coverage here - it's not really providing a bc layer as such, it's just (hopefully) dead code by now.

So I think this is ready to go.

@alexpott - looking at those grep results, they all look like references to https://www.drupal.org/project/entity_reference_revisions, not entity_reference?

We're trying to prevent the module from being installed, so that once it's uninstalled, it won't be re-enabled again before 9.x actually deletes it from the file system.

+++ b/core/modules/entity_reference/entity_reference.install
@@ -0,0 +1,26 @@
+      'description' => t('Entity Reference is deprecated, all functionality has been moved to Core.'),

That's ambiguous... the Entity Reference module IS in core!

Maybe 'the entity Core subsystem' would be clearer?

> Maybe 'the entity Core subsystem' would be clearer?

It's not correct though. It has been moved to Core\Entity, but also system.module and views.module.

'has been moved to other parts of core'?

hmm OK let's just make this a runtime warning in 8.x then.

#3111645: Uninstall entity_reference module and prevent it being enabled again, remove deprecated code was the 9.x issue here, which turns the module into a stub, uninstalls it, prevents it being reinstalled. For actual removal in 10.x (not what I said in #27...).

I filed #3135100: [policy and meta] Provide a proper mechanism for deprecating modules and themes (which was supposed to have been filed like eight months ago, but oh well).

We missed the boat to do this in D8, I think? But the stub is still there in D9, so we still need to do something prior to D10.

Bumping priority.

Patch #21 is not applying anymore so, here I have added a re-roll of patch #21 for Drupal 8.9.x.

Patch #36 is failed to apply on for Drupal 9.1.x , kindly follow a new patch.

Patch #37 is successfully working on 9.1

This got auto-bumped to 9.x, but I think there is nothing to do in 9.x yet? We can still add a runtime warning in 8.x as per #32, but is it even worth doing that now?

I think we should not backport anything related to this to 8.9.x. That is a long-term support branch restricted to non-disruptive changes. This is for completing the deprecation and removal safely so there's nothing left of it in our codebase in D10.

For this issue, we should perhaps also make it so that the stub module can't be installed on new sites (See #3112432: Prevent core, stub simpletest module from being installed on new sites for example), and then later consider the update hook option to uninstall it from existing sites.

Some of these are things we would commit to 9.2.x.

@xjm we already uninstalled it on existing sites and prevented it from being installed again in #3111645: Uninstall entity_reference module and prevent it being enabled again, remove deprecated code

So is there anything else to do here, or should we simply close this and open a postponed D10 issue to rm -rf the wrapper?

I think #43 is the correct course of action here.

Opened #3188858: Remove the entity_reference module entirely on the Drupal 10 branch, see you there. Closing this as duplicate then, since we did not "fix" anything here per say.