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

zstd support #34

Closed
gvollant opened this issue Jul 31, 2022 · 4 comments
Closed

zstd support #34

gvollant opened this issue Jul 31, 2022 · 4 comments

Comments

@gvollant
Copy link

Hello

commit 6318ab3 removed zstd support with this text:

zstd release 1.5.1 broke the build in multiple ways.

Ref: https://ci.appveyor.com/project/curlorg/curl-for-win/builds/41967466
Ref: https://ci.appveyor.com/project/curlorg/curl-for-win/builds/41968643

@Cyan4973 @felixhandte do you have an idea on the problem ?

I was very happy to see the windows build of curl comiled with zstd 1.5.0

@vszakats
Copy link
Member

vszakats commented Jul 31, 2022

@gvollant: zstd has apparently been fixed after those fallouts. I've experimentally restored support just recently in e806006. It's now enabled in the alternate build called "big". It adds 150 KB (which is lower than in some pre-1.5.0 versions).

Every dependency is an extra weight/attention/time to carry, so is zstd support "common enough" on the server side to justify it? (I mean outside facebook properties)

(Moving brotli support to the "big" builds is also on the map, as I feel it doesn't contribute much in practice.)

@Cyan4973
Copy link

I confirm that v1.5.1 was a short-lived version that featured a subtle build issue related to the presence of a single assembly file within the source tree, resulting in weird outcomes depending on the exact platform being used during the build.

v1.5.1 was superseded within a month by v1.5.2, which is current and fixed the issue. No more build issue have been reported since this update.

@gvollant
Copy link
Author

@vszakats I understand the weight attention, but I think there is a "chicken and egg" problem between server and client for zstd (and brotli) support. So it'll be great if brotli and zstd came back on the standard windows binary distributed on the curl website.

By example, an experimental zstd nginx support was written before curl support zstd.
It probably wait more client support to become more used
https://github.com/tokers/zstd-nginx-module

vszakats added a commit that referenced this issue Aug 2, 2022
Bring back zstd support in sync with brotli, enabling it in default
builds. It is disabled in smaller-footprint builds and when building
with -nozstd.

Reported-by: Gilles Vollant
Closes #34
@vszakats
Copy link
Member

vszakats commented Aug 2, 2022

I've (re-)enabled zstd in default builds. Hoping this helps a little bit in server-side adoption.

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

No branches or pull requests

3 participants