lorisleiva / laravel-docker

šŸ³ Generic docker image for Laravel Applications
MIT License
927 stars 313 forks source link

Enable normal caching of composer installs #46

Closed ragingdave closed 4 years ago

ragingdave commented 4 years ago

In normal composer, caching is handled automatically and at least in terms of bitbucket pipelines (and probably other ci/cd infrastructure) they have pre-built caching that handles the lengthy install each time a build is performed.

Due to the setting in the docker image to change the composer home env, this breaks at least the bitbucket pipelines, but maybe more. Is there any reason to no just leave the composer home env alone and just let it work as intended? I see you have a configured custom cache in the gitlab deployment but it never mentions the actual composer cache, but only the vendor directory.

UDPATE: Simply removing the line in the dockerfile for setting the env, will enable the normal caching to work

lorisleiva commented 4 years ago

Hi there šŸ‘‹

Thanks for raising this issue.

This was already like that originally but users had issues with binaries not being found automatically from the $PATH variable. See issue https://github.com/lorisleiva/laravel-docker/issues/20.

Defining an explicit directory for COMPOSER_HOME enabled us to fix this issue. Since then I've had a look at other PHP Dockerfiles that include composer and most of them seem to be using this approach.

I'd love to improve this by making it more cache-friendly for CI/CD tools but not if it's going to re-open issue #20.

Let me know your thoughts on this. šŸ‘

ragingdave commented 4 years ago

While I of course can't test all the ci/cd implementations due to this being docker-ized I would think that all pipelines would work the same...because well otherwise...what's the point of docker. With that said I can say that the modifications in the PR applied in a new image, it does work as expected. I would say that I don't use the pre-installed version of php codesniffer, as I run codesniffer in a pre-commit hook as well so the pipeline check is just for good measure. I'm assuming the pre-installed version of phpcs is what caused that issue in the first place, which for me seems like bad practice and enforces a mentality of "ci will catch it"...IMHO of course.

Link to working (cachable) composer install in docker based off the PR code: https://hub.docker.com/repository/docker/ragingdave/laravel-docker/general

Additionally perhaps a middle ground would be to just update the PATH in the docker container to add the global composer bin install, which would not require the changing of the COMPOSER HOME, but should still make phpcs be part of the PATH

ragingdave commented 4 years ago

So it seems that it doesn't actually work in the PR....did some more checking and it seems neither ends up working in bitbucket for using it's built in caches....looks like I need to do more investigation to get it working...

lorisleiva commented 4 years ago

Iā€™m going to close this for now. Feel free to open a new issue if needed. šŸ™‚