search

michaelmcandrew / civicrm-buildkit-docker

This is a read only copy. Please make PRs here: https://lab.civicrm.org/michaelmcandrew/civicrm-buildkit-docker
https://lab.civicrm.org/michaelmcandrew/civicrm-buildkit-docker
GNU Affero General Public License v3.0
40 stars 31 forks source link

composer update (bumps min php to 7.1); #66

Closed ginkgomzd closed 3 years ago

ginkgomzd commented 3 years ago

I got a deprecation notice from github relating to composer fetching php-mbstring.

I'm not sure that this fixes it... If not, I think the mbstring composer package is to blame.

not very clear description of the problem: https://github.com/composer/composer/issues/8586

I re-generated the Dockerfiles after upgrade and there was no change in the output.

michaelmcandrew commented 3 years ago

hey @ginkgomzd can you provide a bit more background? I am struggling to understand the context :) what commands did you run, from where, why etc.

Cheers!

wmortada commented 3 years ago

I presume that this relates to updating the Dockerfiles in the publish directory?

ginkgomzd commented 3 years ago

Sorry, yeah. I ran composer install and later got a message from github about authenticating via querystring being deprecated. The repo was for mbstring which is puzzling because the issue is supposed to relate to private repositories but while much is ambiguous, the repo was specified.

There is actually a better than 50/50 chance that it was because my composer was out of date. I am not feeling motivated enough to downgrade composer to test it out.

I submitted the PR anyway because I think all this code does is templated generation, so why not keep it apace with dependencies?

Feel free to close without merging if I am underestimating the need for stability.

ginkgomzd commented 3 years ago

re-reviewing the composer.lock... The new minimum PHP is actually 7.2, and the old was actually 7.0, and not 5.x like I thought.

michaelmcandrew commented 3 years ago

The error you are receiving related to the version of php that you have on your host machine which you were using to run publish (as opposed to anything that will actually be running CiviCRM so its not a big deal, I don't think.

You can sidestep these issues by using a version of php from a container to do so, e.g.

docker run -it --rm -u $(id -u):$(id -g) -v "$PWD":/app composer:2 install
docker run -it --rm -u $(id -u):$(id -g) -v "$PWD/..":/usr/src/myapp -w /usr/src/myapp/publish php:7.3 php generate.php

When I run docker run -it --rm -u $(id -u):$(id -g) -v "$PWD":/app composer:2 install, I get the diff as you so happy to merge in any case.