Skip to content

Conversation

@xokdvium
Copy link
Contributor

@xokdvium xokdvium commented Jan 1, 2026

…e:// URI

Motivation

Special characters like + e.t.c. are pct-encoded.
cc @dramforever. There's also a very stupid issue that leads to test failures because of regexes of all things:

auto regex = std::regex(R"((^|\n)overlay )" + config->realStoreDir.get() + R"( .*(\n|$))");
, but that's on Linux. With this change the tests pass on darwin with --build-dir "/nix/var/nix/builds/build-1". Not sure what your issue was when testing it out.

Context


Add 👍 to pull requests you find important.

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

…e:// URI

Special characters like + e.t.c. are pct-encoded.
@xokdvium xokdvium requested a review from edolstra as a code owner January 1, 2026 22:28
@github-actions github-actions bot added the fetching Networking with the outside (non-Nix) world, input locking label Jan 1, 2026
@dramforever
Copy link
Contributor

dramforever commented Jan 2, 2026

cc @vcunat how did you repro it?

in any case #14751 doesn't seem like it was related to fetchers?

@xokdvium
Copy link
Contributor Author

xokdvium commented Jan 2, 2026

The local-overlay-store failures from #14751 is due to the regex issue I mentioned. But those tests are not run on Darwin.

@vcunat
Copy link
Member

vcunat commented Jan 2, 2026

How is this related to me?

@dramforever
Copy link
Contributor

@vcunat From this comment NixOS/nixpkgs#468208 (comment) and NixOS/nixpkgs#469207

I never figured out how to reproduce the appendPatches problem unfortunately, but given this patch it seems to be related to when the test suite is run under a path with + in it?

So it should also fail on Linux. I can give that a try myself but it wouldn't be clear whether that fixes the problems on Darwin

@xokdvium
Copy link
Contributor Author

xokdvium commented Jan 2, 2026

Yes, I did reproduce that on my linux machine with a different sandbox-build-dir. There's another unrelated issue that leads to the test suite failing, but this patch fixes the tarball one.

I can try poking around more and see if there are more issues with Darwin, but this should be good to merge regardless, since it fixes a bug.

@Ericson2314 Ericson2314 merged commit 952c823 into 2.31-maintenance Jan 4, 2026
27 checks passed
@Ericson2314 Ericson2314 deleted the backport-tarball-pct-decode-to-2.31-maintenance branch January 4, 2026 22:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fetching Networking with the outside (non-Nix) world, input locking

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants