set_params is (semi)-broken

[valarauca]

Issue:

The data placed into CompressParams doesn't always apply to the underlying compressor.

Example:

let mut p = CompressParams::new();
p.mode(CompressMode::Text);
p.quality(11);
p.lgwin(24);
p.lgblock(24);
let mut r = BrotliEncoder::new(r, 6);
r.set_params(p);

/* Do encryption */

The above code will give the same result as if set_params is never called. Ultimately the only tunable for Read/Write/BufReader objects are the created level: u32 value.

This patch proposes a change in API instead of

let mut e = Encoder::new();
e.set_params();

This would do

let mut e = Encoder::from_params(r, params);

This patch fixes the issue as is.

[alexcrichton]

Thanks for the PR! Does this mean that Compress::set_params is also wrong? I suppose I'm a little unclear on what the bug here is in terms of why the underlying FFI calls are being ignored?

[valarauca]

From what I can only the initial set_params call applies values. A secondary set_params calls to override an existing argument may not.

I threw together a tool for brotli compression on msvc. Before this fix no matter how I set its flags it would always preform the same compression level.

I'm at a loss to to be completely honest, I really have no clue how the original codebase functions. Just these patches fixed my tool ¯\(ツ)/¯

---

Also while I have your attention I upgraded the xz2-rs library to build with MSVC2014 toolchain, do you want that PR?

Apparently the default install of C/C++ tools with VS community edition doesn't support the MSVC2012 toolchain (anymore(?)).

[alexcrichton]

Ok I'll dig around the brotli source tree and see what's up.

Also yeah I'd love a PR to xz2, sounds great!

[alexcrichton]

Aha yeah looks like on-the-fly configuration is not supported

[alexcrichton]

Ok just one minor comment but otherwise looks great!

[valarauca]

I've updated src/stream.rs and src/read.rs and src/bufread.rs with the suggested changes.

I've also made the CompressionMode enum have the Copy, Clone, and Debug traits. This just a small quality of life change for building abstractions on top of the library.

[alexcrichton]

Thanks!