aspendigital / docker-octobercms

Dockerized October CMS: PHP, Composer, October core and dependencies
MIT License
150 stars 54 forks source link

Revisit composer install triggered by `INIT_PLUGINS` env var #27

Open petehalverson opened 4 years ago

petehalverson commented 4 years ago

The current method may cause conflicting dependencies.

petehalverson commented 4 years ago

I am trying to avoid requiring a persistent /vendor volume for a variety of reasons.

@jimcottrell, @LukeTowers pointed out the OC marketplace plugins add the replace property. I'm assuming that doesn't work unless composer install is run from the project root, correct?

LukeTowers commented 4 years ago

@petehalverson the point of running composer from the root instead of in each plugin is to make sure that all plugin dependencies are considered in addition to the core dependencies when determining what versions and libraries to pull; as well as preventing any duplicate dependencies (potentially with different versions) from being present in the project.

What the marketplace does is a compromise between a proper full install of dependencies in the project root and having a free for all in plugin vendor folders: it includes all dependencies that are already included in the base october project as values in the replace property that it injects into the composer file for each plugin before running composer install on that plugin directory. This prevents most of the issues relating to having multiple versions / copies of the same dependency loaded at the same time.

However, my vote would still be to run the composer install from the project root, as that's how it should be done when deploying to production and causes the least issues overall. By no means am I an expert with Docker though, so I'd like to hear the arguments against doing so.

petehalverson commented 4 years ago

For some context, composer install runs during the image build process and stores dependencies in the /var/www/html/vendor folder of the image.

For local development workflows, I assume volumes are mapped to folders within the project which contain the plugins in use. Most plugins repos ignore the plugin vendor folder, which means those dependencies need to be fetched at runtime (hence the INIT_PLUGINS=true option). Because those plugin folders exist on the host as a part of a project, those dependencies get stored locally and persist outside of the container.

In practice, this means composer install (and the wait that comes with it) is only triggered the first time a project is spun up or a new plugin is introduced.

I think applying a 'replace' fix to the plugin composer.json, similar to what's being done on the marketplace is a good idea to facilitate this assumed project layout.

To be clear, nothing is preventing users from mapping a local volume to /var/www/html/vendor and running composer install at the project root.

Perhaps we could support that workflow better by introducing a different entrypoint env var option e.g. INIT_PLUGINS=root which could automatically copy dependencies to your local filesystem and run composer install from the root.

When I tested a 'composer from root' workflow. I was deterred by:

I recall digging into some other options to improve speed a bit by detecting if the dependencies for plugins already existed, without running composer install. It just felt like a hack...trying to run composer without running composer.

petehalverson commented 4 years ago

I should point out by detecting the presence of a composer file and whether or not a vendor folder exists in each plugin folder, we're able to elegantly support plugins introduced via the marketplace.

And by elegantly support, I mean do nothing because nothing needs to be done.

LukeTowers commented 4 years ago

Those points are fair enough. I agree that replace might be the way to go, my annoyance with the current process is just as a result of having plugins with composer files that aren't really needed (i.e. just pulling in composer/installers or requiring some laravel core dependency) will cause a vendor folder to get generated for the plugin which further more is only really a problem if using gitsubmodules because then it clutters up the git submodule repo status. That's really just correcting for developer mistakes (not including /vendor in the .gitignore and including composer dependencies even when they're not needed) but it would be nice to correct for those mistakes automatically.