From 9d4cf12c4f392dd7fda0b8a97d8e3b3ea0b2b83c Mon Sep 17 00:00:00 2001 From: Madison Steiner Date: Tue, 1 Sep 2026 13:59:04 +0000 Subject: [PATCH 1/7] ci: run nix-store-tests on Windows `unitTests` built only `nix-util-tests`, which links neither libmain nor libstore, so nothing in libstore was exercised on Windows at all. Output checks, content-addressed outputs and fixed-output derivations are handled in platform-neutral code that the Windows builder inherits, but that inheritance was never verified. Adding the suite reports 784 tests from 87 suites under Wine, of which 729 pass and 24 fail. Every failure is a test hardcoding `/nix/store`, which is not an absolute path on Windows because `std::filesystem::path::is_absolute()` requires a root *name* as well as a root directory. Those are fixed separately; this commit is the wiring that makes them visible. Assisted-by: Claude Code (claude-opus-5) --- ci/gha/tests/windows.nix | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ci/gha/tests/windows.nix b/ci/gha/tests/windows.nix index 61155db6b8e..0ae1350625b 100644 --- a/ci/gha/tests/windows.nix +++ b/ci/gha/tests/windows.nix @@ -26,8 +26,8 @@ in { unitTests = { "nix-util-tests" = fixOutput packages."nix-util-tests-x86_64-w64-mingw32".passthru.tests.run; + "nix-store-tests" = fixOutput packages."nix-store-tests-x86_64-w64-mingw32".passthru.tests.run; }; - # `unitTests` builds one suite, which links neither libmain nor libstore. crossBuild = packages."nix-everything-x86_64-w64-mingw32"; } From babc81b62b6a54e32a94cccff563a7041753a8a9 Mon Sep 17 00:00:00 2001 From: Madison Steiner Date: Tue, 1 Sep 2026 14:03:41 +0000 Subject: [PATCH 2/7] ci: clear NIX_STORE for the Windows test run The outer build exports `NIX_STORE` pointing at the host's POSIX store directory, and Wine passes it straight through to the Windows test binary. `StoreDirSetting` consults `NIX_STORE_DIR` then `NIX_STORE` before its own default, so a store declared `FilePathType::Native` ends up validating that POSIX value with `std::filesystem::path::is_absolute()` -- false on Windows, which requires a root name as well as a root directory. Clearing both lets each store type fall back to its own default: the Unix default stays `/nix/store`, and the native one becomes `getProgramData()/nix/store` as intended. Measured on nix-store-tests under Wine: 24 failures before, 8 after, with passes going from 729 to 745 of 784. Assisted-by: Claude Code (claude-opus-5) --- ci/gha/tests/windows.nix | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/ci/gha/tests/windows.nix b/ci/gha/tests/windows.nix index 0ae1350625b..bbb5e6f554f 100644 --- a/ci/gha/tests/windows.nix +++ b/ci/gha/tests/windows.nix @@ -16,6 +16,13 @@ let # hide/show sequences, making logs unreadable in GitHub Actions. buildCommand = '' set -o pipefail + # The outer build sets `NIX_STORE` to the host's POSIX store directory, + # and Wine passes it through to the Windows test binary. A store + # configured as `FilePathType::Native` then validates that value with + # `std::filesystem::path::is_absolute()`, which is false for a + # POSIX-rooted path on Windows because it has no root name. Clearing + # both lets each store type fall back to its own correct default. + unset NIX_STORE NIX_STORE_DIR { ${prev.buildCommand} } 2>&1 | ansi2txt From 8f1962b25413a0744c066c35981b410838f76b67 Mon Sep 17 00:00:00 2001 From: Madison Steiner Date: Tue, 1 Sep 2026 15:19:30 +0000 Subject: [PATCH 3/7] libstore: fix three Windows defects the new suite exposed Wiring nix-store-tests into the Windows CI job surfaced 24 failures. Sixteen were the `NIX_STORE` leak fixed in the previous commit; these are the remaining eight, which are three separate pre-existing defects rather than test bugs. **`LocalOverlayStoreConfig` was unconstructible on Windows.** `upperLayer` was a `Setting` whose default was the sentinel `/upper-layer-must-be-set`. `AbsolutePath` validates on construction and a POSIX-rooted sentinel is not absolute on Windows, so the config threw before the caller's parameters were even considered. It is now `Setting>` defaulting to `std::nullopt`, which is what "must be set" actually means; the serialiser for that type already existed, and the constructor already rejected the unset case, so the sentinel was never meaningfully read. **`parseStorePath` did not normalise on Windows.** It wrapped the input in `std::filesystem::path` and compared `parent_path()`, so `/./x`, `/y/../x` and a trailing separator were rejected instead of being normalised and accepted. It now canonicalises in the syntax the store directory itself uses: a Unix-style store dir stays Unix-style on Windows, which is what `FilePathType::Unix` is for and what allows a Windows client to address a Unix store. Normalising natively would rewrite the separators to `\\` and break the comparison instead. The pre-existing comment questioning these semantics is replaced with one stating them. **A C-API test asserted the Unix default store dir.** The default store is local, hence native, so on Windows it is `getProgramData()/nix/store` rather than `NIX_STORE_DIR`. Measured under Wine: 784 tests, 753 passing, zero failures (was 729 passing with 24 failures). Assisted-by: Claude Code (claude-opus-5) --- src/libstore-tests/local-overlay-store.cc | 9 +++++++-- src/libstore-tests/nix_api_store.cc | 14 +++++++++++++- .../include/nix/store/local-overlay-store.hh | 10 ++++++++-- src/libstore/local-overlay-store.cc | 8 ++++---- src/libstore/store-dir-config.cc | 17 +++++++++++------ 5 files changed, 43 insertions(+), 15 deletions(-) diff --git a/src/libstore-tests/local-overlay-store.cc b/src/libstore-tests/local-overlay-store.cc index eb88d879ce2..225ca1a824a 100644 --- a/src/libstore-tests/local-overlay-store.cc +++ b/src/libstore-tests/local-overlay-store.cc @@ -44,14 +44,19 @@ TEST(LocalOverlayStore, upperLayer_notOverridden) TEST(LocalOverlayStore, upperLayer_overridden) { +#ifdef _WIN32 + constexpr std::string_view upper = "C:\\some\\upper"; +#else + constexpr std::string_view upper = "/some/upper"; +#endif LocalOverlayStoreConfig config{ "", { - {"upper-layer", "/some/upper"}, + {"upper-layer", std::string{upper}}, }, }; EXPECT_TRUE(config.upperLayer.isOverridden()); - EXPECT_EQ(config.upperLayer.get(), std::filesystem::path{"/some/upper"}); + EXPECT_EQ(config.upperLayer.get(), std::optional{std::string{upper}}); } } // namespace nix diff --git a/src/libstore-tests/nix_api_store.cc b/src/libstore-tests/nix_api_store.cc index 0162684daf4..9abd7e32249 100644 --- a/src/libstore-tests/nix_api_store.cc +++ b/src/libstore-tests/nix_api_store.cc @@ -11,6 +11,9 @@ #include "nix/util/tests/string_callback.hh" #include "nix/util/tests/test-data.hh" #include "nix/util/url.hh" +#ifdef _WIN32 +# include "nix/util/windows-known-folders.hh" +#endif #include "store-tests-config.hh" @@ -50,8 +53,17 @@ TEST_F(nix_api_util_context, nix_store_get_storedir_default) assert_ctx_ok(); ASSERT_EQ(NIX_OK, ret); - // These tests run with a unique storeDir, but not a relocated store + // These tests run with a unique storeDir, but not a relocated store. + // + // The default store is a local store, whose store dir is a *native* path. + // `NIX_STORE_DIR` is the Unix spelling, and on Windows it is not even a + // valid absolute path, so the native default is used there instead. See + // `StoreConfigBase::StoreDirSetting` for where that default comes from. +#ifdef _WIN32 + ASSERT_EQ((nix::windows::known_folders::getProgramData() / "nix" / "store").string(), str); +#else ASSERT_STREQ(NIX_STORE_DIR, str.c_str()); +#endif nix_store_free(store); } diff --git a/src/libstore/include/nix/store/local-overlay-store.hh b/src/libstore/include/nix/store/local-overlay-store.hh index 60cde838036..cd2de414851 100644 --- a/src/libstore/include/nix/store/local-overlay-store.hh +++ b/src/libstore/include/nix/store/local-overlay-store.hh @@ -35,9 +35,15 @@ public: Must be used as OverlayFS lower layer for this store's store dir. )"}; - const Setting upperLayer{ + /* Has no default: the constructor rejects a config that leaves it unset. + Spelling that as `std::nullopt` rather than a sentinel path matters + because `AbsolutePath` validates on construction, and a POSIX-rooted + sentinel is not absolute on Windows -- `is_absolute()` wants a root name + as well as a root directory -- so the sentinel made the config + unconstructible there regardless of what the caller passed. */ + const Setting> upperLayer{ (StoreConfig *) this, - "/upper-layer-must-be-set", + std::nullopt, "upper-layer", R"( Directory containing the OverlayFS upper layer for this store's store dir. diff --git a/src/libstore/local-overlay-store.cc b/src/libstore/local-overlay-store.cc index d3b0ccc77a7..d4ecf14f359 100644 --- a/src/libstore/local-overlay-store.cc +++ b/src/libstore/local-overlay-store.cc @@ -39,7 +39,7 @@ StoreReference LocalOverlayStoreConfig::getReference() const std::filesystem::path LocalOverlayStoreConfig::toUpperPath(const StorePath & path) const { - return upperLayer.get() / path.to_string(); + return *upperLayer.get() / path.to_string(); } LocalOverlayStore::LocalOverlayStore(ref config) @@ -49,7 +49,7 @@ LocalOverlayStore::LocalOverlayStore(ref config) , config{config} , lowerStore(openStore(config->lowerStoreUri.get()).dynamic_pointer_cast()) { - if (!config->upperLayer.isOverridden()) + if (!config->upperLayer.get()) throw Error("overlay store at %s requires the 'upper-layer' setting", PathFmt(config->realStoreDir.get())); if (config->checkMount.get()) { @@ -70,9 +70,9 @@ LocalOverlayStore::LocalOverlayStore(ref config) }; auto expectedLowerDir = lowerStore->config.realStoreDir.get(); - if (!checkOption("lowerdir", expectedLowerDir) || !checkOption("upperdir", config->upperLayer.get())) { + if (!checkOption("lowerdir", expectedLowerDir) || !checkOption("upperdir", *config->upperLayer.get())) { debug("expected lowerdir: %s", PathFmt(lowerStore->config.realStoreDir.get())); - debug("expected upperdir: %s", PathFmt(config->upperLayer.get())); + debug("expected upperdir: %s", PathFmt(*config->upperLayer.get())); debug("actual mount: %s", mountInfo); throw Error("overlay filesystem %s mounted incorrectly", PathFmt(config->realStoreDir.get())); } diff --git a/src/libstore/store-dir-config.cc b/src/libstore/store-dir-config.cc index cc099c15c6c..a053b024103 100644 --- a/src/libstore/store-dir-config.cc +++ b/src/libstore/store-dir-config.cc @@ -10,14 +10,19 @@ StorePath StoreDirConfig::parseStorePath(std::string_view path) const { if (path.empty()) throw BadStorePath("empty path is not a valid store path"); - // On Windows, `/nix/store` is not a canonical path. More broadly it - // is unclear whether this function should be using the native - // notion of a canonical path at all. For example, it makes to - // support remote stores whose store dir is a non-native path (e.g. - // Windows <-> Unix ssh-ing). + /* Canonicalise in whatever syntax the store directory itself uses, not + necessarily the native one. A store dir is a *logical* path: a + Unix-style one stays Unix-style even on Windows -- that is what + `FilePathType::Unix` means, and it is what lets a Windows client talk to + a Unix store over ssh -- so normalising it with native semantics would + rewrite the separators to `\` and the comparison below would stop + matching. Previously Windows did not normalise at all, so + `/./x`, `/y/../x` and a trailing separator were all + rejected rather than accepted-and-normalised. */ auto p = #ifdef _WIN32 - std::filesystem::path(path) + storeDir.starts_with('/') ? std::filesystem::path(CanonPath(std::string(path)).abs()) + : std::filesystem::path(path).lexically_normal() #else canonPath(std::string(path)) #endif From b76ea0f9d54471a46c6707bf547b3dec5dee39dc Mon Sep 17 00:00:00 2001 From: Madison Steiner Date: Tue, 1 Sep 2026 15:29:42 +0000 Subject: [PATCH 4/7] ci: build the new Windows suite, and let the job fail the run Two gaps meant the suite added earlier would not actually have protected anything. The job's build step named `unitTests.nix-util-tests` explicitly, so adding an attribute to `ci/gha/tests/windows.nix` was not enough to make CI run it. More importantly the job carried `continue-on-error: true`, so a Windows failure has never blocked a merge. A suite that is allowed to be red provides no protection, and libstore was not covered on Windows at all until now, so the two together meant Windows could regress silently. Dropping it makes `windows unit tests` a real gate. Safe to do now that the suite is green: 784 tests, 753 passing, zero failures under Wine. Assisted-by: Claude Code (claude-opus-5) --- .github/workflows/ci.yml | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 0844f7c8e92..488b57bdd20 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -146,7 +146,6 @@ jobs: needs: basic-checks name: windows unit tests runs-on: ubuntu-24.04 - continue-on-error: true timeout-minutes: 60 steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 @@ -159,7 +158,7 @@ jobs: dogfood: ${{ github.event_name == 'workflow_dispatch' && inputs.dogfood || github.event_name != 'workflow_dispatch' }} - name: Build and test Windows components run: | - nix build --file ci/gha/tests/windows.nix crossBuild unitTests.nix-util-tests -L + nix build --file ci/gha/tests/windows.nix crossBuild unitTests.nix-util-tests unitTests.nix-store-tests -L installer_test: needs: [tests] From d4699e23e1652343a28061913512165c43be48e2 Mon Sep 17 00:00:00 2001 From: Madison Steiner Date: Tue, 1 Sep 2026 16:03:16 +0000 Subject: [PATCH 5/7] libstore: share the scratch-output setup, unblocking CA and fixed-output on Windows `UnixDerivationBuilderImpl::startBuild` decided the scratch path per output and populated `inputRewrites` from it. Nothing in that loop is POSIX --- it uses `needsHashRewrite()`, `makeFallbackPath()`, `scratchOutputs`, `hashPlaceholder()` and `redirectedOutputs`, all of which are either already shared or move here with it --- so it now lives on `DerivationBuilderImpl` as `prepareScratchOutputs()`, along with the two virtuals it needs. `needsHashRewrite()` keeps its `true` default and the chroot builder keeps its override; Windows has no chroot, so the default is right there. The Windows builder had its own cut-down version of that loop which assumed every output was input-addressed: if (!ia) throw UnimplementedError( "only input-addressed derivation outputs are supported on Windows"); That is what actually stopped content-addressed and fixed-output derivations from building on Windows. They were refused before the build started, not by anything downstream, so the platform-neutral output checking and content-addressing in `registerOutputs` was unreachable for them. Calling the shared version instead removes the restriction, because it works off `initialOutputs` and copes with outputs whose final path is not known up front. It also means `inputRewrites` is populated on Windows for the first time, which the next commit makes use of. Verified: native and `x86_64-w64-mingw32` both compile clean; nix-store-tests 792/787 natively, unchanged; under Wine nix-store-tests 784/753 and nix-util-tests 799/791, both with zero failures. Assisted-by: Claude Code (claude-opus-5) --- src/libstore/build/derivation-builder-impl.cc | 84 +++++++++++++++++++ src/libstore/build/derivation-builder-impl.hh | 38 +++++++++ .../build/unix-derivation-builder-impl.hh | 24 ------ .../unix/build/unix-derivation-builder.cc | 81 +----------------- .../build/windows-derivation-builder.cc | 17 ++-- 5 files changed, 131 insertions(+), 113 deletions(-) diff --git a/src/libstore/build/derivation-builder-impl.cc b/src/libstore/build/derivation-builder-impl.cc index ae262e6898e..6d94e696d60 100644 --- a/src/libstore/build/derivation-builder-impl.cc +++ b/src/libstore/build/derivation-builder-impl.cc @@ -764,4 +764,88 @@ SingleDrvOutputs DerivationBuilderImpl::checkSubmittedOutputs() return builtOutputs; } +StorePath DerivationBuilderImpl::makeFallbackPath(OutputNameView outputName) +{ + // This is a bogus path type, constructed this way to ensure that it doesn't collide with any other store path + // See doc/manual/source/protocols/store-path.md for details + // TODO: We may want to separate the responsibilities of constructing the path fingerprint and of actually doing the + // hashing + auto pathType = "rewrite:" + std::string(drvPath.to_string()) + ":name:" + std::string(outputName); + return store.makeStorePath( + pathType, + // pass an all-zeroes hash + Hash(HashAlgorithm::SHA256), + outputPathName(drv.name, outputName)); +} + +StorePath DerivationBuilderImpl::makeFallbackPath(const StorePath & path) +{ + // This is a bogus path type, constructed this way to ensure that it doesn't collide with any other store path + // See doc/manual/source/protocols/store-path.md for details + auto pathType = "rewrite:" + std::string(drvPath.to_string()) + ":" + std::string(path.to_string()); + return store.makeStorePath( + pathType, + // pass an all-zeroes hash + Hash(HashAlgorithm::SHA256), + path.name()); +} + +void DerivationBuilderImpl::prepareScratchOutputs() +{ + for (auto & [outputName, status] : initialOutputs) { + /* Set scratch path we'll actually use during the build. + + If we're not doing a chroot build, but we have some valid + output paths. Since we can't just overwrite or delete + them, we have to do hash rewriting: i.e. in the + environment/arguments passed to the build, we replace the + hashes of the valid outputs with unique dummy strings; + after the build, we discard the redirected outputs + corresponding to the valid outputs, and rewrite the + contents of the new outputs to replace the dummy strings + with the actual hashes. */ + auto scratchPath = !status.known ? makeFallbackPath(outputName) + : !needsHashRewrite() + /* Can always use original path in sandbox */ + ? status.known->path + : !status.known->isPresent() + /* If path doesn't yet exist can just use it */ + ? status.known->path + : buildMode != bmRepair && !status.known->isValid() + /* If we aren't repairing we'll delete a corrupted path, so we + can use original path */ + ? status.known->path + : /* If we are repairing or the path is totally valid, we'll need + to use a temporary path */ + makeFallbackPath(status.known->path); + scratchOutputs.insert_or_assign(outputName, scratchPath); + + /* Substitute output placeholders with the scratch output paths. + We'll use during the build. */ + inputRewrites[hashPlaceholder(outputName)] = store.printStorePath(scratchPath); + + /* Additional tasks if we know the final path a priori. */ + if (!status.known) + continue; + auto fixedFinalPath = status.known->path; + + /* Additional tasks if the final and scratch are both known and + differ. */ + if (fixedFinalPath == scratchPath) + continue; + + /* Ensure scratch path is ours to use. */ + deletePath(store.printStorePath(scratchPath)); + + /* Rewrite and unrewrite paths */ + { + std::string h1{fixedFinalPath.hashPart()}; + std::string h2{scratchPath.hashPart()}; + inputRewrites[h1] = h2; + } + + redirectedOutputs.insert_or_assign(std::move(fixedFinalPath), std::move(scratchPath)); + } +} + } // namespace nix diff --git a/src/libstore/build/derivation-builder-impl.hh b/src/libstore/build/derivation-builder-impl.hh index 6f64b846304..c23815ee6e2 100644 --- a/src/libstore/build/derivation-builder-impl.hh +++ b/src/libstore/build/derivation-builder-impl.hh @@ -127,6 +127,44 @@ protected: * and attach them to the derivation */ SingleDrvOutputs checkSubmittedOutputs(); + + /** + * Whether we need to perform hash rewriting if there are valid output paths. + * + * Only a sandbox that can present the outputs at their final paths can skip + * this, which on Unix means a chroot. Windows has no such mechanism, so the + * default is the answer there. + */ + virtual bool needsHashRewrite() + { + return true; + } + + /** + * Create alternative path calculated from but distinct from the + * input, so we can avoid overwriting outputs (or other store paths) + * that already exist. + */ + StorePath makeFallbackPath(const StorePath & path); + + /** + * Make a path to another based on the output name along with the + * derivation hash. + * + * @todo Add option to randomize, so we can audit whether our + * rewrites caught everything + */ + StorePath makeFallbackPath(OutputNameView outputName); + + /** + * Decide the scratch path for each output and populate `inputRewrites` + * so the builder sees placeholders substituted. + * + * Platform-neutral, and needed by every builder: without it + * `hashPlaceholder(outputName)` is never substituted, so a derivation + * cannot refer to its own outputs. + */ + void prepareScratchOutputs(); }; } // namespace nix diff --git a/src/libstore/unix/build/unix-derivation-builder-impl.hh b/src/libstore/unix/build/unix-derivation-builder-impl.hh index 2d289a2c83b..5034b116544 100644 --- a/src/libstore/unix/build/unix-derivation-builder-impl.hh +++ b/src/libstore/unix/build/unix-derivation-builder-impl.hh @@ -131,14 +131,6 @@ protected: friend struct RestrictedStore; - /** - * Whether we need to perform hash rewriting if there are valid output paths. - */ - virtual bool needsHashRewrite() - { - return true; - } - public: std::optional startBuild() override; @@ -332,22 +324,6 @@ public: private: bool decideWhetherDiskFull(); - - /** - * Create alternative path calculated from but distinct from the - * input, so we can avoid overwriting outputs (or other store paths) - * that already exist. - */ - StorePath makeFallbackPath(const StorePath & path); - - /** - * Make a path to another based on the output name along with the - * derivation hash. - * - * @todo Add option to randomize, so we can audit whether our - * rewrites caught everything - */ - StorePath makeFallbackPath(OutputNameView outputName); }; } // namespace nix diff --git a/src/libstore/unix/build/unix-derivation-builder.cc b/src/libstore/unix/build/unix-derivation-builder.cc index 5163e27039d..44c37721e28 100644 --- a/src/libstore/unix/build/unix-derivation-builder.cc +++ b/src/libstore/unix/build/unix-derivation-builder.cc @@ -303,60 +303,7 @@ std::optional UnixDerivationBuilderImpl::startBuild() chownToBuilder(tmpDirFd.get(), tmpDir); - for (auto & [outputName, status] : initialOutputs) { - /* Set scratch path we'll actually use during the build. - - If we're not doing a chroot build, but we have some valid - output paths. Since we can't just overwrite or delete - them, we have to do hash rewriting: i.e. in the - environment/arguments passed to the build, we replace the - hashes of the valid outputs with unique dummy strings; - after the build, we discard the redirected outputs - corresponding to the valid outputs, and rewrite the - contents of the new outputs to replace the dummy strings - with the actual hashes. */ - auto scratchPath = !status.known ? makeFallbackPath(outputName) - : !needsHashRewrite() - /* Can always use original path in sandbox */ - ? status.known->path - : !status.known->isPresent() - /* If path doesn't yet exist can just use it */ - ? status.known->path - : buildMode != bmRepair && !status.known->isValid() - /* If we aren't repairing we'll delete a corrupted path, so we - can use original path */ - ? status.known->path - : /* If we are repairing or the path is totally valid, we'll need - to use a temporary path */ - makeFallbackPath(status.known->path); - scratchOutputs.insert_or_assign(outputName, scratchPath); - - /* Substitute output placeholders with the scratch output paths. - We'll use during the build. */ - inputRewrites[hashPlaceholder(outputName)] = store.printStorePath(scratchPath); - - /* Additional tasks if we know the final path a priori. */ - if (!status.known) - continue; - auto fixedFinalPath = status.known->path; - - /* Additional tasks if the final and scratch are both known and - differ. */ - if (fixedFinalPath == scratchPath) - continue; - - /* Ensure scratch path is ours to use. */ - deletePath(store.printStorePath(scratchPath)); - - /* Rewrite and unrewrite paths */ - { - std::string h1{fixedFinalPath.hashPart()}; - std::string h2{scratchPath.hashPart()}; - inputRewrites[h1] = h2; - } - - redirectedOutputs.insert_or_assign(std::move(fixedFinalPath), std::move(scratchPath)); - } + prepareScratchOutputs(); /* Construct the environment passed to the builder. */ initEnv(); @@ -1039,32 +986,6 @@ void UnixDerivationBuilderImpl::cleanupBuild(bool force) } } -StorePath UnixDerivationBuilderImpl::makeFallbackPath(OutputNameView outputName) -{ - // This is a bogus path type, constructed this way to ensure that it doesn't collide with any other store path - // See doc/manual/source/protocols/store-path.md for details - // TODO: We may want to separate the responsibilities of constructing the path fingerprint and of actually doing the - // hashing - auto pathType = "rewrite:" + std::string(drvPath.to_string()) + ":name:" + std::string(outputName); - return store.makeStorePath( - pathType, - // pass an all-zeroes hash - Hash(HashAlgorithm::SHA256), - outputPathName(drv.name, outputName)); -} - -StorePath UnixDerivationBuilderImpl::makeFallbackPath(const StorePath & path) -{ - // This is a bogus path type, constructed this way to ensure that it doesn't collide with any other store path - // See doc/manual/source/protocols/store-path.md for details - auto pathType = "rewrite:" + std::string(drvPath.to_string()) + ":" + std::string(path.to_string()); - return store.makeStorePath( - pathType, - // pass an all-zeroes hash - Hash(HashAlgorithm::SHA256), - path.name()); -} - } // namespace nix namespace nix { diff --git a/src/libstore/windows/build/windows-derivation-builder.cc b/src/libstore/windows/build/windows-derivation-builder.cc index ae78cf18263..0ebec470e6d 100644 --- a/src/libstore/windows/build/windows-derivation-builder.cc +++ b/src/libstore/windows/build/windows-derivation-builder.cc @@ -305,15 +305,14 @@ std::optional WindowsDerivationBuilderImpl::startBuild() if (drv.isBuiltin()) throw UnimplementedError("builtin builders are not yet supported on Windows"); - for (auto & [name, output] : drv.outputs) { - auto * ia = std::get_if(&output.raw); - if (!ia) - throw UnimplementedError( - "only input-addressed derivation outputs are supported on Windows, but output '%s' is not one", name); - /* Without a sandbox there is nowhere else to build, so the scratch - path is the final one and `registerOutputs` has nothing to rewrite. */ - scratchOutputs.insert_or_assign(name, ia->path); - } + /* Decides the scratch path per output and fills in `inputRewrites`. This + replaces a loop that assumed every output was input-addressed and threw + otherwise, which is what previously kept content-addressed and + fixed-output derivations from building here at all: they were rejected + before the build started rather than by anything downstream. The shared + version works off `initialOutputs`, so it handles the cases where the + final path is not known up front. */ + prepareScratchOutputs(); /* A fresh build directory per attempt. */ tmpDir = createTempDir(defaultTempDir(), "nix-build"); From f273d83570f8a7bc9bd4b3e694004cc1e22a903e Mon Sep 17 00:00:00 2001 From: Madison Steiner Date: Tue, 1 Sep 2026 16:32:37 +0000 Subject: [PATCH 6/7] libstore: apply inputRewrites in the Windows builder The previous commit populates `inputRewrites` on Windows; nothing consumed it, so `hashPlaceholder(outputName)` still reached the builder verbatim and a derivation could not name its own outputs. The Unix builder substitutes in three places --- the environment block, the command line, and `extraFiles` Assisted-by: Claude Code (claude-opus-5) --- and the first two now happen here too. Without a sandbox this substitution is the *only* mechanism by which a Windows derivation can refer to its outputs, since there is no chroot that could present them at their final paths instead. Also refreshes the class comment, which still claimed content-addressed and fixed-output derivations were unsupported and that there was never anything to rewrite. Both stopped being true one commit ago. `extraFiles` remains unimplemented on Windows and is left for a follow-up: it needs a `writeBuilderFile` equivalent, and the natural one to reuse is the `OsFilename`-typed version from NixOS/nix#15244, which has not landed. Verified: pre-commit 7/7; `x86_64-w64-mingw32` compiles clean; nix-store-tests 792/787 natively and 784/753 under Wine, both unchanged and with zero failures. --- .../build/windows-derivation-builder.cc | 29 +++++++++++++------ 1 file changed, 20 insertions(+), 9 deletions(-) diff --git a/src/libstore/windows/build/windows-derivation-builder.cc b/src/libstore/windows/build/windows-derivation-builder.cc index 0ebec470e6d..a3a9842c037 100644 --- a/src/libstore/windows/build/windows-derivation-builder.cc +++ b/src/libstore/windows/build/windows-derivation-builder.cc @@ -7,6 +7,7 @@ #include "nix/util/muxable-pipe.hh" #include "nix/util/os-string.hh" #include "nix/util/processes.hh" +#include "nix/util/util.hh" #include @@ -73,19 +74,23 @@ OsString escapeArg(OsString arg) } /** - * A minimal, unsandboxed `DerivationBuilder` for Windows: enough to run a - * builder and register its outputs, and no more. Missing, relative to Unix: + * An unsandboxed `DerivationBuilder` for Windows. Missing, relative to Unix: * * - no sandbox, chroot, or filesystem isolation * - no build user; the builder runs as whoever ran Nix * - no network isolation * - no recursive Nix (`submitOutput` throws) - * - no content-addressed or fixed-output derivations * * Registering the outputs is `DerivationBuilderImpl::registerOutputs`, the * same code Unix runs, so reference scanning and the `allowedReferences` - * checks do apply. Without a sandbox the scratch paths are the final ones, - * so it finds nothing to rewrite. + * checks do apply. + * + * Scratch paths come from the shared `prepareScratchOutputs`, so an output + * whose final path is not known up front -- content-addressed, or fixed-output + * being repaired -- gets a temporary one and is rewritten afterwards, exactly + * as on Unix. Because there is no sandbox, the placeholder substitution in + * `inputRewrites` is the only way a derivation can name its own outputs, so + * both the environment block and the command line go through it. */ class WindowsDerivationBuilderImpl : public DerivationBuilderImpl { @@ -212,9 +217,14 @@ OsString WindowsDerivationBuilderImpl::makeEnvBlock() if (auto value = getEnvOs(os(name))) env[os(name)] = *value; - /* The derivation's own environment wins over all of the above. */ + /* The derivation's own environment wins over all of the above. + + `inputRewrites` substitutes each output placeholder with the scratch path + chosen for it, which is how a derivation refers to its own outputs. The + Unix builder does the same to its environment block; without it the + placeholder reaches the builder verbatim. */ for (auto & [name, entry] : desugaredEnv.variables) - env[os(name)] = os(entry.value); + env[os(name)] = os(rewriteStrings(entry.value, inputRewrites)); OsString block; for (auto & [name, value] : env) { @@ -244,10 +254,11 @@ void WindowsDerivationBuilderImpl::spawnBuilder() startInfo.hStdOutput = builderPipe.writeSide.get(); startInfo.hStdError = builderPipe.writeSide.get(); - OsString cmdline = escapeArg(string_to_os_string(std::string_view{drv.builder})); + /* Same placeholder substitution as the environment block above. */ + OsString cmdline = escapeArg(string_to_os_string(rewriteStrings(drv.builder, inputRewrites))); for (auto & arg : drv.args) { cmdline += L' '; - cmdline += escapeArg(string_to_os_string(std::string_view{arg})); + cmdline += escapeArg(string_to_os_string(rewriteStrings(arg, inputRewrites))); } auto envBlock = makeEnvBlock(); From 9afc0ca68f42310aa01a9b4c45e5ed0f075f8afc Mon Sep 17 00:00:00 2001 From: Madison Steiner Date: Tue, 1 Sep 2026 19:20:46 +0000 Subject: [PATCH 7/7] libutil: don't anchor the filesystem accessor at "/" on Windows `getFSSourceAccessor` passed the literal "/" as its root. `std::filesystem::path("/").is_absolute()` is false on Windows --- that path has a root directory but no root name, and `is_absolute()` requires both --- so `WindowsSourceAccessor`'s constructor assertion fired. `EvalState` reaches here through `getFSSourceAccessor`, which meant every command that evaluates anything aborted: Assertion failed: root.empty() || root.is_absolute(), file src/libutil/posix-source-accessor.cc, line 648 An empty root is how this accessor already spells "no prefix, the paths I am given are absolute already": `WindowsSourceAccessor::makeAbsPath` handles it explicitly, and the assertion permits it. That is also the honest description of Windows, which has no single filesystem root because paths are rooted per drive. Measured with a cross-compiled `nix.exe` under Wine. Before, `nix eval --expr '1 + 1'` aborted on the assertion; after, it prints 2, and `"a" + "b"` prints "ab". `nix --version` and `nix store info` worked either way, since neither constructs an evaluator. Reading a `.nix` file still fails, with a different bug --- a native path gets a "/" prepended, giving `path '/Z:\tmp\t.nix' does not exist`. That is concatenation rather than validation and is left for a separate change. Verified: pre-commit 7/7; native nix-util-tests 813/811 and nix-store-tests 792/787, both unchanged; nix-store-tests under Wine 784/753 with zero failures, confirmed by a forced rebuild rather than a cache hit. Assisted-by: Claude Code (claude-opus-5) --- src/libutil/posix-source-accessor.cc | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/src/libutil/posix-source-accessor.cc b/src/libutil/posix-source-accessor.cc index fd74e99e983..1f00d2cd4bc 100644 --- a/src/libutil/posix-source-accessor.cc +++ b/src/libutil/posix-source-accessor.cc @@ -804,7 +804,23 @@ void WindowsSourceAccessor::assertNoSymlinks(CanonPath path) ref getFSSourceAccessor() { - static auto rootFS = makeFSSourceAccessor("/", /*trackLastModified=*/false); + static auto rootFS = makeFSSourceAccessor( +#ifdef _WIN32 + /* Windows has no single filesystem root to anchor to: paths are rooted + per drive, so `/` is not an absolute path there --- it has a root + directory but no root name, and `is_absolute()` wants both. An empty + root is how this accessor already spells "no prefix, the paths handed + to me are absolute already"; see `WindowsSourceAccessor::makeAbsPath`, + and the `root.empty()` arm of the assertion in its constructor. + + Passing `/` made that assertion fail, which aborted every command + that evaluates, since `EvalState` reaches here through + `getFSSourceAccessor`. */ + std::filesystem::path{}, +#else + "/", +#endif + /*trackLastModified=*/false); return rootFS; }