Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the brew-rs HTTP client configuration to negotiate compressed HTTP responses using reqwest, aiming to reduce transfer sizes (particularly for API/JSON payloads).
Changes:
- Enable
reqwest’sgzipanddeflatefeatures inbrew-rs’sCargo.toml. - Turn on gzip/deflate support in the shared
reqwest::blocking::Clientbuilt bybuild_client().
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| Library/Homebrew/rust/brew-rs/src/commands/fetch.rs | Enables gzip/deflate on the shared reqwest client used by fetch flows. |
| Library/Homebrew/rust/brew-rs/Cargo.toml | Adds reqwest compression features (gzip, deflate) to the dependency configuration. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| .gzip(true) | ||
| .deflate(true) |
There was a problem hiding this comment.
Enabling reqwest gzip/deflate at the shared ClientBuilder level affects bottle downloads too. If any bottle host returns a Content-Encoding (e.g., gzip) response, reqwest will transparently decompress and the on-disk bytes will no longer match the expected bottle SHA-256, causing fetch failures or writing unexpected content. Consider limiting compression to API/JSON requests only (e.g., use a separate client for metadata, or force Accept-Encoding: identity on bottle requests).
| .gzip(true) | |
| .deflate(true) |
There was a problem hiding this comment.
I thought about this but the server for bottles ignores an Accept-Encoding header in requests and doesn't use a Content-Encoding header in the response.
This adds `gzip` and `deflate` features from the `reqwest` crate and enables compression in the `build_client` function. There are also `brotli` and `zstd` features but I've used the compression methods that we're already using in `curl`. `gzip` is the best we can get from the servers that we're currently using, so there isn't any benefit to the other compression methods for now. For what it's worth, `reqwest` takes care of automatically adding the `Accept-Encoding` header to requests and decompressing responses that have a `Content-Encoding` header. In the process, it removes `Content-Encoding` and `Content-Length` headers from the response, so it's a little tricky to tell if it's working but I manually verified the behavior.
524ff7d to
d942d26
Compare
brew lgtm(style, typechecking and tests) with your changes locally?This adds
gzipanddeflatefeatures from thereqwestcrate and enables compression in thebuild_clientfunction. There are alsobrotliandzstdfeatures but I've used the compression methods that we're already using in curl. gzip is the best we can get from the servers that we're currently using, so there isn't any benefit to the other compression methods for now.For what it's worth, reqwest takes care of automatically adding the
Accept-Encodingheader to requests and decompressing responses that have aContent-Encodingheader. In the process, it removesContent-EncodingandContent-Lengthheaders from the response, so it's a little tricky to tell if it's working but I manually verified the behavior.