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 PHP bz2 extension #1351

Closed
wants to merge 1 commit into from
Closed

Add PHP bz2 extension #1351

wants to merge 1 commit into from

Conversation

javabean
Copy link

@javabean javabean commented Jan 1, 2021

Some Nextcloud apps require the bz2 PHP extension. This pull request adds bz2 into the official Docker image.

Signed-off-by: Cédrik LIME <REMOVE_ME+github@cedrik.fr>
@J0WI
Copy link
Contributor

J0WI commented Jan 8, 2021

We can't support every app: https://github.com/nextcloud/docker/tree/master/.examples#dockerfiles

@javabean
Copy link
Author

javabean commented Jan 8, 2021

That is very true, we can’t support every app!
But this bz2 module is a bit special, since it is recommended by the installation manual, we would expect it to be included in the official Docker image. Heck, even bcmath, gmp and exif are there, and they are only in the optional section! ;-)

TL;DR: bz2 is part of the Nextcloud recommended PHP modules; omitting it in the official Docker image seems like an oversight.

I promise I won’t come back and ask for LibreOffice to be included! :-p

@J0WI
Copy link
Contributor

J0WI commented Jan 13, 2021

Do you have an example for an affected app?

@javabean
Copy link
Author

javabean commented Jan 18, 2021

The Facerecognition application is one.
And consistency between Nextcloud documentation and the official Docker image is also a must (see my previous comment)!

@J0WI
Copy link
Contributor

J0WI commented Jan 22, 2021

This app also requires PDlib, so you'd need to customize the Dockerfile anyway.

@javabean
Copy link
Author

From your comments, I wonder if I got it all wrong since the begining. From the documentation:

Recommended packages:
PHP module bz2 (recommended, required for extraction of apps)

If we don’t need bz2 to extract apps, then maybe I should contribute a patch on the documentation instead.

What is the official status on bz2 PHP extension: required for apps extraction or not? If it is, I believe it should be included. If it is not, that I shall close this PR and submit a documentation patch instead.
Sorry if I sound confused (I am!), will you please enlighten me? :-)

@J0WI
Copy link
Contributor

J0WI commented Jan 25, 2021

What is the official status on bz2 PHP extension: required for apps extraction or not?

I don't know. Maybe someone from @nextcloud/appstore could comment on the current status of bz2 apps.

@BernhardPosselt
Copy link
Member

Only gz is supported https://github.com/nextcloud/appstore/blob/master/nextcloudappstore/api/v1/release/parser.py#L82

@javabean
Copy link
Author

Only gz is supported

Thanks for your answer. Can you confirm then that the documentation needs to remove reference to bz2 PHP extension?
(Just double-checking here to avoid mistakes!)

@BernhardPosselt
Copy link
Member

No, can't confirm. You'd need to confirm that with people from the server repo. They might be using the extension for something else.

@javabean
Copy link
Author

@BernhardPosselt @J0WI do you know any contact @nextcloud/server that could enlighten us as to whether the bz2 PHP extension is needed for them?

@J0WI
Copy link
Contributor

J0WI commented Feb 1, 2021

Maybe @nextcloud/documentation? I couldn't find any other usage of bz2 in the server repo.

@markuman
Copy link

markuman commented Feb 24, 2021

https://github.com/nextcloud/server/blob/f552f23e437371b1a34c7df0fc7a11004f4cc1c2/lib/private/Archive/TAR.php#L56

Even when nextcloud itself always request for .gz files, the nextcloud server itself can also handle bz2 files (install apps or update nextcloud itself).

Maybe you should consider to use it. Your servers are on fire with every release. Switch to bz2 (or better zstd) will reduce your egress traffic.

@J0WI
Copy link
Contributor

J0WI commented Mar 1, 2021

Switch to bz2 (or better zstd) will reduce your egress traffic.

The Docker image uses tar.bz2 but everything else needs to be done in nextcloud/server.

@javabean
Copy link
Author

javabean commented Mar 1, 2021

@J0WI So shouldn’t we enable bz2 PHP module, since Nextcloud server has a code path that expects it to be available?

@drsassafras
Copy link

bz2 is used extensively and is becoming more and more common. It should not be a question of if one of the best and most common ways to compress data should be supported.

@kale1d0code
Copy link

Can the error messages for bz2 and imagick be removed?
I am using the official image, if the official image doesn't deem then necessary, it shouldn't be an error message.

@h0lley
Copy link

h0lley commented Feb 14, 2024

Can the error messages for bz2 and imagick be removed? I am using the official image, if the official image doesn't deem then necessary, it shouldn't be an error message.

it already is. read here.

@joshtrichards
Copy link
Member

Closing. No longer applicable.

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

Successfully merging this pull request may close these issues.

8 participants