Repository navigation
Conversation
The same URL can reach downloadTarball_() in different encodings, e.g. `+` vs. `%2B`. Cache keys were inconsistent about this: * The lookup key was the URL string as passed in. * The insert keys were the URLs in the redirect chain returned by the file transfer. The first one is `request.uri.to_string()`, which is re-encoded by ParsedURL (`+` becomes `%2B`), while the subsequent ones are curl's effective URLs, which are the `Location` headers verbatim. This caused cache misses for FlakeHub flakes. E.g. https://flakehub.com/f/NixOS/nixpkgs/0.2605 redirects to https://api.flakehub.com/f/pinned/NixOS/nixpkgs/0.2605.1017737+rev-.../source.tar.gz, which is cached with the `+`. However, the locked URL that ends up in flake.lock goes through ParsedURL and so contains `%2B`. As a result, the next command missed the cache and downloaded the tarball again. Conversely, a tarball URL with a `+` passed to `fetchTree` was looked up verbatim but inserted with `%2B`, so it never hit the cache. The fix is to canonicalise all URLs used as cache keys via ParsedURL::to_string(). Since the first URL in the redirect chain was already canonical, existing cache entries for it remain valid. Assisted-by: Claude Opus 5.5 <noreply@anthropic.com>
lisanna-dettwyler
approved these changes
Nov 9, 2026
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
After
nix flake updateon a FlakeHub input likehttps://flakehub.com/f/NixOS/nixpkgs/0.2605, the next command missed the tarball cache and downloaded nixpkgs again. The cache entry was keyed on the redirect target as sent by the server (.../0.2605.1017737+rev-.../source.tar.gz), while the locked URL inflake.lockhas%2Brev-. Similarly,fetchTreeon a tarball URL containing a+never hit the cache.Taken from DeterminateSystems#760.
Implementation
Use the canonical encoding of the URL (
ParsedURL::to_string()) both for the cache lookup and for every URL in the redirect chain on insert. Existing cache entries for the original request URL stay valid, since those were already canonical.Tests: a functional test for a
+in afile://tarball URL, and thetarball-flakesNixOS test now redirects to a+URL and checks that the locked URL is fetched from the cache with the web server stopped.Add 👍 to pull requests you find important.
The Nix maintainer team uses a GitHub project board to schedule and track reviews.