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 9.0, "buster" aliases, and use "drupal/recommended-project" layout #176
Conversation
@skyred if you can spare some time to review this, it would be very much appreciated! 🙏 ❤️ |
Thank you for asking. I will be happy to |
8.8/apache-buster/Dockerfile
Outdated
cd /var/www/; \ | ||
# composer won't install to a non-empty dir: 'Project directory "/var/www/" is not empty.' | ||
rmdir /var/www/html; \ | ||
composer create-project --no-install "drupal/recommended-project:$DRUPAL_VERSION" ./; \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hopefully the time period between releases to https://github.com/drupal/drupal/releases (and I guess more relevantly, new version numbers presented in the update API we're scraping) and new tags on https://github.com/drupal/recommended-project/releases is short. 😅
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This repository is auto-generated based on https://git.drupalcode.org/project/drupal/-/tree/8.8.x/composer%2FTemplate%2FRecommendedProject
So, I would hope this will be updated in a timely fashion on each release.
8.8/apache-buster/Dockerfile
Outdated
# composer won't install to a non-empty dir: 'Project directory "/var/www/" is not empty.' | ||
rmdir /var/www/html; \ | ||
composer create-project --no-install "drupal/recommended-project:$DRUPAL_VERSION" ./; \ | ||
sed -i 's!web/!html/!g' composer.json; \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we maybe not do this for 9.0+? (I understand why we would do it for 8.x to keep backwards compatibility, but if it's the "recommended" layout, perhaps we should follow it for the new versions and update our Apache configuration instead?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was thinking to always do it so that we don't have the weird transition for users where they get confused that their mount for /var/www/html/modules
no longer works (https://github.com/docker-library/docs/tree/2d2443ed9d71dafa0f328b546c39f6562603ce7d/drupal#volumes)
(honestly the "recommended" layout should have some way to choose the target subdirectory)
I guess we could do both by installing to web and making html a symlink?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Letting it install where they "recommend" and having our own symlink seems sane IMO -- gives us a good balance of following their official layout and not breaking users.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can provide it like:
composer create-project --no-install "drupal/recommended-project:$DRUPAL_VERSION" /var/www/html
per the documentation:
https://www.drupal.org/docs/develop/using-composer/using-composer-to-install-drupal-and-manage-dependencies#s-create-a-project
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
or am I misunderstanding the problem?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
as far as I know, the "root" composer.json
file is "customizable" by the user (i.e. you would not use it to load modules or themes or whatever). So I think it's fine to modify it when you build it. Personally... it seems a little gross, but better than having a symlink that I might not know about....
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it might be weird if someone mounts the /var/www
folder and expects the web
subdirectory to work, but I would just document that they need to change web
to html
in their composer.json
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What it sounds like is that we're maybe looking at the problem in the wrong way -- what if we instead install to something like /opt/drupal
, and then make /var/www/html
a symlink out to /opt/drupal/web
? Then we get the best of both hacks, without all the weird edge cases:
- bind mounts into
/var/www/html/...
will still work - no Apache configuration changes required
- users can still provide all of
/var/www/html
or even use/opt/drupal
themselves to manage (and the standard "recommended" layout will Just Work)
Maybe there's even an official upstream-recommended location for a "system" install (rather than inventing /opt/drupal
ourselves)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That seems reasonable to me. That means /opt/drupal
will point to it's subdirectory web
and /var/html
wont contain the composer.json
file in the first place. :)
I do not know of a system install location convention, so whatever you think is best. Is there a convention in Docker? maybe /app
?
The documentation should probably be updated to use/mount /opt/drupal
(and I suppose that should be the WORKDIR
?) instead of /var/www
.
It would be nice if I could extend the image, and do something like:
composer require 'drupal/paragraphs:^1.12'
and have it install the Paragraphs module as is described in the documentation:
https://www.drupal.org/project/paragraphs/releases/8.x-1.12
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great example usage! After some tweaks that I'll push up shorty, this will work just fine.
$ docker build -
FROM drupal:8.9-new
RUN composer require 'drupal/paragraphs:^1.12'
Sending build context to Docker daemon 2.048kB
Step 1/2 : FROM drupal:8.9-new
---> 82e0b578fbc4
Step 2/2 : RUN composer require 'drupal/paragraphs:^1.12'
---> Running in d7ad66c57f7b
./composer.json has been updated
Loading composer repositories with package information
Updating dependencies (including require-dev)
Package operations: 2 installs, 0 updates, 0 removals
As there is no 'unzip' command installed zip files are being unpacked using the PHP zip extension.
This may cause invalid reports of corrupted archives. Besides, any UNIX permissions (e.g. executable) defined in the archives will be lost.
Installing 'unzip' may remediate them.
- Installing drupal/entity_reference_revisions (1.8.0): Downloading (100%)
- Installing drupal/paragraphs (1.12.0): Downloading (100%)
drupal/paragraphs suggests installing drupal/entity_browser (Recommended for an improved user experience when using the Paragraphs library module)
Writing lock file
Generating autoload files
23 packages you are using are looking for funding.
Use the `composer fund` command to find out more!
Removing intermediate container d7ad66c57f7b
---> fc0923aa982b
Successfully built fc0923aa982b
Basically the same diff as before but the workdir is relatively one level up (to support diff -U 4 <(docker run -i --rm drupal:8.9 sh -c 'pwd; ls -a; echo; echo '../'; ls -a ../') <(docker run -i --rm drupal:8.9-new sh -c 'echo "$PWD/web"; ls -a web/; echo; echo './'; ls -a ./')
--- /dev/fd/63 2020-08-05 15:05:43.442129748 -0700
+++ /dev/fd/62 2020-08-05 15:05:43.443129771 -0700
@@ -1,20 +1,15 @@
-/var/www/html
+/opt/drupal/web
.
..
.csslintrc
-.editorconfig
.eslintignore
.eslintrc.json
-.gitattributes
.ht.router.php
.htaccess
INSTALL.txt
-LICENSE.txt
README.txt
autoload.php
-composer.json
-composer.lock
core
example.gitignore
index.php
modules
@@ -22,11 +17,15 @@
robots.txt
sites
themes
update.php
-vendor
web.config
-../
+./
.
..
-html
+.editorconfig
+.gitattributes
+composer.json
+composer.lock
+vendor
+web |
Just copying the excellent use case/example from #176 (comment) so we don't lose it; this will make things like the following possible: FROM drupal:8.9
RUN composer require 'drupal/paragraphs:^1.12' |
Changes: - docker-library/drupal@201b7c0: Update to 8.9.3 - docker-library/drupal@be98c13: Update to 9.0.3 - docker-library/drupal@e7d2cad: Merge pull request docker-library/drupal#176 from infosiftr/9.0-rc - docker-library/drupal@530833e: Move installation to /opt/drupal; symlink from /var/www/html - docker-library/drupal@4a5ce8d: Keep templates for 7, minor 8&9 fixes - docker-library/drupal@e2c7e05: Update generated README - docker-library/drupal@999ac25: Skip --from files when finding the commit - docker-library/drupal@0785264: Reorganize to be explicit about Alpine version as well as Debian release - docker-library/drupal@71ffa1c: Add drupal 9.0 release
Changes: - docker-library/drupal@8620ae0: Fix Architectures calculation to take *all* FROM values into account - docker-library/drupal@201b7c0: Update to 8.9.3 - docker-library/drupal@be98c13: Update to 9.0.3 - docker-library/drupal@e7d2cad: Merge pull request docker-library/drupal#176 from infosiftr/9.0-rc - docker-library/drupal@530833e: Move installation to /opt/drupal; symlink from /var/www/html - docker-library/drupal@4a5ce8d: Keep templates for 7, minor 8&9 fixes - docker-library/drupal@e2c7e05: Update generated README - docker-library/drupal@999ac25: Skip --from files when finding the commit - docker-library/drupal@0785264: Reorganize to be explicit about Alpine version as well as Debian release - docker-library/drupal@71ffa1c: Add drupal 9.0 release
Fixes #169
Closes #175
The
drupal/recommended-project
is adjusted to usehtml/
instead ofweb/
so there is no effect to8.8
and8.9
exceptvendor
(andcomposer.{json,lock}
) moving to/var/www
:New tags: