Switch to libbz2-rs-sys and finish bz2 impl
#5709
Conversation
|
See also #5605 |
|
This one is better than mine ... |
|
please check #5605 and rebase on it or pick some idea if possible |
|
Oh, shoot, sorry - I didn't see that that PR was open. |
| @@ -12,28 +12,48 @@ mod _bz2 { | |||
| object::{PyPayload, PyResult}, | |||
| types::Constructor, | |||
| }; | |||
| use crate::zlib::{ | |||
There was a problem hiding this comment.
Since there is at-least 1 other decompression module that uses the same format (_lzma). I think this should be moved someplace common.
There was a problem hiding this comment.
I agree - I was planning on doing that in a followup, to reduce the amount of code movement in this pr.
| @@ -1001,6 +1005,8 @@ def test_encoding_error_handler(self): | |||
| as f: | |||
| self.assertEqual(f.read(), "foobar") | |||
|
|
|||
| # TODO: RUSTPYTHON | |||
| @unittest.expectedFailure | |||
There was a problem hiding this comment.
@coolreader18 @arihant2math what cause this regression? could this be fixed in future?
There was a problem hiding this comment.
A flag probably needs to be added to the decompressor state. I believe the issue is the wt and rt formats. Although I'm not to sure, I suppose looking at the cpython source might help.
There was a problem hiding this comment.
The reason this needed to be flagged now is because the whole test file wasn't running at all before - bz2 was not included in the --features=stdlib,threading,... flag in CI, so the module wasn't even getting compiled or tested at all.
There was a problem hiding this comment.
Oh, didn't noticed that. Thanks!
|
I checkout out of this pr for #5717 so try not to change the trait too much 😄 . |
Co-authored-by: Ashwin Naren <arihant2math@gmail.com>
| data1: &'a [u8], | ||
| data2: &'a [u8], | ||
| } | ||
| impl<'a> Chunker<'a> { | ||
| fn new(data: &'a [u8]) -> Self { | ||
| pub(crate) fn new(data: &'a [u8]) -> Self { |
There was a problem hiding this comment.
| pub(crate) fn new(data: &'a [u8]) -> Self { | |
| pub fn new(data: &'a [u8]) -> Self { |
When the struct is pub(crate), only pub here is automatically pub(crate)
There was a problem hiding this comment.
True, I think I'll change that in the follow-up.
I realized that
libbz2-rs-sysexists, by the same folks who makelibz-rs-sys- it's a Rust reimplementation oflibbz2. This means we can do that same thing as in #5562, and avoid having to cross-compile C. I then realized after I removed the feature flag that we weren't actually running bz2 regrtests in CI, because we weren't passing it as a feature to cargo build, and so something like half of the tests intest_bz2.pywere failing. So, I more or less finished up the impl, by making some code inzlib.rsgeneric over the specificDecompressstruct.