Skip to content

Conversation

@pkpbynum
Copy link
Contributor

@pkpbynum pkpbynum commented Dec 12, 2025

Motivation

I'd like to allow more flexibility in selecting the HTTP version used for a specific store.

This PR refactors the logic for selecting the HTTP versions implemented by libcurl, which is currently controlled by the global http2 setting. Instead we add an http-version setting to the HTTP binary cache store. In addition to allowing experimentation with new protocols, this allows different stores to implement different HTTP versions.

Backwards compatibility
When the store setting is http-version == none (the default), the global http2 setting is checked:

  • if http2 == true (default) -> use CURL_HTTP_VERSION_NONE -- this differs from previous behavior using CURL_HTTP_VERSION_2TLS, and lets curl decide the best version to use based on the scheme (http/https) and handshake.
  • if http2 == false -> use CURL_HTTP_VERSION_1_1 (matching previous behavior)

TLDR; users that have explicitly set http2 == false will experience no change. The default behavior, however, is now using CURL_HTTP_VERSION_NONE rather than CURL_HTTP_VERSION_TLS.

One might also argue for the deprecation of the http2 setting at this point, and just let folks override directly with http-version=http1.1 if they want it.

Context

Some brief discussion around QUIC here.


Add 👍 to pull requests you find important.

The Nix maintainer team uses a GitHub project board to schedule and track reviews.

@github-actions github-actions bot added the store Issues and pull requests concerning the Nix store label Dec 12, 2025
@pkpbynum pkpbynum marked this pull request as draft December 12, 2025 20:45
Copy link
Contributor

@xokdvium xokdvium left a comment

Choose a reason for hiding this comment

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

Making this per-store seems nice, but exposing all the internal bits of curl seems like a step in the wrong direction. Wouldn't H11/H2/H3 be enough?

@pkpbynum
Copy link
Contributor Author

Making this per-store seems nice, but exposing all the internal bits of curl seems like a step in the wrong direction. Wouldn't H11/H2/H3 be enough?

Yup sure, I just added them here for completeness but happy to trim it down. Although I am specifically interested in http2-prior-knowledge. We could also consider CURL_HTTP_VERSION_NONE to let curl handle the upgrades dynamically.

Also, reverted to draft while I figure out why it's not building on Linux.

@pkpbynum pkpbynum force-pushed the pb/http-store-version branch from d8901ba to ec27ea4 Compare December 16, 2025 17:40
@pkpbynum pkpbynum force-pushed the pb/http-store-version branch from ec27ea4 to af7a2d8 Compare December 16, 2025 17:46
@pkpbynum
Copy link
Contributor Author

pkpbynum commented Dec 16, 2025

I updated this PR to compile properly on Linux. I had to copy-paste some BaseSettings templates--including the generic implementation in configuration.hh and abstract-settings-to-json.hh caused issues with ExternalBuilders. I'm not sure how to make this better.

@xokdvium I also trimmed down the allowed versions, and change the names to match exactly the CLI flags afforded by curl, e.g. --http1.1, --http2, --http2-prior-knowledge, --http3 ==> becomes http1.1, http2, http2-prior-knowledge, http3 as http-version settings

@pkpbynum pkpbynum marked this pull request as ready for review December 16, 2025 18:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

store Issues and pull requests concerning the Nix store

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants