r-lib / archive

R bindings to libarchive, supporting a large variety of archive formats
https://archive.r-lib.org/
Other
145 stars 17 forks source link

expose zstd filter to R #19

Closed coolbutuseless closed 6 years ago

coolbutuseless commented 6 years ago

Enable 'zstd' as a filter.

I'm unsure which exact ARCHIVE_VERSION_NUMBER this was added to libarchive, and I don't know how to find it out. So i've lumped it in with the same version number when LZ4 was added for now.

Any guidance appreciated.

jimhester commented 6 years ago
screen shot 2018-09-27 at 7 38 08 am

It was added in https://github.com/libarchive/libarchive/commit/f59fec02545917911a632ccff2d2305f0d65b401 which looks like it is only available in 3.3.3

So you need a #if ARCHIVE_VERSION_NUMBER >= 3003003 define.

codecov[bot] commented 6 years ago

Codecov Report

Merging #19 into master will decrease coverage by 2.04%. The diff coverage is 67.5%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #19      +/-   ##
==========================================
- Coverage   86.51%   84.46%   -2.05%     
==========================================
  Files          11       11              
  Lines         556      573      +17     
==========================================
+ Hits          481      484       +3     
- Misses         75       89      +14
Impacted Files Coverage Δ
src/archive.cpp 96.42% <ø> (ø) :arrow_up:
R/archive.R 53.96% <0%> (-7.94%) :arrow_down:
R/utils.R 46.15% <0%> (+1.7%) :arrow_up:
src/read_file.cpp 96.82% <100%> (ø) :arrow_up:
src/read.cpp 86.45% <100%> (+0.43%) :arrow_up:
src/r_archive.cpp 71.05% <60.86%> (-23.4%) :arrow_down:
R/archive_files.R 100% <0%> (ø) :arrow_up:
R/archive_connections.R 100% <0%> (ø) :arrow_up:
... and 2 more

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 11e65d7...ccb3781. Read the comment docs.

coolbutuseless commented 6 years ago

I tried adding ".zst" and ".tar.zst" to your suite of tests, but appveyor said they failed, so I've removed them for now.

any thoughts?

jimhester commented 6 years ago

It looks like you need to rebase this on the current master. Something like the following should get you there

# assumes my repo is called upstream and you are on your local zstd branch
git fetch upstream
git rebase -i upstream/master
# fix any rebase conflicts, add them and and git rebase --continue
git push --force-with-lease
jimhester commented 6 years ago

The windows archive binary is not built with support for zstandard, so it will not work. You will need to skip the tests if the archive version is older than 3.3.3.