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

Update Dockerfile to install PHP zip extension #7

Merged
merged 1 commit into from Oct 9, 2015

Conversation

skyred
Copy link
Contributor

@skyred skyred commented Aug 4, 2015

Drupal uses Composer a lot; especially Drupal 8, and there are a lot of modules, requiring composer, are being back ported to Drupal 7. Composer use PHP zip extension. Otherwise, see docker-library/php#60

@skyred
Copy link
Contributor Author

skyred commented Aug 4, 2015

When run

composer drupal-update

under Drupal/core/

It fails without php zip extension. So, we need php zip extension for Drupal 8.

@yosifkit
Copy link
Member

yosifkit commented Aug 5, 2015

With those backports, do you think it should also be added it to the drupal 7 image as well?

@skyred
Copy link
Contributor Author

skyred commented Aug 6, 2015

For D7, without Composer, the Core doesn't fail.

But, this is Composer Manager statistics https://www.drupal.org/project/usage/composer_manager

Currently, there are about 1,500 sites using Composer on D7. I expect this number to increase and more people to use docker. Therefore, I think it's a good idea to add PHP zip extension to D7 image.

@tianon
Copy link
Member

tianon commented Aug 11, 2015

Is Composer part of Drupal's core? I can't seem to find any mention of either composer or drush (which is the primary context I see mention of Composer+Drupal in my searching):

$ docker run -it --rm drupal bash
root@8f71ce34dd8d:/var/www/html# find -name '*drush*'
root@8f71ce34dd8d:/var/www/html# find -name '*composer*'
root@8f71ce34dd8d:/var/www/html# grep -rn 'composer' .
root@8f71ce34dd8d:/var/www/html# 

I think the argument that it's used by a majority of modules is compelling, but I want to make sure we don't rush into it. If it is indeed used directly by a compelling number of modules, we should likely add Composer itself to the base image as well, right?

@skyred
Copy link
Contributor Author

skyred commented Aug 11, 2015

D8 core ships with

./composer.json
./core/vendor/autoload.php

When people follow the instructions below

Workflow
Download the desired modules (such as Commerce).
Inside your core/ directory run  composer drupal-update.
This rebuilds core/composer.json and downloads the new module's requirements.
Install the modules.

https://www.drupal.org/node/2405811

This workflow will fail because of #7 (comment)

On the other hand, D7 core doesn't have files directly related to Composer

@skyred
Copy link
Contributor Author

skyred commented Sep 18, 2015

I'd like to point it out, if this image doesn't support php zip (therefore, composer digitalkaoz/composer@3862445)

The ways to install many D8 modules are:

  1. Support Is a Volume Needed? #3, so we can spin up another container with Composer and use --volumes-from=THE_CONTAINER_BUILT_ON_THIS_IMAGE, then do Composer Update in the new container. I have written an example here https://github.com/INsReady/Data-SSH-Container
  2. "docker exec -ti THE_CONTAINER_BUILT_ON_THIS_IMAGE bash", then install php zip extension

@bojanz
Copy link

bojanz commented Oct 8, 2015

Composer is here to stay, and will be the only way many modules (such as Commerce, Address) will be installable. It will also eventually replace Drush Make.

Not supporting Composer makes this solution unusable for Drupal 8, so I encourage this issue to move forward.

@yosifkit
Copy link
Member

yosifkit commented Oct 8, 2015

Sounds good to me. What do you think @tianon?

@skyred do you want to rebase this PR and apply to 7 as well?

Drupal uses Composer a lot; especially Drupal 8, and there are a lot of modules, requiring composer, are being back ported to Drupal 7. Composer use PHP zip extension. Otherwise, see docker-library/php#60

Also add zip extension to Drupal 7.
@skyred
Copy link
Contributor Author

skyred commented Oct 9, 2015

As @yosifkit suggested in #7 (comment)

The updated patch is ready for your reviewing

@yosifkit
Copy link
Member

yosifkit commented Oct 9, 2015

LGTM

1 similar comment
@tianon
Copy link
Member

tianon commented Oct 9, 2015

LGTM

tianon added a commit that referenced this pull request Oct 9, 2015
Update Dockerfile to install PHP zip extension
@tianon tianon merged commit 0092ddc into docker-library:master Oct 9, 2015
tianon added a commit to infosiftr/stackbrew that referenced this pull request Oct 14, 2015
- `docker`: 1.8.3, 1.9.0-rc1
- `docker-dev`: 1.8.3
- `drupal`: add PHP zip extension (docker-library/drupal#7)
- `httpd`: 2.4.17
- `mariadb`: 5.5.46+maria-1~wheezy
- `mongo`: 3.0.7
- `owncloud`: add FPM variants (docker-library/owncloud#28)
- `postgres`: explicit uid/gid (docker-library/postgres#93)
- `tomcat`: 8.0.28
@skyred skyred deleted the patch-1 branch October 21, 2015 05:35
RichardScothern pushed a commit to RichardScothern/official-images that referenced this pull request Jun 14, 2016
- `docker`: 1.8.3, 1.9.0-rc1
- `docker-dev`: 1.8.3
- `drupal`: add PHP zip extension (docker-library/drupal#7)
- `httpd`: 2.4.17
- `mariadb`: 5.5.46+maria-1~wheezy
- `mongo`: 3.0.7
- `owncloud`: add FPM variants (docker-library/owncloud#28)
- `postgres`: explicit uid/gid (docker-library/postgres#93)
- `tomcat`: 8.0.28
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

4 participants