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

Adding PHP 8 to the documentation and to Docker image #123

Merged
merged 7 commits into from
Apr 28, 2021
Merged

Adding PHP 8 to the documentation and to Docker image #123

merged 7 commits into from
Apr 28, 2021

Conversation

cgsmith
Copy link
Contributor

@cgsmith cgsmith commented Mar 24, 2021

This also fixes the image magic install and updates composer.

@samdark samdark requested a review from schmunk42 March 24, 2021 08:54
@samdark samdark added the status:code review The pull request needs review. label Mar 24, 2021
@schmunk42
Copy link
Contributor

PHP8 support is nice, but I am still undecided about composer 2, since there's no fxp support, see fxpio/composer-asset-plugin#334

Shall we drop fxp?

@bizley
Copy link
Member

bizley commented Mar 24, 2021

I would say so. I switched to asset packagist and never looked back.

@samdark
Copy link
Member

samdark commented Mar 24, 2021

Agree. I've switched long long time ago as well.

@cebe
Copy link
Member

cebe commented Mar 24, 2021

Is it possible to have old composer on old php version tags and new composer on the newer versions?

Otherwise rebuilding and existing tag would break a lot of setups.

I had some issues in the past where on one machine we had 7.4-apache on two machines but these were actually different images.

Moving to composer 2.0 and removing the plugin is a BC break that should not be pushed to the same tag.

@schmunk42
Copy link
Contributor

Actually we had additional suffixes like 7.4-apache-18.12.0 to build images with fixed tags, but there's an issue with Travis building them.

I'll try look into that first, before merging.

@cgsmith
Copy link
Contributor Author

cgsmith commented Mar 24, 2021

Actually we had additional suffixes like 7.4-apache-18.12.0 to build images with fixed tags, but there's an issue with Travis building them.

I'll try look into that first, before merging.

I was trying to find where this is done. Could you comment about this in the Readme or doc file if it isn't there? I'm curious about how I can assist on future cycles of the package.

@schmunk42
Copy link
Contributor

@cgsmith I was confused with old Travis builds. We have tagged images now (with a missing - though, but this will be fixed with above PR).

Looks like GH-actions are working fine and we can move on.

We also need to add PHP 8.0 here https://github.com/yiisoft/yii2-docker/blob/master/.github/workflows/docker-image.yml

@cgsmith
Copy link
Contributor Author

cgsmith commented Mar 24, 2021

@schmunk42 if I add PHP8 to the docker-image.yml file on my fork do you know if that will run on mine for testing?

@schmunk42
Copy link
Contributor

@cgsmith Don't know sorry (do you have enabled them?), I didn't work a lot with GH actions, maybe @bizley and @samdark can shed some light.

@bizley
Copy link
Member

bizley commented Mar 24, 2021

@cgsmith are you asking if Yii runs on php 8? Yes it does.

@cgsmith
Copy link
Contributor Author

cgsmith commented Mar 24, 2021

@bizley no. just curious how i can test the github actions on my fork of this repo.

@bizley
Copy link
Member

bizley commented Mar 24, 2021

Github runs what is inside workflows folder so you could for example fork it and make PR to your main fork branch.

@schmunk42
Copy link
Contributor

Github runs what is inside workflows folder so you could for example fork it and make PR to your main fork branch.

But why are there no builds/actions on the commits on this PR?

@bizley
Copy link
Member

bizley commented Mar 24, 2021

Oh, there were none? Weird...

@bizley
Copy link
Member

bizley commented Mar 24, 2021

I can see the workflow builds and pushes the docker image to the registry. We need something additional to only test the images.

@bizley
Copy link
Member

bizley commented Mar 24, 2021

I'm using this condition to build and push images when tagging the release with X.X.X semver schema.

on:
  push:
    tags:
      - '*.*.*'

@schmunk42
Copy link
Contributor

I can see the workflow builds and pushes the docker image to the registry. We need something additional to only test the images.

I don't quite understand, the workflows looked good to me?!

@bizley
Copy link
Member

bizley commented Mar 24, 2021

Oh, my bad, login and push to registry is conditional. So it's fine, maybe it could be separated to simplify the workflows. Anyway.
I believe

on: [push, pull_request]

is missing.

@cgsmith
Copy link
Contributor Author

cgsmith commented Mar 24, 2021

@bizley gotcha i needed to enable actions on my end. sorry for the test commit. I forgot it came down in the PR. I'll revert that.

@cgsmith cgsmith marked this pull request as draft March 24, 2021 15:52
@cgsmith
Copy link
Contributor Author

cgsmith commented Mar 24, 2021

I'll put this back into a pending PR after the checks are all done. This PR should also close #110

@cgsmith cgsmith changed the title Adding PHP 8 to the documentation Adding PHP 8 to the documentation and to Docker image Mar 24, 2021
@schmunk42
Copy link
Contributor

We have correctly tagged images like 7.4-apache-21.3.1 now, so we can proceed with moving to composer 2.
The above tag is the last image which will have composer 1 installed by default.

@bizley bizley added status:under development Someone is working on a pull request. and removed status:code review The pull request needs review. labels Mar 26, 2021
@Alex-DA
Copy link
Contributor

Alex-DA commented Apr 16, 2021

Hello @schmunk42 @bizley @cgsmith , what is the status of this PR?.
Exist some official image to use YII2 with PHP 8.0 ?.
Thank you for your work.

@schmunk42
Copy link
Contributor

@Alex-DA This PR is in draft stage, but I think we could move forward with it.

Could you give https://hub.docker.com/r/yiisoftware/yii-php/tags?page=1&ordering=last_updated a try in the meantime?
Actually this should also work.

@cgsmith
Copy link
Contributor Author

cgsmith commented Apr 17, 2021 via email

@schmunk42
Copy link
Contributor

schmunk42 commented Apr 19, 2021

Could you try updating TEST_YII_VERSION to 5fbd28e05609bd31c8415ede663747c2133d1e46 (2.0.41.1) - I got an error from GitHub when trying to create a new PR from this PR.

@cgsmith cgsmith marked this pull request as ready for review April 19, 2021 13:39
@schmunk42
Copy link
Contributor

Hmm, actions do not run.
Could you resolve the conflicts and remove WIP; not sure if actions run on WIP PRs?

@schmunk42
Copy link
Contributor

At least the action ran now: https://github.com/yiisoft/yii2-docker/actions/runs/763731034

But we need a change, that pushing only happens on master - I contacted GH-support about actions and they said relevant is the current workflow from master(!) not the one from the PR (which makes also sense, but it was different with travis I think)

So conflicts must be resolved for the PHP-8 images to appear.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status:under development Someone is working on a pull request.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants