Skip to content

brew-rs: enable compression in reqwest#21835

Open
samford wants to merge 1 commit intomainfrom
brew-rs-enable-compression
Open

brew-rs: enable compression in reqwest#21835
samford wants to merge 1 commit intomainfrom
brew-rs-enable-compression

Conversation

@samford
Copy link
Member

@samford samford commented Mar 25, 2026


  • Have you followed the guidelines in our Contributing document?
  • Have you checked to ensure there aren't other open Pull Requests for the same change?
  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests (excluding integration tests) for your changes? Here's an example.
  • Have you successfully run brew lgtm (style, typechecking and tests) with your changes locally?

  • AI was used to generate or assist with generating this PR. Please specify below how you used AI to help you, and what steps you have taken to manually verify the changes.

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.

Copilot AI review requested due to automatic review settings March 25, 2026 21:18
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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’s gzip and deflate features in brew-rs’s Cargo.toml.
  • Turn on gzip/deflate support in the shared reqwest::blocking::Client built by build_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.

Comment on lines +634 to +635
.gzip(true)
.deflate(true)
Copy link

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

Suggested change
.gzip(true)
.deflate(true)

Copilot uses AI. Check for mistakes.
Copy link
Member Author

@samford samford Mar 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.
@samford samford force-pushed the brew-rs-enable-compression branch from 524ff7d to d942d26 Compare March 25, 2026 21:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants