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

Add pdf+other image preview support to alpine images after Alpine's packaging change #2248

Conversation

jessebot
Copy link
Contributor

I'm also creating this PR to add PDF preview support, which doesn't work without the imagemagick-pdf package. This is in reference to #2246 (comment):

But let's also create an issue now (or, better yet, WIP PR) to propose & firm up what others should also be added back in, in a follow-up[1].

[1] We probably want to keep parity. Is there anything that doesn't belong or should we just load all of them back in to mirror the old packaging?

Cc: @felix-egli

We could also combine this PR with #2246 since we missed the release for 29.0.3 this time anyway. Open to either way, or if I misunderstood, please feel free to give me gentle feedback and I'll remedy what's needed! 🙏

Also open to adding additional packages. svg and pdf are just the file formats that stuck out to me from the beginning.

@joshtrichards
Copy link
Member

Per https://git.alpinelinux.org/aports/commit/community/imagemagick/imagemagick.post-upgrade?h=3.19-stable&id=61dce5f6a84d9f96ab7aa6437f12d2845b572ed8 these are our options:

+	* imagemagick support for various modules was split into subpackages.
+	* they autoinstall with the requisite library already installed.
+	* if not already present, install the module you want to use manually:
+	* (prefixed with imagemagick- )
+	* -heic -jpeg -pdf -raw
+	* -svg -tiff -webp -jxl
+	* if you want to exclude the support regardless, use e.g. 'apk add !imagemagick-pdf'

Looking at the formats supported in our Debian images, looks like we do all of these except for jxl. I guess we should include them all save for that one? 🤔

since we missed the release for 29.0.3 this time anyway

Yeah. :-/ Up to you whether to combine them or not. Doesn't matter either way on this side now.

@jessebot
Copy link
Contributor Author

jessebot commented Jun 28, 2024

Looking at the formats supported in our Debian images, looks like we do all of these except for jxl. I guess we should include them all save for that one? 🤔

Sure, I'll add the other formats you listed 👍 -except for jxl (what even is jxl 🤷 ).

Yeah. :-/ Up to you whether to combine them or not. Doesn't matter either way on this side now.

I'll leave em' separate for now.

@jessebot jessebot force-pushed the add-imagemagick-pdf-support-for-alpine branch from 0b66ab4 to 437a2e0 Compare June 28, 2024 13:10
@jessebot jessebot marked this pull request as ready for review June 28, 2024 13:10
@jessebot jessebot changed the title WIP: add pdf image preview support to alpine images by adding imagemagick-pdf Add pdf image preview support to alpine images by adding imagemagick-pdf Jun 28, 2024
@joshtrichards
Copy link
Member

(what even is jxl 🤷 )

I had the exact same reaction. 😆

@joshtrichards
Copy link
Member

Ah, crap. It thinks there's a conflict now due to the one line change from the svg PR since that just got merged. 😜 Can you resolve however you prefer and then let's get this one in too.

@jessebot
Copy link
Contributor Author

Ah, crap. It thinks there's a conflict now due to the one line change from the svg PR since that just got merged. 😜 Can you resolve however you prefer and then let's get this one in too.

No problem, that's what I get for not combining the PRs haha :) Let me know if you need anything else!

@joshtrichards joshtrichards changed the title Add pdf image preview support to alpine images by adding imagemagick-pdf Add pdf+other image preview support to alpine images after Alpine's packaging change Jul 2, 2024
@joshtrichards
Copy link
Member

Wow, all tests passed without having to kick them a few times to re-run. That's rare, but I'll take it!

@joshtrichards joshtrichards merged commit 7d0795c into nextcloud:master Jul 2, 2024
21 checks passed
@jessebot
Copy link
Contributor Author

jessebot commented Jul 2, 2024

Amazing, and I agree very rare 😁

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants