-
Notifications
You must be signed in to change notification settings - Fork 102
zstd 1.3 -> zstd 1.3.3 #23
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
Conversation
|
Hi @ghatdev thanks for your PR! |
|
oops I missed it. I just did! --- FAIL: TestZstdReaderLong (0.00s) goroutine 8 [running]: |
|
I checked compress, decompress function only but they worked. |
|
it seems to be some changes on ZSTD_decompress |
|
What's status of this PR? |
|
Hi, I haven't looked at updating it yet myself, I'll have more time the next upcoming week. |
|
Thanks for status update. I use this zstd bindings for a tool creating tar.zstd files (https://github.com/develar/zstd-archiver) and it works great, but 1.3.3 brings multirthreads and long range matching. |
|
Hi @ghatdev , I was able to take some time this morning to debug your PR. I added a PR to fix (== completly get rid of) the static mapping of error code between Go and C: #29 I also pulled your branch and was able to make everything pass: #28 Basically:
Once this is done, I can merge that PR 👍 |
|
Ah github might be too aggressive when pruning loose objects (using I re-pushed to 1697446 I think in your 5c04739 you missed (Build should be passing here: https://travis-ci.org/DataDog/zstd/builds/353765142) |
|
Great thanks a lot for your contribution! Looks good now and tests are passing. |
|
@Viq111 I'm not good at using git :(.. Sorry about it. |
@Viq111 Hint — you can merge this PR using "Squash & merge" to have one rebased commit. |
4f81c11 to
9269a8a
Compare
We have this function in Go code to not have to do a cgo call (really consuming). This function in a hot loop, that's why we can't call C code every time. This mirrors latest zstd implementation (they switched to 128kB window)
Viq111
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perfect! Thanks a lot for the PR!
|
@develar I usually don't like squashing because you loose git history (why a change was made). |
No description provided.