JuliaIO / GZip.jl

A Julia interface for gzip functions in zlib
https://juliaio.github.io/GZip.jl/dev
MIT License
39 stars 30 forks source link

Get position of GZip.Stream in compressed bytes #27

Closed quinnj closed 9 years ago

quinnj commented 9 years ago

This PR changes position to position(::GZip.Stream,of_raw::Bool=false), with position(f,true) returning the value of gzoffset.

My use case is splitting gzipped files into smaller chunks by entire lines, so I open a gzip file, read a line, then write it out to a new GZip.Stream. In the mean time, I need to track how many compressed bytes my new gzip file is to know when to close it.

kmsquire commented 9 years ago

I think you need to rebase this off of origin/master.

quinnj commented 9 years ago

Yeah, I got the commits a little mixed up. Should be fixed now.

quinnj commented 9 years ago

Ok, think I finally got it figured out.

coveralls commented 9 years ago

Coverage Status

Coverage decreased (-17.41%) to 54.67% when pulling 88d438c78dbd447b6387e5b9bafc4d60e25ddb2f on jq/compressedposition into dbd40e5f7d0e829ff53066965e6789117c0d61a1 on master.

coveralls commented 9 years ago

Coverage Status

Coverage decreased (-17.41%) to 54.67% when pulling 88d438c78dbd447b6387e5b9bafc4d60e25ddb2f on jq/compressedposition into dbd40e5f7d0e829ff53066965e6789117c0d61a1 on master.

kmsquire commented 9 years ago

When I run this locally, I get:

julia> Pkg.test("GZip")
INFO: Testing GZip
ERROR: LoadError: test failed: (1775 == 5765)
 in expression: position(gzfile,true) == pos
 in error at error.jl:21
 in default_handler at test.jl:29
 in do_test at test.jl:52
 in include at ./boot.jl:252
 in include_from_node1 at loading.jl:134
 in process_options at ./client.jl:310
 in _start at ./client.jl:409
while loading /Users/kevin/.julia/v0.4/GZip/test/runtests.jl, in expression starting on line 96
quinnj commented 9 years ago

As far as I can tell, it was version 1.2.3 where they re-orged a lot of the repository and introduced gzoffset; that was back in 2011.

kmsquire commented 9 years ago

(Ignore the 5765's--I had added some debug print statments. Removed inline on github.)

kmsquire commented 9 years ago

Maybe there's no gzoffset64 then? At least in the version on github?

https://travis-ci.org/JuliaLang/GZip.jl/jobs/61714711#L129

quinnj commented 9 years ago

Ok, fixed the test, renamed the arg to raw and added GZLIB_VERSION and made calling gzoffset conditional on having a recent enough version of the library. Hopefully that will do it?

coveralls commented 9 years ago

Coverage Status

Coverage decreased (-21.75%) to 50.33% when pulling 2dcb99a8d9bb3348c3ba80776e549467aec2c0e0 on jq/compressedposition into dbd40e5f7d0e829ff53066965e6789117c0d61a1 on master.

coveralls commented 9 years ago

Coverage Status

Coverage decreased (-17.77%) to 54.3% when pulling 2dcb99a8d9bb3348c3ba80776e549467aec2c0e0 on jq/compressedposition into dbd40e5f7d0e829ff53066965e6789117c0d61a1 on master.

quinnj commented 9 years ago

Weird, not sure why it's not finding gzoffset64. I'm seeing that defined all the way back to when gzoffset was first introduced.

coveralls commented 9 years ago

Coverage Status

Coverage decreased (-0.47%) to 71.61% when pulling 36a7819394a2b1610f808ea5262770804ee683dc on jq/compressedposition into dbd40e5f7d0e829ff53066965e6789117c0d61a1 on master.

quinnj commented 9 years ago

Good to merge?

kmsquire commented 9 years ago

Would you be willing to change all of the versions from strings to Versions? I don't think the Version type existed when this was originally written, and Versions would make more sense when testing, well, versions.

quinnj commented 9 years ago

The problem is that VersionNumber doesn't support 1.2.3.9, only 1.2.3. I figured it was better to be more accurate vs. truncate the version number.

kmsquire commented 9 years ago

Ah, right, forgot about that limitation. I proposed once to extend Version (for BinDeps/Debian versioning), and that wasn't accepted, but it would be great to have extended version functionality in a package so we could use it for these things.

quinnj commented 9 years ago

Yeah, seems like that would be good to have. Thanks for merging. I'll tag a version in METADATA.