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.

Sign up for GitHub

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 ImageMagick, ghostscript, bc_math and PDF thumbnails #1292

Merged
merged 2 commits into from
May 28, 2021
Merged

Add ImageMagick, ghostscript, bc_math and PDF thumbnails #1292

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e5cd61e
Replace GD with ImageMagick; Add bc_math extension and ghostscript
joshuafredrickson May 21, 2021
d924947
Enable PDF thumbnail generation
joshuafredrickson May 21, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions roles/common/defaults/main.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,10 @@ _apt_packages_default:
build-essential: "{{ apt_package_state }}"
curl: "{{ apt_package_state }}"
dbus: "{{ apt_package_state }}"
ghostscript: "{{ apt_package_state }}"
git: "{{ apt_package_state }}"
imagemagick: "{{ apt_package_state }}"
libgs-dev: "{{ apt_package_state }}"
libnss-myhostname: "{{ apt_package_state }}"
python: "{{ apt_package_state }}"
unzip: "{{ apt_package_state }}"
Expand Down
8 changes: 8 additions & 0 deletions roles/php/tasks/main.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,3 +46,11 @@
src: php-cli.ini.j2
dest: /etc/php/{{ php_version }}/cli/php.ini
mode: '0644'

- name: Change ImageMagick policy.xml to allow for PDFs
replace:
path: /etc/ImageMagick-6/policy.xml
regexp: '<policy domain="coder" rights="none" pattern="PDF" />'
replace: '<policy domain="coder" rights="read" pattern="PDF" />'
backup: no
notify: reload php-fpm
2 changes: 1 addition & 1 deletion roles/php/vars/7.4.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ php_extensions_default:
php7.4-curl: "{{ apt_package_state }}"
php7.4-dev: "{{ apt_package_state }}"
php7.4-fpm: "{{ apt_package_state }}"
php7.4-gd: "{{ apt_package_state }}"
Copy link
Member

@tangrufus tangrufus May 29, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: Removing this line doesn't actually uninstall php7.4-gd. php7.4-gd would linger around if a server is already provisioned before this PR.

php7.4-gd: absent is required to ensure it is being uninstalled.

Although I don't see any damage if having both php7.4-gd and php7.4-imagick, hard-to-reproduce tickets might come up in the future.

Same goes to php8.0-gd.

cc @swalkinshaw

Copy link
Contributor Author

@joshuafredrickson joshuafredrickson May 29, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a good point. This would only keep php-gd from being added during provisioning. I agree that there shouldn't be issues with having php-gd and php-imagick installed concurrently on existing deployments.

As of 3.5, WordPress will default to Imagick if available and fallback to GD as well as adds a filter for overriding this behavior.

Would having both extensions always available be the more preferable route here?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is kind of expected with Trellis by now and is mostly consistent with how we do other package changes. We don't uninstall PHP version packages either for example.

I'm not against adding php7.4-gd: absent though.

php7.4-imagick: "{{ apt_package_state }}"
php7.4-intl: "{{ apt_package_state }}"
php7.4-mbstring: "{{ apt_package_state }}"
php7.4-mysql: "{{ apt_package_state }}"
Expand Down
2 changes: 1 addition & 1 deletion roles/php/vars/8.0.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ php_extensions_default:
php8.0-curl: "{{ apt_package_state }}"
php8.0-dev: "{{ apt_package_state }}"
php8.0-fpm: "{{ apt_package_state }}"
php8.0-gd: "{{ apt_package_state }}"
php8.0-imagick: "{{ apt_package_state }}"
php8.0-intl: "{{ apt_package_state }}"
php8.0-mbstring: "{{ apt_package_state }}"
php8.0-mysql: "{{ apt_package_state }}"
Expand Down