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

lz4 cython bindings #3601

Closed
totaam opened this issue Aug 15, 2022 · 2 comments
Closed

lz4 cython bindings #3601

totaam opened this issue Aug 15, 2022 · 2 comments
Labels
enhancement New feature or request

Comments

@totaam
Copy link
Collaborator

totaam commented Aug 15, 2022

https://github.com/python-lz4/python-lz4 has had 74 releases and 1144 commits as of now!

$ find lz4 -type f -exec cat {} \; | wc -l
5091

Over 5000 lines of C and python code for just 2 API calls we actually need?
On top of that, installing it requires extra tools that make it more difficult to package.

Perhaps use this as an opportunity to drop the redundant size header from packets?
(xpra already sends the size of the packet)

@totaam totaam added the enhancement New feature or request label Aug 15, 2022
totaam added a commit that referenced this issue Aug 17, 2022
totaam added a commit that referenced this issue Aug 17, 2022
inefficiently since this packet encoder is deprecated
totaam added a commit that referenced this issue Aug 17, 2022
@totaam
Copy link
Collaborator Author

totaam commented Aug 17, 2022

So long python-lz4, you won't be missed!
Especially in the MacOS builds: https://github.com/Xpra-org/gtk-osx-build/blob/0f8b9600f935dfb620ba3464117b0d9393681ccd/xpra-python3.modules#L409-L416 :
We can get rid of the dependencies on python3-pkgconfig, python3-deprecation, python3-test-runner and python3-setuptools-scm.

Edit: I was wrong about the header being redundant, we only know the compressed size not the uncompressed size.

@totaam totaam closed this as completed Aug 17, 2022
totaam added a commit that referenced this issue Aug 17, 2022
totaam added a commit to Xpra-org/gtk-osx-build that referenced this issue Aug 18, 2022
totaam added a commit that referenced this issue Aug 18, 2022
totaam added a commit that referenced this issue Aug 27, 2022
totaam added a commit that referenced this issue Aug 27, 2022
@totaam
Copy link
Collaborator Author

totaam commented Aug 30, 2022

... and of course Debian Buster is shipping an out of date version of lz4:

xpra/net/lz4.c: In function ‘__pyx_pf_4xpra_3net_3lz4_10compressor___init__’:
xpra/net/lz4.c:1543:3: error: implicit declaration of function ‘LZ4_resetStream_fast’; did you mean ‘LZ4_resetStreamState’? [-Werror=implicit-function-declaration]
   LZ4_resetStream_fast((&__pyx_v_self->state));
   ^~~~~~~~~~~~~~~~~~~~
   LZ4_resetStreamState

We may need a Debian specific patch to get this to build on Buster.

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

No branches or pull requests

1 participant