Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DX: Add Docker dev setup #5234

Merged
merged 1 commit into from
Jan 17, 2021
Merged

DX: Add Docker dev setup #5234

merged 1 commit into from
Jan 17, 2021

Conversation

julienfalque
Copy link
Member

@julienfalque julienfalque commented Nov 2, 2020

No description provided.

@keradus
Copy link
Member

keradus commented Nov 3, 2020

I would somehow explicitly notice whether this is a PHP CS Fixer docker image, to be used by folks willing to run the fixer, or dev-purpose image to use internally for development of this repo itself?

@julienfalque
Copy link
Member Author

These images are intended for internal usage (dev and tests). I'll add information in CONTRIBUTING.md after the setup itself is approved.

xdebug.remote_autostart=1
xdebug.remote_enable=1
xdebug.remote_host=host.docker.internal
xdebug.remote_port=9003
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Images that use Xdebug 2 (PHP < 7.3) use the port 9003 instead of the default 9000 because on Xdebug 3 the default port has been changed to 9003. This removes the need to change the port to listen in e.g. PhpStorm everytime you switch between versions of PHP.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe we don't need xdebug for 5.6, which we gonna drop soon ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's exactly the same config than for PHP 7.0, 7.1 and 7.2, so PHP 5.6 does not specifically make the whole setup more complicated.

&& php composer-setup.php --install-dir=/usr/local/bin --filename=composer \
&& rm composer-setup.php \
# xdebug command
&& curl --location --output /usr/local/bin/xdebug https://github.com/julienfalque/xdebug/releases/download/v1.1.0/xdebug \
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

on first look, i thought it's simply fork of the xdebug itself.
maybe consider renaming the cli file?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You mean in the upstream repository or here in the Dockerfile?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i would dare to suggest both :D xdebug is already a name in ecosystem, reusing it can be confusing

@@ -0,0 +1,29 @@
version: '3.8'
Copy link
Member

@keradus keradus Dec 28, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i am not sure about best practice here. can we somehow differentiate that this is a docker image only to be used inside php-cs-fixer itself, and not the docker image for ppl willing to run PHP CS Fixer?

maybe we can extract it to separated package, like https://github.com/PHP-CS-Fixer/php-docker-toolkit (whatever name) ? would that work for us , or it would not let us run it from PHP-CS-Fixer path?
like, i can imagine reusing same docker-compose for any PHP tool development

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think docker-compose.yaml are meant to be shared between different projects. I'd say we can safely assume this will be considered as the setup to work on the project itself, not to use it elsewhere.

You can then build the images:

```console
$ docker-compose build --parallel
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ker@dus:~/github/PHP-CS-Fixer λ docker-compose build
ERROR: Version in "./docker-compose.yaml" is unsupported. You might be seeing this error because you're using the wrong Compose file version. Either specify a supported version (e.g "2.2" or "3.3") and place your service definitions under the `services` key, or omit the `version` key and place your service definitions at the root of the file to use version 1.
For more on the Compose file format versions, see https://docs.docker.com/compose/compose-file/
ker@dus:~/github/PHP-CS-Fixer λ docker-compose --version
docker-compose version 1.25.0, build unknown
ker@dus:~/github/PHP-CS-Fixer λ docker --version
Docker version 19.03.8, build afacb8b7f0

what am I doing wrong?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your Docker Compose version (1.25.0) is too old to support version 3.8 of the Compose file format (supported since 1.25.5). But I don't think features specific to this version are used here so we can probably use a lower one.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

interesting, sudo apt-get docker-compose on ubuntu brings only 1.25.0 and does not want to offer anything newer.
(yes, i know, i can download the binaries manually...)

docker/php-7.4/Dockerfile Outdated Show resolved Hide resolved
docker/php-7.1/Dockerfile Outdated Show resolved Hide resolved

Make sure the versions installed support [Compose file format 3.8](https://docs.docker.com/compose/compose-file/).

Next, copy [`docker-compose.override.yaml.dist`](./docker-compose.override.yaml.dist) to `docker-compose.override.yaml`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question: is this step obligatory?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently yes: the images require the DOCKER_USER_ID and DOCKER_GROUP_ID arguments to create a guest user that matches the host user to avoid file permission issues. If there's a better alternative, I'd be happy to update.

docker/php-8.0/Dockerfile Outdated Show resolved Hide resolved
docker/php-7.3/Dockerfile Outdated Show resolved Hide resolved
@keradus keradus changed the title Add Docker setup DX: Add Docker dev setup Jan 17, 2021
@keradus keradus changed the base branch from master to 2.17 January 17, 2021 23:12
@keradus keradus added this to the 2.17.4 milestone Jan 17, 2021
@keradus
Copy link
Member

keradus commented Jan 17, 2021

Thank you @julienfalque.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants