Recommend MultiGzDecoder over GzDecoder in docs #324

jsha · 2022-10-31T03:36:44Z

Part of #178.

I removed the text that says "the specification, however, allows ..." and the comment about bioinformatics because they make it sound like MultiGzDecoder is a rare thing that you should only enable if you need it, but it's actually the correct choice for almost all cases since it implements what the RFC considers a "gzip file."

src/gz/bufread.rs

jsha · 2023-07-13T20:39:57Z

I see you've also made some tweaks to these docs in #347. Thanks! I went ahead and updated this PR, and I still think it's worthwhile.

In this PR I reference the concept of the "gzip file" instead of multistream, which is what the RFC talks about as the core concept (with "members" being a sub-concept).

Also, I removed the list of cases where gzip files containing multiple members are commonly used, since that seems to suggest that gzip files containing multiple members are only used in particular endeavors, when really they can be used any time you have a .gz file or a Content-Encoding: gzip HTTP response.

Similarly, I removed the comment "Use [MultiGzDecoder] if your file has multiple streams" because in many cases the user of this library doesn't know in advance if they will be processing a file with multiple streams or not; but it is always correct to use a MultiGzDecoder because it will handle a single-stream file just fine.

Byron

Thanks a lot!

I feel the narrative is now inverted, and the way it should be, by making MultiGzDecoder effectively the default. To me it feels like ultimately, MutliGzDecoder really wants to be the default also by name, as right now it still feels special in that regard. Of course, I am not proposing to make name changes to types in this PR, which already gets the documentation to a place where most people who want to use GzDecoder will end up using the MutiGzDecoder instead.

src/gz/bufread.rs

Byron · 2023-07-17T07:12:58Z

I think it would be nice if @JohnTitor could have another look. Overall, I believe this PR would help to get a better understanding on the intended use of MultiGzDecoder, and I would definitely have benefitted from that while working with #348 . So I see a lot of value in getting it merged. Thank you.

joshtriplett · 2023-07-21T01:10:47Z

Whether the specification supports it or not, in many cases I think people will be surprised if they encounter a gzip stream with multiple files in it. I think that people typically think of gzip as a single-file format, and combine it with things like tar when storing multiple files.

So, I don't think we should go as far as saying that most people should use MultiGzDecoder. I think it's perfectly fine to adjust the documentation to say that it's allowed, and explain the distinction, but I think we should actually explain the distinction and the common usage rather than presenting MultiGzDecoder as the most common case.

joshtriplett · 2023-07-21T01:12:31Z

src/gz/write.rs

-/// bioinformatics, for example when using the BGZF compressed data.
+/// A gzip file consists of a series of "members" concatenated one after another.
+/// MultiGzDecoder decodes all members of a file, while [GzDecoder] will only decode
+/// the first member. MultiGzDecoder is preferable in most cases.


Suggested change

/// the first member. MultiGzDecoder is preferable in most cases.

/// the first member. Many gzip files in the wild will typically have one member,

/// but not all. Use MultiGzDecoder if you need to handle multiple members, or

/// [GzDecoder] if you know you only need to handle a single member.

~~I have applied this change here and to the other 2 places where the same text appears.~~

Edit: I reverted the change as it is contended - din't read the messages below until now.

joshtriplett · 2023-07-21T01:13:05Z

src/gz/write.rs

@@ -373,17 +373,16 @@ impl<W: Read + Write> Read for GzDecoder<W> {
    }
 }

-/// A gzip streaming decoder that decodes all members of a multistream
+/// A gzip streaming decoder that decodes a full [gzip file].


Suggested change

/// A gzip streaming decoder that decodes a full [gzip file].

/// A gzip streaming decoder that decodes a [gzip file] with multiple members.

joshtriplett · 2023-07-21T01:14:16Z

src/gz/write.rs

+/// A decoder for a single member of a gzip file. Prefer [MultiGzDecoder] for
+/// most uses.


Suggested change

/// A decoder for a single member of a gzip file. Prefer [MultiGzDecoder] for

/// most uses.

/// A decoder for a gzip file with a single member.

I'd agree that we shouldn't just present this as a decoder for gzip in general, and should capture the distinction in the summary, but I'd stop short of telling people to prefer MultiGzDecoder.

joshtriplett · 2023-07-21T01:15:27Z

src/gz/write.rs

 ///
 /// This structure exposes a [`Write`] interface that will emit uncompressed data
 /// to the underlying writer `W`.
-/// Use [`MultiGzDecoder`] if your file has multiple streams.
 ///


Suggested change

///

///

/// This decoder only handles gzipped data with a single stream.

/// Use [`MultiGzDecoder`] for gzipped data with multiple streams.

///

joshtriplett · 2023-07-21T01:16:17Z

src/gz/read.rs

-/// for a chunk of input data.
+/// A gzip file consists of a series of "members" concatenated one after another.
+/// MultiGzDecoder decodes all members of a file, while [GzDecoder] will only decode
+/// the first member. MultiGzDecoder is preferable in most cases.


Suggested change

/// the first member. MultiGzDecoder is preferable in most cases.

/// the first member. Many gzip files in the wild will typically have one member,

/// but not all. Use MultiGzDecoder if you need to handle multiple members, or

/// [GzDecoder] if you know you only need to handle a single member.

joshtriplett · 2023-07-21T01:17:06Z

src/gz/bufread.rs

-/// for a chunk of input data.
+/// A gzip file consists of a series of "members" concatenated one after another.
+/// MultiGzDecoder decodes all members of a file, while [GzDecoder] will only decode
+/// the first member. MultiGzDecoder is preferable in most cases.


Suggested change

/// the first member. MultiGzDecoder is preferable in most cases.

/// the first member. Many gzip files in the wild will typically have one member,

/// but not all. Use MultiGzDecoder if you need to handle multiple members, or

/// [GzDecoder] if you know you only need to handle a single member.

joshtriplett · 2023-07-21T01:17:32Z

src/gz/bufread.rs

+/// A decoder for a single member of a gzip file. Prefer [MultiGzDecoder] for
+/// most uses.


Suggested change

/// A decoder for a single member of a gzip file. Prefer [MultiGzDecoder] for

/// most uses.

/// A decoder for a gzip file with a single member.

I'd agree that we shouldn't just present this as a decoder for gzip in general, and should capture the distinction in the summary, but I'd stop short of telling people to prefer MultiGzDecoder.

joshtriplett · 2023-07-21T01:17:44Z

src/gz/read.rs

+/// A decoder for a single member of a gzip file. Prefer [MultiGzDecoder] for
+/// most uses.


Suggested change

/// A decoder for a single member of a gzip file. Prefer [MultiGzDecoder] for

/// most uses.

/// A decoder for a gzip file with a single member.

I'd agree that we shouldn't just present this as a decoder for gzip in general, and should capture the distinction in the summary, but I'd stop short of telling people to prefer MultiGzDecoder.

joshtriplett · 2023-07-21T01:18:11Z

src/gz/bufread.rs

@@ -397,20 +397,16 @@ impl<R: BufRead + Write> Write for GzDecoder<R> {
    }
 }

-/// A gzip streaming decoder that decodes all members of a multistream
+/// A gzip streaming decoder that decodes a complete [gzip file].


Suggested change

/// A gzip streaming decoder that decodes a complete [gzip file].

/// A gzip streaming decoder that decodes a [gzip file] with multiple members.

joshtriplett · 2023-07-21T01:18:21Z

src/gz/read.rs

@@ -180,21 +178,16 @@ impl<R: Read + Write> Write for GzDecoder<R> {
    }
 }

-/// A gzip streaming decoder that decodes all members of a multistream
+/// A gzip streaming decoder that decodes a full [gzip file].


Suggested change

/// A gzip streaming decoder that decodes a full [gzip file].

/// A gzip streaming decoder that decodes a [gzip file] with multiple members.

joshtriplett · 2023-07-21T01:19:03Z

src/gz/read.rs

 ///
 /// This structure exposes a [`Read`] interface that will consume compressed
 /// data from the underlying reader and emit uncompressed data.
-/// Use [`MultiGzDecoder`] if your file has multiple streams.
-///
-/// [`Read`]: https://doc.rust-lang.org/std/io/trait.Read.html
 ///


Suggested change

///

///

/// This decoder only handles gzipped data with a single stream.

/// Use [`MultiGzDecoder`] for gzipped data with multiple streams.

///

joshtriplett · 2023-07-21T01:19:31Z

src/gz/bufread.rs

 ///
 /// This structure consumes a [`BufRead`] interface, reading compressed data
 /// from the underlying reader, and emitting uncompressed data.
-/// Use [`MultiGzDecoder`] if your file has multiple streams.
 ///


Suggested change

///

///

/// This decoder only handles gzipped data with a single stream.

/// Use [`MultiGzDecoder`] for gzipped data with multiple streams.

///

Whatever the final text, it should stick to the RFC terminology of "member" rather than using "stream" in some places.

joshtriplett · 2023-07-21T01:21:05Z

I added some suggestions implementing my comment #324 (comment) .

I am not attached to exact wording here, feel free to wordsmith. Just going for the general spirit of documenting the situation without pushing people towards MultiGzDecoder.

workingjubilee · 2023-07-21T04:57:29Z

Is there a disadvantage to using MultiGzDecoder when the stream only has a single file?

workingjubilee · 2023-07-21T05:26:20Z

I agree with jsha: the evidence that Rust programs which do not adhere to this advisory will drop data on the floor is evidence enough that we should in fact recommend MultiGzDecoder is used. While not memory unsoundness, Rust programmers often are... irked when they find out an interface that they were using sends half or more of their data to /dev/null.

jongiddy · 2023-07-21T06:09:18Z

There is a difference in behavior that should be noted. MultiGzDecoder behaves differently for a gzip member embedded in other data. If there is non-gzip data following the gzip data GzipDecoder succeeds, MultiGzDecoder fails.

As an example, for the write versions GzipDecoder returns the number of bytes consumed, leaving any remaining bytes for the caller to handle. MultiGzDecoder returns an Err since it expects more gzip data.

Correct behavior depends on knowledge of the data. Is it only gzip data and may have multiple members, or is it a stream of data elements that contains gzipped data? HTTP clients have a real problem here as some servers send multi-member gzip responses and some other servers send additional data after the gzip-encoded content. There is probably a server that does both.

That is why it is legitimate to have two types of decoder, and it is the caller who needs to choose which is most appropriate for their data.

Byron · 2023-07-21T07:38:27Z

After having read this I feel that instead of nudging towards a particular implementation, it would be preferable to teach about the concept of "members" on the front-page while providing some of the examples we have seen here to explain what this can mean for an application. From there, users should be able to make the choice for their particular application.

For this PR this could mean one more section in the lib.rs documentation, along with the explicit lack of preferences in the wording for the documentation of Gz and MultiGz variants of types.

If this sounds feasible, give this comment a thumbs up. Otherwise, you could thumbs down (just to have an easy count) along with suggestions on what to do instead. Thanks everyone!

joshtriplett · 2023-07-21T07:41:33Z

(FWIW, I withdraw some of the github suggestions I left, in favor of what's summarized in this comment.)

@jongiddy wrote:

MultiGzDecoder behaves differently for a gzip member embedded in other data. If there is non-gzip data following the gzip data GzipDecoder succeeds, MultiGzDecoder fails.

That's one of the reasons, yeah. We should document that as a reason people should consider when choosing what to use.

Also, as @jsha noted, single-member decoding seems to be what many browsers and web clients do, and it'll be surprising if you get different results in different tools. Especially if the subsequent members are silently appended when they may be intended to be different "files".

It'd be worth coordinating with browser vendors to find out whether they have any practical considerations here (e.g. some hard-won knowledge about what happens in the wild). Are there some Firefox or Curl developers in the relevant area that we could poke?

@jsha wrote:

If we were designing from scratch, I would argue that the library shouldn't have a single-stream decoder that wraps Read / BufRead, because of the likelihood of silently dropping data. Instead, to support use cases that want to know about stream boundaries, there should be a secondary API that yields an Iterator<Item = GzipMember>,

Yeah, I'd agree with this. We could still add that API, which seems much more reasonable than MultiGzDecoder since it doesn't silently traverse the boundaries between files. And that API seems more reasonable to steer people towards, because it doesn't make the same tradeoffs of not being able to continue reading. In particular, if we're careful, you could iterate over the first member and then read the underlying stream for non-gzip data (without any issue at the boundary). (The iterator type could have a "give me the rest of the data as a Read/BufRead instance" method that consumes the iterator.) And conversely, we could have a method on the iterator that consumes the iterator and gives you a single stream but errors if there's more data afterwards, so that when people aren't expecting appended data they can check that assumption.

Summarizing:

Could we check with Firefox and/or Curl developers to find out if there's practical knowledge here that we're lacking? Or, are there open bugs, where knowledge like "this is a bug but here's why we can't fix it" or "this is a bug and we should actually fix it but we haven't yet" might live? If they have specific hard-won considerations about data in the wild, I'd be happy to defer to those. But the fact that multiple pieces of software out there don't decode multiple streams by default makes me hesitant to diverge.
In the meantime, it seems entirely reasonable for the documentation to document, at length, the considerations that should go into picking between the options we have. Perhaps we could explain this in detail at the top level documentation of the crate, and then link that from all the various decoders to avoid duplication? This includes explaining the single-stream/multi-stream issue, the tradeoffs about being able to have non-gzip data after one gzip stream, the fact that the single-stream decoder will ignore a subsequent gzip stream, the fact that many pieces of software don't seem to decode multiple streams...
We might be able to have a better API that better encourages people to explicitly handle this consideration. I don't think we should block merging better documentation until we have a better API, but I do think that having a better API would make the issue visible in the type system so that people need to handle it.

workingjubilee · 2023-07-22T06:03:06Z

Hearing there are common use-cases that combine a gz-encoded stream with something else does seem worth thinking about. It feels like people should be introduced to the idea that if they are handling a simple "gz file" that they probably should just handle it with MultiGzDecoder but that if they are handling a more complex encoding they might need to do something more complex and that is where GzDecoder comes into play. Ironically, it is the "simpler" version that is used for the "complex" cases, which violates many intuitions.

jongiddy · 2023-07-22T06:50:21Z

Exactly, and it is also the case that multi-member gzip files are uncommon (outside of Bioinformatics) so GzDecoder works for the common use case most of the time, which makes it unexpected when it doesn't.

I wonder whether it is worth introducing new types GzipDecoder and GzipMemberDecoder that alias MultiGzDecoder and GzDecoder and then deprecate the Gz types? On one hand it fixes the simple vs complex problem, on the other it might just add more confusion.

workingjubilee · 2023-07-22T08:28:00Z

Maybe in the future? It seems like it'd just add more confusion right now. Improving docs and adding the hypothetical iterator seems like the "way to go" in terms of cleaning up the API, probably.

jongiddy · 2023-07-22T09:37:47Z

The iterator idea is a cool abstraction of the problem but not really needed by users of this crate. Almost no-one cares about the individual members, they just want all the uncompressed data from the gzip data whether or not it is multi-member and whether or not it is embedded in other data. The one exception of which I am aware is the BGZF format, where an index is used to seek to the correct gzip member and then uncompress just that one member. Neither benefits from the iterator abstraction.

workingjubilee · 2023-07-22T10:26:15Z

I don't disagree that most users will basically go .iter().collect::<String>() or whatever, but it was also feature requested in #192 for "My data is HUGE! Please let me lazily iterate it!" use-cases.

the8472 · 2023-07-22T11:54:50Z

The iterator idea is a cool abstraction of the problem but not really needed by users of this crate. Almost no-one cares about the individual members, they just want all the uncompressed data from the gzip data whether or not it is multi-member and whether or not it is embedded in other data.

As a low-level crate it still makes sense to expose the basic building blocks for those who need it and then provide an abstraction for the most common use-cases.

workingjubilee · 2023-07-22T20:33:14Z

along with the explicit lack of preferences in the wording for the documentation of Gz and MultiGz variants of types.

If this sounds feasible, give this comment a thumbs up. Otherwise, you could thumbs down (just to have an easy count) along with suggestions on what to do instead. Thanks everyone!

Altogether, for this PR in particular, I'm in favor of this, modulo "we shouldn't be shy about saying that MultiGzDecoder is the simpler option for a 'basic' file". 👍

Could we check with Firefox and/or Curl developers to find out if there's practical knowledge here that we're lacking? Or, are there open bugs, where knowledge like "this is a bug but here's why we can't fix it" or "this is a bug and we should actually fix it but we haven't yet" might live? If they have specific hard-won considerations about data in the wild, I'd be happy to defer to those. But the fact that multiple pieces of software out there don't decode multiple streams by default makes me hesitant to diverge.

Let's make this a separate issue, but I suspect this might be a case of "EXTREMELY pedantic adherence to a specific RFC's spec in a way that is unnecessarily underachieving".

workingjubilee · 2023-07-22T20:36:08Z

Also see:

GzDecoder stops decoding file toward the start. #339

This issue is an extremely common papercut.

I also added a reference to the general section about the differences in the crate documentation. Co-Authored-By: Josh Triplett <[email protected]>

Byron · 2023-07-23T13:52:04Z

I have added a new top-level section to explain the intricacies of the gzip file format. Further I did my best to integrate all of Josh's suggestions, while also not applying a new paragraph if it appeared to say the same thing as a paragraph above it.

I particularly invite for a thorough review of the new top-level section.

jsha

This is looking great. Thanks for all the thoughtful conversation and feedback, all. One of the things it's clarified is that the main thing that winds up being risky / surprising is that the single-member variants drop data on the floor; so we should explicitly document that fact on those variants. I'll write up another commit with that change plus some minor tweaks that I just commented, and push that shortly.

src/gz/write.rs

src/lib.rs

- Use relative paths to link to the introduction. - Use consistent language across {Read,BufRead,Write}{Gz,MultiGz}Decoder. - Use `member` rather than `stream`. - Document what happens to unused data for `Gz` variants.

jsha · 2023-07-25T01:26:12Z

Okay, pushed a batch of changes. Sorry they wound up bigger than I intended; I was hoping to converge piecemeal, but wound up wanting to try and get all six places using the same language.

Could we check with Firefox and/or Curl developers to find out if there's practical knowledge here that we're lacking?

This is a good idea. I'm not likely to get it done soon, but I think it would be really informative and interesting.

To make sure I got it all right, copy-pasting from the generated docs for {Read,BufRead,Write}{Gz,MultiGz}Decoder:

bufread::GzDecoder

A decoder for the first member of a gzip file.

This structure exposes a BufRead interface, reading compressed data from the underlying reader, and emitting uncompressed data.

After reading the first member of a gzip file (which is often, but not always, the only member), this reader will return Ok(0) even if there are more bytes available in the underlying reader. If you want to be sure not to drop bytes on the floor, call into_inner() after Ok(0) to recover the underlying reader.

To handle gzip files that may have multiple members, see MultiGzDecoder or read more in the introduction.

read::GzDecoder

A decoder for the first member of a gzip file.

This structure exposes a Read interface that will consume compressed data from the underlying reader and emit uncompressed data.

After reading the first member of a gzip file (which is often, but not always, the only member), this reader will return Ok(0) even if there are more bytes available in the underlying reader. If you want to be sure not to drop bytes on the floor, call into_inner() after Ok(0) to recover the underlying reader.

To handle gzip files that may have multiple members, see MultiGzDecoder or read more in the introduction.

write::GzDecoder

A decoder for the first member of a gzip file.

This structure exposes a Write interface, receiving compressed data and writing uncompressed data to the underlying writer.

After decoding the first member of a gzip file, this writer will return XXX to all subsequent writes.

To handle gzip files that may have multiple members, see MultiGzDecoder or read more in the introduction.

bufread::MultiGzDecoder

A gzip streaming decoder that decodes a gzip file that may have multiple members.

This structure exposes a BufRead interface that will consume compressed data from the underlying reader and emit uncompressed data.

A gzip file consists of a series of members concatenated one after another. MultiGzDecoder decodes all members of a file and returns Ok(0) once the underlying reader does.

To handle members seperately, see GzDecoder or read more in the introduction.

read::MultiGzDecoder

A gzip streaming decoder that decodes a gzip file that may have multiple members.

This structure exposes a Read interface that will consume compressed data from the underlying reader and emit uncompressed data.

A gzip file consists of a series of members concatenated one after another. MultiGzDecoder decodes all members of a file and returns Ok(0) once the underlying reader does.

To handle members seperately, see GzDecoder or read more in the introduction.

write::MultiGzDecoder

A gzip streaming decoder that decodes a gzip file with multiple members.

This structure exposes a Write interface that will consume compressed data and write uncompressed data to the underlying writer.

A gzip file consists of a series of members concatenated one after another. MultiGzDecoder decodes all members of a file and writes them to the underlying writer one after another.

To handle members separately, see GzDecoder or read more in the introduction.

Byron

This looks great to me, thanks everyone for their help and contributions!

I'd be fine merging this PR and probably will do so if there was silence here for a week or so, or nobody does it before me 😁.

src/gz/bufread.rs

jongiddy · 2023-07-29T07:16:37Z

src/gz/read.rs

@@ -90,13 +90,22 @@ impl<R: Read + Write> Write for GzEncoder<R> {
    }
 }

-/// A gzip streaming decoder
+/// A decoder for the first member of a [gzip file].


Same changes as for bufread

src/gz/write.rs

src/lib.rs

jongiddy · 2023-07-29T07:34:35Z

src/lib.rs

+//! `gunzip`, and `zcat` command line tools.
+//!
+//! Chrome and Firefox appear to implement behavior like `GzDecoder`, ignoring data
+//! after the first member. `curl` appears to implement behavior somewhat like
+//! `GzDecoder`, only decoding the first member, but emitting an error if there is
+//! data after the first member, whether or not it is gzip data.


This is interesting for determining common practice, but the documentation should describe flate2 behavior, not that of other tools.

I agree and will remove the last paragraph.

jongiddy · 2023-07-29T07:57:22Z

We can wordsmith endlessly, so these are just suggestions that would clarify things for me. The only one that definitely needs to be changed is the instance that currently says writer will return XXX.

src/gz/read.rs

Co-authored-by: jongiddy <[email protected]>

… `flate2`

Byron · 2023-07-30T07:35:31Z

Thanks a lot, @jongiddy , for pushing this PR over the finishing line.

I have applied all of your suggestions and followed up on all comments as well as I could. Given the massive size of this PR, working with it doesn't get easier and I would like to make the cut here by merging what we have, and invite those interested to double-check what's there to possibly submit follow-up PRs for fixes.

I for one learned a lot here and believe this PR represents a substantial improvement to the docs that will also help users to avoid these member-releated pitfalls (along with pitfalls related to GzDecoder reading a little bit ahead of the underlying stream so one wants to use the BufRead implementation to see these nonetheless).

jongiddy mentioned this pull request Nov 25, 2022

Add write::MultiGzDecoder for multi-member gzip data #325

Merged

JohnTitor reviewed Apr 23, 2023

View reviewed changes

src/gz/bufread.rs Outdated Show resolved Hide resolved

Recommend MultiGzDecoder over GzDecoder in docs

7d5856d

jsha force-pushed the prefer-multigz branch from 70c30e6 to 7d5856d Compare July 13, 2023 20:32

Byron reviewed Jul 14, 2023

View reviewed changes

src/gz/bufread.rs Outdated Show resolved Hide resolved

Byron requested a review from JohnTitor July 14, 2023 15:18

jongiddy reviewed Jul 14, 2023

View reviewed changes

src/gz/bufread.rs Outdated Show resolved Hide resolved

Byron mentioned this pull request Jul 17, 2023

Export all headers from MultiGzDecoder #348

Closed

Byron self-assigned this Jul 17, 2023

minor improvements to the MultiGzDecoder documentation

7cfdd4e

applies copies of minor improvements

a232574

Byron self-requested a review July 20, 2023 06:36

Byron approved these changes Jul 20, 2023

View reviewed changes

joshtriplett reviewed Jul 21, 2023

View reviewed changes

Byron force-pushed the prefer-multigz branch from cfa8214 to a232574 Compare July 21, 2023 07:27

Byron and others added 2 commits July 23, 2023 15:21

Add top-level comparison between GzDecoder and MultiGzDecoder

e21986e

Apply suggestions to impartial to Gz and MultiGz implementations.

1e09571

I also added a reference to the general section about the differences in the crate documentation. Co-Authored-By: Josh Triplett <[email protected]>

Byron force-pushed the prefer-multigz branch from fa1a8e2 to 1e09571 Compare July 23, 2023 13:50

jsha commented Jul 25, 2023

View reviewed changes

src/gz/write.rs Outdated Show resolved Hide resolved

src/lib.rs Outdated Show resolved Hide resolved

Tweak the {Gz,MultiGz}Decoder docs more

955728b

- Use relative paths to link to the introduction. - Use consistent language across {Read,BufRead,Write}{Gz,MultiGz}Decoder. - Use `member` rather than `stream`. - Document what happens to unused data for `Gz` variants.

Byron approved these changes Jul 25, 2023

View reviewed changes

jongiddy reviewed Jul 29, 2023

View reviewed changes

src/gz/read.rs Outdated Show resolved Hide resolved

Byron and others added 2 commits July 30, 2023 09:29

Apply suggestions from code review

f0bf8a6

Co-authored-by: jongiddy <[email protected]>

remove introductory paragraph that described other tools unrelated to…

fc30d9e

… `flate2`

Byron force-pushed the prefer-multigz branch from 1287e49 to fc30d9e Compare July 30, 2023 07:31

Byron merged commit 956397a into rust-lang:main Jul 30, 2023
10 checks passed

-/// the first member. MultiGzDecoder is preferable in most cases.
+/// the first member. Many gzip files in the wild will typically have one member,
+/// but not all. Use MultiGzDecoder if you need to handle multiple members, or
+/// [GzDecoder] if you know you only need to handle a single member.

	/// A gzip streaming decoder that decodes a full [gzip file].
	/// A gzip streaming decoder that decodes a [gzip file] with multiple members.

		/// A decoder for a single member of a gzip file. Prefer [MultiGzDecoder] for
		/// most uses.

	/// A decoder for a single member of a gzip file. Prefer [MultiGzDecoder] for
	/// most uses.
	/// A decoder for a gzip file with a single member.

-///
+///
+/// This decoder only handles gzipped data with a single stream.
+/// Use [`MultiGzDecoder`] for gzipped data with multiple streams.
+///

Recommend MultiGzDecoder over GzDecoder in docs #324

Recommend MultiGzDecoder over GzDecoder in docs #324

Conversation

jsha commented Oct 31, 2022

jsha commented Jul 13, 2023

Byron left a comment

Choose a reason for hiding this comment

Byron commented Jul 17, 2023

joshtriplett commented Jul 21, 2023

Choose a reason for hiding this comment

Byron Jul 21, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joshtriplett Jul 21, 2023 • edited Loading

Choose a reason for hiding this comment

joshtriplett Jul 21, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joshtriplett commented Jul 21, 2023 • edited Loading

workingjubilee commented Jul 21, 2023

workingjubilee commented Jul 21, 2023

jongiddy commented Jul 21, 2023

Byron commented Jul 21, 2023

joshtriplett commented Jul 21, 2023 • edited Loading

workingjubilee commented Jul 22, 2023

jongiddy commented Jul 22, 2023

workingjubilee commented Jul 22, 2023

jongiddy commented Jul 22, 2023

workingjubilee commented Jul 22, 2023

the8472 commented Jul 22, 2023

workingjubilee commented Jul 22, 2023

workingjubilee commented Jul 22, 2023

Byron commented Jul 23, 2023

jsha left a comment

Choose a reason for hiding this comment

jsha commented Jul 25, 2023

Byron left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jongiddy commented Jul 29, 2023

Byron commented Jul 30, 2023

Byron Jul 21, 2023 •

edited

Loading

joshtriplett Jul 21, 2023 •

edited

Loading

joshtriplett Jul 21, 2023 •

edited

Loading

joshtriplett commented Jul 21, 2023 •

edited

Loading

joshtriplett commented Jul 21, 2023 •

edited

Loading