media_entity_copy_icons is called in media_entity.install file which has been removed from the codebase. This is causing the installation of the module to fail.
| Comment | File | Size | Author |
|---|---|---|---|
| #11 | 2934599-11.patch | 1.41 KB | jorgensean |
media_entity_copy_icons is called in media_entity.install file which has been removed from the codebase. This is causing the installation of the module to fail.
| Comment | File | Size | Author |
|---|---|---|---|
| #11 | 2934599-11.patch | 1.41 KB | jorgensean |
Comments
Comment #2
berdirThe 8.x-2.x branch only exists to update an existing installation. You should not install the module in that version anywhere, use the core media module instead.
That said, yes, no point in keeping that function around. Or maybe throw an exception, but it's already too late at that point, can we use an install-time requirement maybe?
Comment #3
hussainweb@Berdir, that makes sense, however, installing Lightning fails right now because Lightning depends on this module (even if Drupal 8.4 is installed). Do you have any suggestions for handling this?
Comment #4
berdirThat sounds like a bug in lightning, you should open an issue there.
Comment #5
hussainwebThis patch should fix the problem. Seeing that we can't avoid installing this module in some scenarios, please consider committing.
Comment #6
hussainweb@Berdir, I agree. I only think it is this way to provide a transition. Still, this could be "fixed" in some way.
Comment #7
berdirI don't think this goes into the right direction. lightning needs to add the module for the upgrade path but the installer should not rely on it and the module should uninstall itself.
Media Entity 8.x-2.x must not be installed and we should explicitly prevent users from doing that, not fixing it.
Comment #8
hussainweb@Berdir, I see. Thanks for responding.
Comment #9
nikunjkotecha@Berdir there are cases were site is installed in automated scripts (travis build / deployment scripts). For such cases, it is important to make sure install of module works without a FATAL error.
Just a suggestion - we can have nothing in hook_install() of the module considering the fact that it is for update only but having code which throws FATAL error is not good IMO.
Comment #10
berdirautomated scripts do whatever you tell them to. media_entity 8.x-2.x should not be installed, it only exists for the upgrade path.
This was discussed a bit today in slack #media and the summary was that we should add a hook_requirments() that disallows to install completely.
Comment #11
jorgensean commentedI agree with @nikunjkotecha, we shouldn't be breaking installable state ever, even if the module is never meant to be installed stand-alone again. Automated scripts that make sure the system is in an installable state will trip here. Having a non-existing function in a module that "only exists for the upgrade path" isn't very helpful either.
Rerolled patch #5 against update_8201 which moved settings.