lorisleiva / laravel-docker

🐳 Generic docker image for Laravel Applications
MIT License
934 stars 314 forks source link

Add the xdebug extension back #22

Closed lorisleiva closed 5 years ago

lorisleiva commented 5 years ago

In a commit comments, @ajcastro asked why xdebug was temporarly removed from this Docker image. I'm opening a new issue with my answer and the potential work that could be done to add it back.

Why was is removed?

The builds were failing due to the pecl extension xdebug. See related issue here.

Basically, the xdebug extension is still in RC phase for php 7.3.

However, after we've removed xdebug from the build, we also decided to support the latest 3 versions of php. So xdebug could be added back to 7.1 and 7.2.

We should hopefully be able to add it to 7.3 and latest very soon.

screenshot 2019-02-17 at 12 08 28

This raises potential issues when maintaining the latest image which is automatically update to the latest version of PHP since there will likely be a small gap in time where the latest version of PHP is ready but some of its extensions are not.

In summary

fgilio commented 5 years ago

The last point is the tricky one. Just to list what's on my head:

lorisleiva commented 5 years ago

Hi @fgilio, Thanks for your input on this issue. 💯

The only thing that "annoys" me about not having xdebug (or any other extension that potentially creates compatibility issues) in the latest image is that it makes it a lot less attractive to use than the others. Personally, I'm planning on using the latest image 90% of the time and I'd like it to be as complete as possible.

An alternative would be to manually define the latest version of PHP in the latest image. That way we have full control on when to actually release the latest version of PHP on our repository. I.e. we would use FROM php:7.2-alpine in the latest image until everything we need is available on 7.3.

So in our current case we would have:

Dockerfile PHP version Features
7.1 7.1 ✅ Everything
7.2 7.2 ✅ Everything
7.3 7.3 ⬇️ Lacks xdebug
latest 7.2 ✅ Everything

The negatives of this alternative are:

What do you think? I'm also open to other alternatives. 🙂

fgilio commented 5 years ago
  • the PHP version used in latest is not very intuitive (as show the table above).

Yep, this one I think is a real problem and I have some suggestions below.

  • it requires a bit more maintenance from this repository to keep track of PHP releases and the availability of the extensions we use (although I think this one is inevitable).

100% agree, there will be some manual work cause automating it would be hard and not worth it. On the plus side, it's not something that would need to be done every month.

My 2 cents are: A) What about ditching latest altogether? When I deploy something I actively choose a specific PHP version (or at the very least I know which one I'm using), I guess it's perfectly valid to have to choose the matching laravel-docker version. I mean, it's the only real way of knowing that there won't be any compatibility issues. Personally, I'm going to do this on all my projects. B) Just accept the fact that latest could not have every extension. Make it plain clear in the readme, and explain why. Maybe even disencourage it's use or explain that it's not the recommended one if you may ever require the extensions.

And what you suggest also sounds fine and I think is a viable alternative.

TBH, I don't feel 100% comfortable with any option and I think that maybe there's no correct one. I feel like we could use more opinions here.

ajcastro commented 5 years ago

I agree with @fgilio's 2 cents. This way we can make sure that our image will always be the same. Usually we dont want unwanted surprises. :-) And choosing for the right image for our desired php version will be intuitive. The latest may usually mean unstable and/or should be used with caution or at own's risk. I think this is normal use-case for latest builds, so I agree that we just use a matching laravel-docker version.

lorisleiva commented 5 years ago

Thanks for your feedback guys, I think we're converging towards a solution.

What do you think about this?

Dockerfile PHP version Features
7.1 7.1 ✅ Everything
7.2 7.2 ✅ Everything
7.3 7.3 ⬇️ Lacks xdebug
stable 7.2 🔗 Alias the 7.2 image until 7.3 is ready
latest 7.3 🔗 Alias the 7.3 image

That way...

fgilio commented 5 years ago

Yes, that looks fine 👍 The only thing that slightly worries me is the maintenance cost of supporting stable and latest. But, as we said before, some maintenance is kinda inevitable. Lol xdebug could be releasing stable 2.7 by the time we finish this. But I guess it's nice to talk about this things, cause it'll happen again and again and again.

lorisleiva commented 5 years ago

Hahaha yeah I was thinking the same.

Yes that's why I wanted to see stable and latest more like aliases. Which makes me wonder if we could use actual symlinks with GitHub and Docker Hub...

I mean worth case scenario it's just a copy/paste a couple of times a year.

Fogest commented 5 years ago

Can you clarify, if I currently need xdebug in one of my CI environments what version should I currently be using instead of latest?

And will I then in the future need to switch this? In the ideal world I would like to just stick it on latest and not have to adjust it later. In the ideal world I would expect it to have everything as well even if it was a PHP version behind.

xdebug missing right now for example is breaking phpunit working properly.

lorisleiva commented 5 years ago

Hi @Fogest,

I've just added xdebug back to the images 7.1 and 7.2 in my latest commit.

Once the builds have been completed you should be able to use Laravel Docker with xdebug on those images.

fgilio commented 5 years ago

Awesome @lorisleiva! I'll stop using my fork next week :D

EDIT:

Yes that's why I wanted to see stable and latest more like aliases. Which makes me wonder if we could use actual symlinks with GitHub and Docker Hub...

Mmm I've used symlinks in git. I made a symlink of 1 directory in the repo, to another location in the same repo. Works just fine.

kreitje commented 5 years ago

It looks like as of 2.7.0 they support PHP 7.3

lorisleiva commented 5 years ago

@kreitje Awesome, thanks for sharing this information. I'll add xdebug to the 7.3 image. @fgilio I'll also try the symlinks for stable and latest and see if Docker Hub is happy with them.

Btw, should I start adding the 7.4 image (without xdebug)?

Edit: There isn't a 7.4-alpine yet (only RC versions) so that answers my question.

lorisleiva commented 5 years ago

Okay, so Dockerfiles as symlinks are not recognised by Docker Hub. I'm just going to use the automated build rules from Docker Hub to automatically create the stable and latest tags.

Screenshot 2019-06-28 at 10 10 00

I'll just need to update those rules every now and then instead of updating a symlink which is actually more convenient.

lorisleiva commented 5 years ago

Fixed by #31 and documented in https://github.com/lorisleiva/laravel-docker/commit/4a8eee42f9401af2bc96f7cb7a1ea0b24c96b158.

kreitje commented 5 years ago

Thanks for the super quick turn around!

fgilio commented 5 years ago

Awesome!