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

Add docker compose #126

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from
Draft

Conversation

mikk150
Copy link

@mikk150 mikk150 commented Feb 26, 2025

No description provided.

@mikk150
Copy link
Author

mikk150 commented Feb 26, 2025

Should probably add some kind of Contributing.md file to document how to run those tests

@SamMousa
Copy link
Collaborator

SamMousa commented Feb 26, 2025

I don't like doing composer install inside docker. You need the uid config etc and it's a hassle. It's reasonable to assume contributors have composer installed.

I do like the test part. Here you could even mount a different volume for dependencies per PHP version. That way permissions are not relevant.
Edit: in this case you do run composer install inside but it's not persisted to the outside.

Please mount the source as read only, with the exception of the output directory.

Also, can we use some standard image instead of building our own? Alternatively we can build an image in this repo and reference it in the compose file.

@mikk150
Copy link
Author

mikk150 commented Feb 26, 2025

I don't like doing composer install inside docker. You need the uid config etc and it's a hassle. It's reasonable to assume contributors have composer installed.

How do you plan on fixing different PHP versions having different dependencies?

I may have php8.2 installed on my host OS, but I would like to run tests on php 8.4

I do like the test part. Here you could even mount a different volume for dependencies per PHP version. That way permissions are not relevant.

that could be done.. presumably using volumes?

Please mount the source as read only, with the exception of the output directory.

yup

Also, can we use some standard image instead of building our own? Alternatively we can build an image in this repo and reference it in the compose file.

who is going to maintain these images? I am not familiar with github actions as I predomionantly use Gitlab, so someone else needs to add actions to auto-update these images then

@mikk150
Copy link
Author

mikk150 commented Feb 26, 2025

I am not sure if I can mount source DIR as RO, as composer does need to write to it(remember composer.lock file)

@SamMousa
Copy link
Collaborator

I am not sure if I can mount source DIR as RO, as composer does need to write to it(remember composer.lock file)

We should get it to write the lock file somewhere else.

The abstract reasoning for me is that:

  • Docker compose should do the full test suite (ie all supported PHP versions)
  • It MUST NOT alter the working directory

You could even do something like:

  1. Mount source dir as RO
  2. Copy source to tmp dir
  3. Do your thing in the tmp dir, like compose install run tests etc

What do you think?

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.

2 participants