From f6aeca052212c341c3bae4b4da9f983c6804623e Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Wed, 15 Oct 2025 22:14:59 +0200 Subject: [PATCH 01/40] Clarify setStackSize error message Show the actual attempted stack size value (capped at hard limit) separately from the desired value, making it clearer what's happening when the hard limit is lower than requested. --- src/libutil/current-process.cc | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/libutil/current-process.cc b/src/libutil/current-process.cc index c7d3b78d0..988621014 100644 --- a/src/libutil/current-process.cc +++ b/src/libutil/current-process.cc @@ -65,13 +65,15 @@ void setStackSize(size_t stackSize) struct rlimit limit; if (getrlimit(RLIMIT_STACK, &limit) == 0 && static_cast(limit.rlim_cur) < stackSize) { savedStackSize = limit.rlim_cur; - limit.rlim_cur = std::min(static_cast(stackSize), limit.rlim_max); + auto requestedSize = std::min(static_cast(stackSize), limit.rlim_max); + limit.rlim_cur = requestedSize; if (setrlimit(RLIMIT_STACK, &limit) != 0) { logger->log( lvlError, HintFmt( - "Failed to increase stack size from %1% to %2% (maximum allowed stack size: %3%): %4%", + "Failed to increase stack size from %1% to %2% (desired: %3%, maximum allowed: %4%): %5%", savedStackSize, + requestedSize, stackSize, limit.rlim_max, std::strerror(errno)) From 2349c3dbde77ea7ef32a89eea2c09048d93c671e Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Wed, 15 Oct 2025 22:40:31 +0200 Subject: [PATCH 02/40] setStackSize: Warn when the desired stack size can't be set --- src/libutil/current-process.cc | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/libutil/current-process.cc b/src/libutil/current-process.cc index 988621014..10522d5ae 100644 --- a/src/libutil/current-process.cc +++ b/src/libutil/current-process.cc @@ -65,6 +65,16 @@ void setStackSize(size_t stackSize) struct rlimit limit; if (getrlimit(RLIMIT_STACK, &limit) == 0 && static_cast(limit.rlim_cur) < stackSize) { savedStackSize = limit.rlim_cur; + if (limit.rlim_max < static_cast(stackSize)) { + logger->log( + lvlWarn, + HintFmt( + "Stack size hard limit is %1%, which is less than the desired %2%. If possible, increase the hard limit, e.g. with 'ulimit -Hs %3%'.", + limit.rlim_max, + stackSize, + stackSize / 1024) + .str()); + } auto requestedSize = std::min(static_cast(stackSize), limit.rlim_max); limit.rlim_cur = requestedSize; if (setrlimit(RLIMIT_STACK, &limit) != 0) { From 08e218eb0b315bf176923dd331318c8c28d270b7 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Tue, 4 Nov 2025 22:51:33 +0100 Subject: [PATCH 03/40] Reduce the stack size to a bit under 64 MiB --- src/nix/main.cc | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/nix/main.cc b/src/nix/main.cc index 74d22e433..217e5fa9b 100644 --- a/src/nix/main.cc +++ b/src/nix/main.cc @@ -566,7 +566,9 @@ int main(int argc, char ** argv) #ifndef _WIN32 // Increase the default stack size for the evaluator and for // libstdc++'s std::regex. - nix::setStackSize(64 * 1024 * 1024); + // This used to be 64 MiB, but macOS as deployed on GitHub Actions has a + // hard limit slightly under that, so we round it down a bit. + nix::setStackSize(60 * 1024 * 1024); #endif return nix::handleExceptions(argv[0], [&]() { nix::mainWrapped(argc, argv); }); From 261f674a2531eb4d4a5473a0179ec792dbd79ca2 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Tue, 4 Nov 2025 23:14:21 +0100 Subject: [PATCH 04/40] tests: Suppress environment-dependent warnings ... via _NIX_TEST_NO_ENVIRONMENT_WARNINGS This environment variable suppresses warnings that depend on the test environment (such as ulimit warnings in builds on systems with lower limits, which may well succeed if it weren't for the warning). This prevents non-deterministic test failures in golden/characterization tests. Alternative considered: filtering stderr in test scripts, but that approach is fragile with binary test output, and potentially multiple call sites. --- src/libutil/current-process.cc | 19 +++++++++++-------- tests/functional/common/vars.sh | 3 +++ 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/src/libutil/current-process.cc b/src/libutil/current-process.cc index 10522d5ae..c07ed8371 100644 --- a/src/libutil/current-process.cc +++ b/src/libutil/current-process.cc @@ -7,6 +7,7 @@ #include "nix/util/file-system.hh" #include "nix/util/processes.hh" #include "nix/util/signals.hh" +#include "nix/util/environment-variables.hh" #include #ifdef __APPLE__ @@ -66,14 +67,16 @@ void setStackSize(size_t stackSize) if (getrlimit(RLIMIT_STACK, &limit) == 0 && static_cast(limit.rlim_cur) < stackSize) { savedStackSize = limit.rlim_cur; if (limit.rlim_max < static_cast(stackSize)) { - logger->log( - lvlWarn, - HintFmt( - "Stack size hard limit is %1%, which is less than the desired %2%. If possible, increase the hard limit, e.g. with 'ulimit -Hs %3%'.", - limit.rlim_max, - stackSize, - stackSize / 1024) - .str()); + if (getEnv("_NIX_TEST_NO_ENVIRONMENT_WARNINGS") != "1") { + logger->log( + lvlWarn, + HintFmt( + "Stack size hard limit is %1%, which is less than the desired %2%. If possible, increase the hard limit, e.g. with 'ulimit -Hs %3%'.", + limit.rlim_max, + stackSize, + stackSize / 1024) + .str()); + } } auto requestedSize = std::min(static_cast(stackSize), limit.rlim_max); limit.rlim_cur = requestedSize; diff --git a/tests/functional/common/vars.sh b/tests/functional/common/vars.sh index ed4b47727..d4d917dae 100644 --- a/tests/functional/common/vars.sh +++ b/tests/functional/common/vars.sh @@ -49,6 +49,9 @@ if ! isTestOnNixOS; then fi export _NIX_IN_TEST=$TEST_ROOT/shared export _NIX_TEST_NO_LSOF=1 + # Suppress warnings that depend on the test environment (e.g., ulimit warnings) + # to avoid non-deterministic test failures in golden tests + export _NIX_TEST_NO_ENVIRONMENT_WARNINGS=1 export NIX_REMOTE=${NIX_REMOTE_-} fi # ! isTestOnNixOS From 294acfd807ec046d73c58e948c195cc466935aa7 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Thu, 13 Nov 2025 02:27:19 -0500 Subject: [PATCH 05/40] Create infrastructure for "checkpoint" characterization tests These do a read/write test in the middles of some computation. They are an imperative way to test intermediate values rather than functionally testing end outputs. --- .../nix/util/tests/characterization.hh | 4 +-- .../nix/util/tests/json-characterization.hh | 29 +++++++++++++++++++ 2 files changed, 30 insertions(+), 3 deletions(-) diff --git a/src/libutil-test-support/include/nix/util/tests/characterization.hh b/src/libutil-test-support/include/nix/util/tests/characterization.hh index 0434590f7..d8fad1df9 100644 --- a/src/libutil-test-support/include/nix/util/tests/characterization.hh +++ b/src/libutil-test-support/include/nix/util/tests/characterization.hh @@ -31,16 +31,14 @@ static inline bool testAccept() /** * Mixin class for writing characterization tests */ -class CharacterizationTest : public virtual ::testing::Test +struct CharacterizationTest : virtual ::testing::Test { -protected: /** * While the "golden master" for this characterization test is * located. It should not be shared with any other test. */ virtual std::filesystem::path goldenMaster(PathView testStem) const = 0; -public: /** * Golden test for reading * diff --git a/src/libutil-test-support/include/nix/util/tests/json-characterization.hh b/src/libutil-test-support/include/nix/util/tests/json-characterization.hh index 0ee6fd2fd..bd70ba830 100644 --- a/src/libutil-test-support/include/nix/util/tests/json-characterization.hh +++ b/src/libutil-test-support/include/nix/util/tests/json-characterization.hh @@ -39,6 +39,30 @@ void writeJsonTest(CharacterizationTest & test, PathView testStem, const T & val [](const auto & file, const auto & got) { return writeFile(file, got.dump(2) + "\n"); }); } +/** + * Golden test in the middle of something + */ +template +void checkpointJson(CharacterizationTest & test, PathView testStem, const T & got) +{ + using namespace nlohmann; + + auto file = test.goldenMaster(Path{testStem} + ".json"); + + json gotJson = static_cast(got); + + if (testAccept()) { + std::filesystem::create_directories(file.parent_path()); + writeFile(file, gotJson.dump(2) + "\n"); + ADD_FAILURE() << "Updating golden master " << file; + } else { + json expectedJson = json::parse(readFile(file)); + ASSERT_EQ(gotJson, expectedJson); + T expected = adl_serializer::from_json(expectedJson); + ASSERT_EQ(got, expected); + } +} + /** * Mixin class for writing characterization tests for `nlohmann::json` * conversions for a given type. @@ -67,6 +91,11 @@ struct JsonCharacterizationTest : virtual CharacterizationTest { nix::writeJsonTest(*this, testStem, value); } + + void checkpointJson(PathView testStem, const T & value) + { + nix::checkpointJson(*this, testStem, value); + } }; } // namespace nix From 620a6947ab80795ae28dc1ffe2857f2c08d7ce9a Mon Sep 17 00:00:00 2001 From: John Ericson Date: Thu, 13 Nov 2025 02:27:19 -0500 Subject: [PATCH 06/40] Dedup some derivation initialization logic, and test `nix derivation add`, and its C API counterpart, now works a bit closer to `builtins.derivation` in that they don't require the user to fill-in input addressed paths correctly ahead of time. The logic for this is carefully deduplicated, between all 3 entry points, and also between the existing `checkInvariants` function. There are some more functional tests, and there are also many more unit tests. Co-authored-by: Sergei Zimmerman Co-authored-by: edef --- src/libexpr/primops.cc | 23 +- src/libstore-c/nix_api_store.cc | 8 +- .../invariants/bad-depends-on-drv-pre.json | 27 ++ .../derivation/invariants/bad-env-var.json | 18 ++ .../data/derivation/invariants/bad-path.json | 20 ++ .../invariants/depends-on-drv-pre.json | 25 ++ ...filled-in-deferred-empty-env-var-post.json | 20 ++ .../filled-in-deferred-empty-env-var-pre.json | 18 ++ .../filled-in-deferred-no-env-var-post.json | 20 ++ .../filled-in-deferred-no-env-var-pre.json | 17 ++ .../invariants/filled-in-idempotent.json | 20 ++ .../external-formats.cc} | 45 +-- src/libstore-tests/derivation/invariants.cc | 264 ++++++++++++++++++ src/libstore-tests/derivation/test-support.hh | 52 ++++ src/libstore-tests/meson.build | 3 +- src/libstore/derivations.cc | 218 +++++++++++---- src/libstore/include/nix/store/derivations.hh | 62 ++++ src/nix/derivation-add.cc | 4 +- tests/functional/derivation-json.sh | 17 +- 19 files changed, 744 insertions(+), 137 deletions(-) create mode 100644 src/libstore-tests/data/derivation/invariants/bad-depends-on-drv-pre.json create mode 100644 src/libstore-tests/data/derivation/invariants/bad-env-var.json create mode 100644 src/libstore-tests/data/derivation/invariants/bad-path.json create mode 100644 src/libstore-tests/data/derivation/invariants/depends-on-drv-pre.json create mode 100644 src/libstore-tests/data/derivation/invariants/filled-in-deferred-empty-env-var-post.json create mode 100644 src/libstore-tests/data/derivation/invariants/filled-in-deferred-empty-env-var-pre.json create mode 100644 src/libstore-tests/data/derivation/invariants/filled-in-deferred-no-env-var-post.json create mode 100644 src/libstore-tests/data/derivation/invariants/filled-in-deferred-no-env-var-pre.json create mode 100644 src/libstore-tests/data/derivation/invariants/filled-in-idempotent.json rename src/libstore-tests/{derivation.cc => derivation/external-formats.cc} (91%) create mode 100644 src/libstore-tests/derivation/invariants.cc create mode 100644 src/libstore-tests/derivation/test-support.hh diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index 35f16a68d..05ba7475e 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -1774,28 +1774,7 @@ static void derivationStrictInternal(EvalState & state, std::string_view drvName drv.outputs.insert_or_assign(i, DerivationOutput::Deferred{}); } - auto hashModulo = hashDerivationModulo(*state.store, Derivation(drv), true); - switch (hashModulo.kind) { - case DrvHash::Kind::Regular: - for (auto & i : outputs) { - auto h = get(hashModulo.hashes, i); - if (!h) - state.error("derivation produced no hash for output '%s'", i).atPos(v).debugThrow(); - auto outPath = state.store->makeOutputPath(i, *h, drvName); - drv.env[i] = state.store->printStorePath(outPath); - drv.outputs.insert_or_assign( - i, - DerivationOutput::InputAddressed{ - .path = std::move(outPath), - }); - } - break; - ; - case DrvHash::Kind::Deferred: - for (auto & i : outputs) { - drv.outputs.insert_or_assign(i, DerivationOutput::Deferred{}); - } - } + drv.fillInOutputPaths(*state.store); } /* Write the resulting term into the Nix store directory. */ diff --git a/src/libstore-c/nix_api_store.cc b/src/libstore-c/nix_api_store.cc index 024ed9785..b95e5b749 100644 --- a/src/libstore-c/nix_api_store.cc +++ b/src/libstore-c/nix_api_store.cc @@ -223,13 +223,7 @@ nix_derivation * nix_derivation_from_json(nix_c_context * context, Store * store if (context) context->last_err_code = NIX_OK; try { - auto drv = static_cast(nlohmann::json::parse(json)); - - auto drvPath = nix::writeDerivation(*store->ptr, drv, nix::NoRepair, /* read only */ true); - - drv.checkInvariants(*store->ptr, drvPath); - - return new nix_derivation{drv}; + return new nix_derivation{nix::Derivation::parseJsonAndValidate(*store->ptr, nlohmann::json::parse(json))}; } NIXC_CATCH_ERRS_NULL } diff --git a/src/libstore-tests/data/derivation/invariants/bad-depends-on-drv-pre.json b/src/libstore-tests/data/derivation/invariants/bad-depends-on-drv-pre.json new file mode 100644 index 000000000..8454cf548 --- /dev/null +++ b/src/libstore-tests/data/derivation/invariants/bad-depends-on-drv-pre.json @@ -0,0 +1,27 @@ +{ + "args": [], + "builder": "/bin/sh", + "env": { + "__doc": "InputAddressed throws when should be deferred", + "out": "" + }, + "inputs": { + "drvs": { + "lg4c4b8r9hlczwprl6kgnzfd9mc1xmkk-dependency.drv": { + "dynamicOutputs": {}, + "outputs": [ + "out" + ] + } + }, + "srcs": [] + }, + "name": "depends-on-drv", + "outputs": { + "out": { + "path": "c015dhfh5l0lp6wxyvdn7bmwhbbr6hr9-wrong-name" + } + }, + "system": "x86_64-linux", + "version": 4 +} diff --git a/src/libstore-tests/data/derivation/invariants/bad-env-var.json b/src/libstore-tests/data/derivation/invariants/bad-env-var.json new file mode 100644 index 000000000..cb0c9492f --- /dev/null +++ b/src/libstore-tests/data/derivation/invariants/bad-env-var.json @@ -0,0 +1,18 @@ +{ + "args": [], + "builder": "/bin/sh", + "env": { + "__doc": "Wrong env var value throws error", + "out": "/nix/store/c015dhfh5l0lp6wxyvdn7bmwhbbr6hr9-wrong-name" + }, + "inputs": { + "drvs": {}, + "srcs": [] + }, + "name": "bad-env-var", + "outputs": { + "out": {} + }, + "system": "x86_64-linux", + "version": 4 +} diff --git a/src/libstore-tests/data/derivation/invariants/bad-path.json b/src/libstore-tests/data/derivation/invariants/bad-path.json new file mode 100644 index 000000000..688f2d4e6 --- /dev/null +++ b/src/libstore-tests/data/derivation/invariants/bad-path.json @@ -0,0 +1,20 @@ +{ + "args": [], + "builder": "/bin/sh", + "env": { + "__doc": "Wrong InputAddressed path throws error", + "out": "/nix/store/c015dhfh5l0lp6wxyvdn7bmwhbbr6hr9-wrong-name" + }, + "inputs": { + "drvs": {}, + "srcs": [] + }, + "name": "bad-path", + "outputs": { + "out": { + "path": "c015dhfh5l0lp6wxyvdn7bmwhbbr6hr9-wrong-name" + } + }, + "system": "x86_64-linux", + "version": 4 +} diff --git a/src/libstore-tests/data/derivation/invariants/depends-on-drv-pre.json b/src/libstore-tests/data/derivation/invariants/depends-on-drv-pre.json new file mode 100644 index 000000000..d782cc756 --- /dev/null +++ b/src/libstore-tests/data/derivation/invariants/depends-on-drv-pre.json @@ -0,0 +1,25 @@ +{ + "args": [], + "builder": "/bin/sh", + "env": { + "__doc": "Deferred stays deferred with CA dependencies", + "out": "" + }, + "inputs": { + "drvs": { + "lg4c4b8r9hlczwprl6kgnzfd9mc1xmkk-dependency.drv": { + "dynamicOutputs": {}, + "outputs": [ + "out" + ] + } + }, + "srcs": [] + }, + "name": "depends-on-drv", + "outputs": { + "out": {} + }, + "system": "x86_64-linux", + "version": 4 +} diff --git a/src/libstore-tests/data/derivation/invariants/filled-in-deferred-empty-env-var-post.json b/src/libstore-tests/data/derivation/invariants/filled-in-deferred-empty-env-var-post.json new file mode 100644 index 000000000..c5abdf692 --- /dev/null +++ b/src/libstore-tests/data/derivation/invariants/filled-in-deferred-empty-env-var-post.json @@ -0,0 +1,20 @@ +{ + "args": [], + "builder": "/bin/sh", + "env": { + "__doc": "Fill in deferred output with empty env var", + "out": "/nix/store/bilpz1nq8qi9r3bzsp72n34yjgqg43ws-filled-in-deferred-empty-env-var" + }, + "inputs": { + "drvs": {}, + "srcs": [] + }, + "name": "filled-in-deferred-empty-env-var", + "outputs": { + "out": { + "path": "bilpz1nq8qi9r3bzsp72n34yjgqg43ws-filled-in-deferred-empty-env-var" + } + }, + "system": "x86_64-linux", + "version": 4 +} diff --git a/src/libstore-tests/data/derivation/invariants/filled-in-deferred-empty-env-var-pre.json b/src/libstore-tests/data/derivation/invariants/filled-in-deferred-empty-env-var-pre.json new file mode 100644 index 000000000..bc5338925 --- /dev/null +++ b/src/libstore-tests/data/derivation/invariants/filled-in-deferred-empty-env-var-pre.json @@ -0,0 +1,18 @@ +{ + "args": [], + "builder": "/bin/sh", + "env": { + "__doc": "Fill in deferred output with empty env var", + "out": "" + }, + "inputs": { + "drvs": {}, + "srcs": [] + }, + "name": "filled-in-deferred-empty-env-var", + "outputs": { + "out": {} + }, + "system": "x86_64-linux", + "version": 4 +} diff --git a/src/libstore-tests/data/derivation/invariants/filled-in-deferred-no-env-var-post.json b/src/libstore-tests/data/derivation/invariants/filled-in-deferred-no-env-var-post.json new file mode 100644 index 000000000..709d7bca0 --- /dev/null +++ b/src/libstore-tests/data/derivation/invariants/filled-in-deferred-no-env-var-post.json @@ -0,0 +1,20 @@ +{ + "args": [], + "builder": "/bin/sh", + "env": { + "__doc": "Fill in deferred with missing env var", + "out": "/nix/store/wpk9qrgg77fyswhailap0gicgw98izx9-filled-in-deferred-no-env-var" + }, + "inputs": { + "drvs": {}, + "srcs": [] + }, + "name": "filled-in-deferred-no-env-var", + "outputs": { + "out": { + "path": "wpk9qrgg77fyswhailap0gicgw98izx9-filled-in-deferred-no-env-var" + } + }, + "system": "x86_64-linux", + "version": 4 +} diff --git a/src/libstore-tests/data/derivation/invariants/filled-in-deferred-no-env-var-pre.json b/src/libstore-tests/data/derivation/invariants/filled-in-deferred-no-env-var-pre.json new file mode 100644 index 000000000..194e33086 --- /dev/null +++ b/src/libstore-tests/data/derivation/invariants/filled-in-deferred-no-env-var-pre.json @@ -0,0 +1,17 @@ +{ + "args": [], + "builder": "/bin/sh", + "env": { + "__doc": "Fill in deferred with missing env var" + }, + "inputs": { + "drvs": {}, + "srcs": [] + }, + "name": "filled-in-deferred-no-env-var", + "outputs": { + "out": {} + }, + "system": "x86_64-linux", + "version": 4 +} diff --git a/src/libstore-tests/data/derivation/invariants/filled-in-idempotent.json b/src/libstore-tests/data/derivation/invariants/filled-in-idempotent.json new file mode 100644 index 000000000..9b99fb812 --- /dev/null +++ b/src/libstore-tests/data/derivation/invariants/filled-in-idempotent.json @@ -0,0 +1,20 @@ +{ + "args": [], + "builder": "/bin/sh", + "env": { + "__doc": "Correct path stays unchanged", + "out": "/nix/store/w4bk7hpyxzgy2gx8fsa8f952435pll3i-filled-in-already" + }, + "inputs": { + "drvs": {}, + "srcs": [] + }, + "name": "filled-in-already", + "outputs": { + "out": { + "path": "w4bk7hpyxzgy2gx8fsa8f952435pll3i-filled-in-already" + } + }, + "system": "x86_64-linux", + "version": 4 +} diff --git a/src/libstore-tests/derivation.cc b/src/libstore-tests/derivation/external-formats.cc similarity index 91% rename from src/libstore-tests/derivation.cc rename to src/libstore-tests/derivation/external-formats.cc index 6b33e5442..056eeaa8a 100644 --- a/src/libstore-tests/derivation.cc +++ b/src/libstore-tests/derivation/external-formats.cc @@ -1,57 +1,14 @@ #include #include -#include "nix/util/experimental-features.hh" #include "nix/store/derivations.hh" - -#include "nix/store/tests/libstore.hh" +#include "derivation/test-support.hh" #include "nix/util/tests/json-characterization.hh" namespace nix { using nlohmann::json; -class DerivationTest : public virtual CharacterizationTest, public LibStoreTest -{ - std::filesystem::path unitTestData = getUnitTestData() / "derivation"; - -public: - std::filesystem::path goldenMaster(std::string_view testStem) const override - { - return unitTestData / testStem; - } - - /** - * We set these in tests rather than the regular globals so we don't have - * to worry about race conditions if the tests run concurrently. - */ - ExperimentalFeatureSettings mockXpSettings; -}; - -class CaDerivationTest : public DerivationTest -{ - void SetUp() override - { - mockXpSettings.set("experimental-features", "ca-derivations"); - } -}; - -class DynDerivationTest : public DerivationTest -{ - void SetUp() override - { - mockXpSettings.set("experimental-features", "dynamic-derivations ca-derivations"); - } -}; - -class ImpureDerivationTest : public DerivationTest -{ - void SetUp() override - { - mockXpSettings.set("experimental-features", "impure-derivations"); - } -}; - TEST_F(DerivationTest, BadATerm_version) { ASSERT_THROW( diff --git a/src/libstore-tests/derivation/invariants.cc b/src/libstore-tests/derivation/invariants.cc new file mode 100644 index 000000000..6d7cee968 --- /dev/null +++ b/src/libstore-tests/derivation/invariants.cc @@ -0,0 +1,264 @@ +#include +#include + +#include "nix/store/derivations.hh" +#include "nix/store/tests/libstore.hh" +#include "nix/store/dummy-store-impl.hh" +#include "nix/util/tests/json-characterization.hh" + +#include "derivation/test-support.hh" + +namespace nix { + +class FillInOutputPathsTest : public LibStoreTest, public JsonCharacterizationTest +{ + std::filesystem::path unitTestData = getUnitTestData() / "derivation" / "invariants"; + +protected: + FillInOutputPathsTest() + : LibStoreTest([]() { + auto config = make_ref(DummyStoreConfig::Params{}); + config->readOnly = false; + return config->openDummyStore(); + }()) + { + } + + /** + * Create a CA floating output derivation and write it to the store. + * This is useful for creating dependencies that will cause downstream + * derivations to remain deferred. + */ + StorePath makeCAFloatingDependency(std::string_view name) + { + Derivation depDrv; + depDrv.name = name; + depDrv.platform = "x86_64-linux"; + depDrv.builder = "/bin/sh"; + depDrv.outputs = { + { + "out", + // will ensure that downstream is deferred + DerivationOutput{DerivationOutput::CAFloating{ + .method = ContentAddressMethod::Raw::NixArchive, + .hashAlgo = HashAlgorithm::SHA256, + }}, + }, + }; + depDrv.env = {{"out", ""}}; + + // Fill in the dependency derivation's output paths + depDrv.fillInOutputPaths(*store); + + // Write the dependency to the store + return writeDerivation(*store, depDrv, NoRepair); + } + +public: + std::filesystem::path goldenMaster(std::string_view testStem) const override + { + return unitTestData / testStem; + } +}; + +TEST_F(FillInOutputPathsTest, fillsDeferredOutputs_emptyStringEnvVar) +{ + using nlohmann::json; + + // Before: Derivation with deferred output + Derivation drv; + drv.name = "filled-in-deferred-empty-env-var"; + drv.platform = "x86_64-linux"; + drv.builder = "/bin/sh"; + drv.outputs = { + {"out", DerivationOutput{DerivationOutput::Deferred{}}}, + }; + drv.env = {{"__doc", "Fill in deferred output with empty env var"}, {"out", ""}}; + + // Serialize before state + checkpointJson("filled-in-deferred-empty-env-var-pre", drv); + + drv.fillInOutputPaths(*store); + + // Serialize after state + checkpointJson("filled-in-deferred-empty-env-var-post", drv); + + // After: Should have been converted to InputAddressed + auto * outputP = std::get_if(&drv.outputs.at("out").raw); + ASSERT_TRUE(outputP); + auto & output = *outputP; + + // Environment variable should be filled in + EXPECT_EQ(drv.env.at("out"), store->printStorePath(output.path)); +} + +TEST_F(FillInOutputPathsTest, fillsDeferredOutputs_empty_string_var) +{ + using nlohmann::json; + + // Before: Derivation with deferred output + Derivation drv; + drv.name = "filled-in-deferred-no-env-var"; + drv.platform = "x86_64-linux"; + drv.builder = "/bin/sh"; + drv.outputs = { + {"out", DerivationOutput{DerivationOutput::Deferred{}}}, + }; + drv.env = { + {"__doc", "Fill in deferred with missing env var"}, + }; + + // Serialize before state + checkpointJson("filled-in-deferred-no-env-var-pre", drv); + + drv.fillInOutputPaths(*store); + + // Serialize after state + checkpointJson("filled-in-deferred-no-env-var-post", drv); + + // After: Should have been converted to InputAddressed + auto * outputP = std::get_if(&drv.outputs.at("out").raw); + ASSERT_TRUE(outputP); + auto & output = *outputP; + + // Environment variable should be filled in + EXPECT_EQ(drv.env.at("out"), store->printStorePath(output.path)); +} + +TEST_F(FillInOutputPathsTest, preservesInputAddressedOutputs) +{ + auto expectedPath = StorePath{"w4bk7hpyxzgy2gx8fsa8f952435pll3i-filled-in-already"}; + + Derivation drv; + drv.name = "filled-in-already"; + drv.platform = "x86_64-linux"; + drv.builder = "/bin/sh"; + drv.outputs = { + {"out", DerivationOutput{DerivationOutput::InputAddressed{.path = expectedPath}}}, + }; + drv.env = { + {"__doc", "Correct path stays unchanged"}, + {"out", store->printStorePath(expectedPath)}, + }; + + // Serialize before state + checkpointJson("filled-in-idempotent", drv); + + auto drvBefore = drv; + + drv.fillInOutputPaths(*store); + + // Should still be no change + EXPECT_EQ(drv, drvBefore); +} + +TEST_F(FillInOutputPathsTest, throwsOnIncorrectInputAddressedPath) +{ + auto wrongPath = StorePath{"c015dhfh5l0lp6wxyvdn7bmwhbbr6hr9-wrong-name"}; + + Derivation drv; + drv.name = "bad-path"; + drv.platform = "x86_64-linux"; + drv.builder = "/bin/sh"; + drv.outputs = { + {"out", DerivationOutput{DerivationOutput::InputAddressed{.path = wrongPath}}}, + }; + drv.env = { + {"__doc", "Wrong InputAddressed path throws error"}, + {"out", store->printStorePath(wrongPath)}, + }; + + // Serialize before state + checkpointJson("bad-path", drv); + + ASSERT_THROW(drv.fillInOutputPaths(*store), Error); +} + +TEST_F(FillInOutputPathsTest, throwsOnIncorrectEnvVar) +{ + auto wrongPath = StorePath{"c015dhfh5l0lp6wxyvdn7bmwhbbr6hr9-wrong-name"}; + + Derivation drv; + drv.name = "bad-env-var"; + drv.platform = "x86_64-linux"; + drv.builder = "/bin/sh"; + drv.outputs = { + {"out", DerivationOutput{DerivationOutput::Deferred{}}}, + }; + drv.env = { + {"__doc", "Wrong env var value throws error"}, + {"out", store->printStorePath(wrongPath)}, + }; + + // Serialize before state + checkpointJson("bad-env-var", drv); + + ASSERT_THROW(drv.fillInOutputPaths(*store), Error); +} + +TEST_F(FillInOutputPathsTest, preservesDeferredWithInputDrvs) +{ + using nlohmann::json; + + // Create a CA floating dependency derivation + auto depDrvPath = makeCAFloatingDependency("dependency"); + + // Create a derivation that depends on the dependency + Derivation drv; + drv.name = "depends-on-drv"; + drv.platform = "x86_64-linux"; + drv.builder = "/bin/sh"; + drv.outputs = { + {"out", DerivationOutput{DerivationOutput::Deferred{}}}, + }; + drv.env = { + {"__doc", "Deferred stays deferred with CA dependencies"}, + {"out", ""}, + }; + // Add the real input derivation dependency + drv.inputDrvs = {.map = {{depDrvPath, {.value = {"out"}}}}}; + + // Serialize before state + checkpointJson("depends-on-drv-pre", drv); + + auto drvBefore = drv; + + // Apply fillInOutputPaths + drv.fillInOutputPaths(*store); + + // Derivation should be unchanged + EXPECT_EQ(drv, drvBefore); +} + +TEST_F(FillInOutputPathsTest, throwsOnPatWhenShouldBeDeffered) +{ + using nlohmann::json; + + // Create a CA floating dependency derivation + auto depDrvPath = makeCAFloatingDependency("dependency"); + + auto wrongPath = StorePath{"c015dhfh5l0lp6wxyvdn7bmwhbbr6hr9-wrong-name"}; + + // Create a derivation that depends on the dependency + Derivation drv; + drv.name = "depends-on-drv"; + drv.platform = "x86_64-linux"; + drv.builder = "/bin/sh"; + drv.outputs = { + {"out", DerivationOutput{DerivationOutput::InputAddressed{.path = wrongPath}}}, + }; + drv.env = { + {"__doc", "InputAddressed throws when should be deferred"}, + {"out", ""}, + }; + // Add the real input derivation dependency + drv.inputDrvs = {.map = {{depDrvPath, {.value = {"out"}}}}}; + + // Serialize before state + checkpointJson("bad-depends-on-drv-pre", drv); + + // Apply fillInOutputPaths + ASSERT_THROW(drv.fillInOutputPaths(*store), Error); +} + +} // namespace nix diff --git a/src/libstore-tests/derivation/test-support.hh b/src/libstore-tests/derivation/test-support.hh new file mode 100644 index 000000000..f48e6caef --- /dev/null +++ b/src/libstore-tests/derivation/test-support.hh @@ -0,0 +1,52 @@ +#pragma once + +#include + +#include "nix/util/experimental-features.hh" +#include "nix/store/tests/libstore.hh" +#include "nix/util/tests/characterization.hh" + +namespace nix { + +class DerivationTest : public virtual CharacterizationTest, public LibStoreTest +{ + std::filesystem::path unitTestData = getUnitTestData() / "derivation"; + +public: + std::filesystem::path goldenMaster(std::string_view testStem) const override + { + return unitTestData / testStem; + } + + /** + * We set these in tests rather than the regular globals so we don't have + * to worry about race conditions if the tests run concurrently. + */ + ExperimentalFeatureSettings mockXpSettings; +}; + +class CaDerivationTest : public DerivationTest +{ + void SetUp() override + { + mockXpSettings.set("experimental-features", "ca-derivations"); + } +}; + +class DynDerivationTest : public DerivationTest +{ + void SetUp() override + { + mockXpSettings.set("experimental-features", "dynamic-derivations ca-derivations"); + } +}; + +class ImpureDerivationTest : public DerivationTest +{ + void SetUp() override + { + mockXpSettings.set("experimental-features", "impure-derivations"); + } +}; + +} // namespace nix diff --git a/src/libstore-tests/meson.build b/src/libstore-tests/meson.build index f76df8bcb..58f624611 100644 --- a/src/libstore-tests/meson.build +++ b/src/libstore-tests/meson.build @@ -58,7 +58,8 @@ sources = files( 'common-protocol.cc', 'content-address.cc', 'derivation-advanced-attrs.cc', - 'derivation.cc', + 'derivation/external-formats.cc', + 'derivation/invariants.cc', 'derived-path.cc', 'downstream-placeholder.cc', 'dummy-store.cc', diff --git a/src/libstore/derivations.cc b/src/libstore/derivations.cc index f96109a6c..66e25732e 100644 --- a/src/libstore/derivations.cc +++ b/src/libstore/derivations.cc @@ -1210,6 +1210,136 @@ std::optional Derivation::tryResolve( return resolved; } +/** + * Process `InputAddressed`, `Deferred`, and `CAFixed` outputs. + * + * For `InputAddressed` outputs or `Deferred` outputs: + * + * - with `Regular` hash kind, validate `InputAddressed` outputs have + * the correct path (throws if mismatch). For `Deferred` outputs: + * - if `fillIn` is true, fill in the output path to make `InputAddressed` + * - if `fillIn` is false, throw an error + * Then validate or fill in the environment variable with the path. + * + * - with `Deferred` hash kind, validate that the output is either + * `InputAddressed` (error) or `Deferred` (correct). + * + * For `CAFixed` outputs, validate or fill in the environment variable + * with the computed path. + * + * @tparam fillIn If true, fill in missing output paths and environment + * variables. If false, validate that all paths are correct (throws on + * mismatch). + */ +template +static void processDerivationOutputPaths(Store & store, auto && drv, std::string_view drvName) +{ + std::optional hashesModulo; + + for (auto & [outputName, output] : drv.outputs) { + auto envHasRightPath = [&](const StorePath & actual) { + if constexpr (fillIn) { + auto j = drv.env.find(outputName); + /* Fill in mode: fill in missing or empty environment + variables */ + if (j == drv.env.end()) + drv.env.insert(j, {outputName, store.printStorePath(actual)}); + else if (j->second == "") + j->second = store.printStorePath(actual); + /* We know validation will succeed after fill-in, but + just to be extra sure, validate unconditionally */ + } + auto j = drv.env.find(outputName); + if (j == drv.env.end()) + throw Error( + "derivation has missing environment variable '%s', should be '%s' but is not present", + outputName, + store.printStorePath(actual)); + if (j->second != store.printStorePath(actual)) + throw Error( + "derivation has incorrect environment variable '%s', should be '%s' but is actually '%s'", + outputName, + store.printStorePath(actual), + j->second); + }; + auto hash = [&](const Output & outputVariant) { + if (!hashesModulo) { + // somewhat expensive so we do lazily + hashesModulo = hashDerivationModulo(store, drv, true); + } + switch (hashesModulo->kind) { + case DrvHash::Kind::Regular: { + auto h = get(hashesModulo->hashes, outputName); + if (!h) + throw Error("derivation produced no hash for output '%s'", outputName); + auto outPath = store.makeOutputPath(outputName, *h, drvName); + + if constexpr (std::is_same_v) { + if (outputVariant.path == outPath) { + return; // Correct case + } + /* Error case, an explicitly wrong path is + always an error. */ + throw Error( + "derivation has incorrect output '%s', should be '%s'", + store.printStorePath(outputVariant.path), + store.printStorePath(outPath)); + } else if constexpr (std::is_same_v) { + if constexpr (fillIn) + /* Fill in output path for Deferred + outputs */ + output = DerivationOutput::InputAddressed{ + .path = outPath, + }; + else + /* Validation mode: deferred outputs + should have been filled in */ + throw Error( + "derivation has incorrect deferred output, should be '%s'", store.printStorePath(outPath)); + } else { + /* Will never happen, based on where + `hash` is called. */ + static_assert(false); + } + envHasRightPath(outPath); + break; + } + case DrvHash::Kind::Deferred: + if constexpr (std::is_same_v) { + /* Error case, an explicitly wrong path is + always an error. */ + throw Error( + "derivation has incorrect output '%s', should be deferred", + store.printStorePath(outputVariant.path)); + } else if constexpr (std::is_same_v) { + /* Correct: Deferred output with Deferred + hash kind. */ + } else { + /* Will never happen, based on where + `hash` is called. */ + static_assert(false); + } + break; + } + }; + std::visit( + overloaded{ + [&](const DerivationOutput::InputAddressed & o) { hash(o); }, + [&](const DerivationOutput::Deferred & o) { hash(o); }, + [&](const DerivationOutput::CAFixed & dof) { envHasRightPath(dof.path(store, drvName, outputName)); }, + [&](const auto &) { + // Nothing to do for other output types + }, + }, + output.raw); + } + + /* Don't need the answer, but do this anyways to assert is proper + combination. The code above is more general and naturally allows + combinations that are currently prohibited. */ + drv.type(); +} + void Derivation::checkInvariants(Store & store, const StorePath & drvPath) const { assert(drvPath.isDerivation()); @@ -1217,67 +1347,43 @@ void Derivation::checkInvariants(Store & store, const StorePath & drvPath) const drvName = drvName.substr(0, drvName.size() - drvExtension.size()); if (drvName != name) { - throw Error("Derivation '%s' has name '%s' which does not match its path", store.printStorePath(drvPath), name); + throw Error("derivation '%s' has name '%s' which does not match its path", store.printStorePath(drvPath), name); } - auto envHasRightPath = [&](const StorePath & actual, const std::string & varName) { - auto j = env.find(varName); - if (j == env.end() || store.parseStorePath(j->second) != actual) - throw Error( - "derivation '%s' has incorrect environment variable '%s', should be '%s'", - store.printStorePath(drvPath), - varName, - store.printStorePath(actual)); - }; - - // Don't need the answer, but do this anyways to assert is proper - // combination. The code below is more general and naturally allows - // combinations that are currently prohibited. - type(); - - std::optional hashesModulo; - for (auto & i : outputs) { - std::visit( - overloaded{ - [&](const DerivationOutput::InputAddressed & doia) { - if (!hashesModulo) { - // somewhat expensive so we do lazily - hashesModulo = hashDerivationModulo(store, *this, true); - } - auto currentOutputHash = get(hashesModulo->hashes, i.first); - if (!currentOutputHash) - throw Error( - "derivation '%s' has unexpected output '%s' (local-store / hashesModulo) named '%s'", - store.printStorePath(drvPath), - store.printStorePath(doia.path), - i.first); - StorePath recomputed = store.makeOutputPath(i.first, *currentOutputHash, drvName); - if (doia.path != recomputed) - throw Error( - "derivation '%s' has incorrect output '%s', should be '%s'", - store.printStorePath(drvPath), - store.printStorePath(doia.path), - store.printStorePath(recomputed)); - envHasRightPath(doia.path, i.first); - }, - [&](const DerivationOutput::CAFixed & dof) { - auto path = dof.path(store, drvName, i.first); - envHasRightPath(path, i.first); - }, - [&](const DerivationOutput::CAFloating &) { - /* Nothing to check */ - }, - [&](const DerivationOutput::Deferred &) { - /* Nothing to check */ - }, - [&](const DerivationOutput::Impure &) { - /* Nothing to check */ - }, - }, - i.second.raw); + try { + checkInvariants(store); + } catch (Error & e) { + e.addTrace({}, "while checking derivation '%s'", store.printStorePath(drvPath)); + throw; } } +void Derivation::checkInvariants(Store & store) const +{ + processDerivationOutputPaths(store, *this, name); +} + +void Derivation::fillInOutputPaths(Store & store) +{ + processDerivationOutputPaths(store, *this, name); +} + +Derivation Derivation::parseJsonAndValidate(Store & store, const nlohmann::json & json) +{ + auto drv = static_cast(json); + + drv.fillInOutputPaths(store); + + try { + drv.checkInvariants(store); + } catch (Error & e) { + e.addTrace({}, "while checking derivation from JSON with name '%s'", drv.name); + throw; + } + + return drv; +} + const Hash impureOutputHash = hashString(HashAlgorithm::SHA256, "impure"); } // namespace nix diff --git a/src/libstore/include/nix/store/derivations.hh b/src/libstore/include/nix/store/derivations.hh index 259314d3f..aa33245e3 100644 --- a/src/libstore/include/nix/store/derivations.hh +++ b/src/libstore/include/nix/store/derivations.hh @@ -368,9 +368,48 @@ struct Derivation : BasicDerivation * This is mainly a matter of checking the outputs, where our C++ * representation supports all sorts of combinations we do not yet * allow. + * + * This overload does not validate the derivation name or add path + * context to errors. Use this when you don't have a `StorePath` or + * when you want to handle error context yourself. + * + * @param store The store to use for validation + */ + void checkInvariants(Store & store) const; + + /** + * This overload does everything the base `checkInvariants` does, + * but also validates that the derivation name matches the path, and + * improves any error messages that occur using the derivation path. + * + * @param store The store to use for validation + * @param drvPath The path to this derivation */ void checkInvariants(Store & store, const StorePath & drvPath) const; + /** + * Fill in output paths as needed. + * + * For input-addressed derivations (ready or deferred), it computes + * the derivation hash modulo and based on the result: + * + * - If `Regular`: converts `Deferred` outputs to `InputAddressed`, + * and ensures all `InputAddressed` outputs (whether preexisting + * or newly computed) have the right computed paths. Likewise + * defines (if absent or the empty string) or checks (if + * preexisting and non-empty) environment variables for each + * output with their path. + * + * - If `Deferred`: converts `InputAddressed` to `Deferred`. + * + * Also for fixed-output content-addressed derivations, likewise + * updates output paths in env vars. + * + * @param store The store to use for path computation + * @param drvName The derivation name (without .drv extension) + */ + void fillInOutputPaths(Store & store); + Derivation() = default; Derivation(const BasicDerivation & bd) @@ -383,6 +422,29 @@ struct Derivation : BasicDerivation { } + /** + * Parse a derivation from JSON, and also perform various + * conveniences such as: + * + * 1. Filling in output paths in as needed/required. + * + * 2. Checking invariants in general. + * + * In the future it might also do things like: + * + * - assist with the migration from older JSON formats. + * + * - (a somewhat example of the above) initialize + * `DerivationOptions` from their traditional encoding inside the + * `env` and `structuredAttrs`. + * + * @param store The store to use for path computation and validation + * @param json The JSON representation of the derivation + * @return A validated derivation with output paths filled in + * @throws Error if parsing fails, output paths can't be computed, or validation fails + */ + static Derivation parseJsonAndValidate(Store & store, const nlohmann::json & json); + bool operator==(const Derivation &) const = default; // TODO libc++ 16 (used by darwin) missing `std::map::operator <=>`, can't do yet. // auto operator <=> (const Derivation &) const = default; diff --git a/src/nix/derivation-add.cc b/src/nix/derivation-add.cc index 48e935092..bbaa87597 100644 --- a/src/nix/derivation-add.cc +++ b/src/nix/derivation-add.cc @@ -33,12 +33,10 @@ struct CmdAddDerivation : MixDryRun, StoreCommand { auto json = nlohmann::json::parse(drainFD(STDIN_FILENO)); - auto drv = static_cast(json); + auto drv = Derivation::parseJsonAndValidate(*store, json); auto drvPath = writeDerivation(*store, drv, NoRepair, /* read only */ dryRun); - drv.checkInvariants(*store, drvPath); - writeDerivation(*store, drv, NoRepair, dryRun); logger->cout("%s", store->printStorePath(drvPath)); diff --git a/tests/functional/derivation-json.sh b/tests/functional/derivation-json.sh index 06f934cfe..63c114403 100755 --- a/tests/functional/derivation-json.sh +++ b/tests/functional/derivation-json.sh @@ -4,11 +4,20 @@ source common.sh drvPath=$(nix-instantiate simple.nix) -nix derivation show "$drvPath" | jq .[] > "$TEST_HOME"/simple.json - -drvPath2=$(nix derivation add < "$TEST_HOME"/simple.json) +nix derivation show "$drvPath" | jq '.[]' > "$TEST_HOME/simple.json" +# Round tripping to JSON works +drvPath2=$(nix derivation add < "$TEST_HOME/simple.json") [[ "$drvPath" = "$drvPath2" ]] +# Derivation is input addressed, all outputs have a path +jq -e '.outputs | .[] | has("path")' < "$TEST_HOME/simple.json" + # Input addressed derivations cannot be renamed. -jq '.name = "foo"' < "$TEST_HOME"/simple.json | expectStderr 1 nix derivation add | grepQuiet "has incorrect output" +jq '.name = "foo"' < "$TEST_HOME/simple.json" | expectStderr 1 nix derivation add | grepQuiet "has incorrect output" + +# If we remove the input addressed to make it a deferred derivation, we +# still get the same result because Nix will see that need not be +# deferred and fill in the right input address for us. +drvPath3=$(jq '.outputs |= map_values(del(.path))' < "$TEST_HOME/simple.json" | nix derivation add) +[[ "$drvPath" = "$drvPath3" ]] From d689b764f3f18ceab4de16720d9b120cb41ebd77 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Sat, 22 Nov 2025 12:30:18 -0500 Subject: [PATCH 07/40] Use `WorkerProto::Serialise` abstraction for `DrvOutput` It's better to consistently use the abstraction, rather than code which happens to do the same thing. See also d782c5e5863cdcfd3fc8013c84efa1053b3d2e80 for the same sort of change. --- src/libstore/daemon.cc | 4 ++-- src/libstore/remote-store.cc | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/libstore/daemon.cc b/src/libstore/daemon.cc index 937946134..2a382dca7 100644 --- a/src/libstore/daemon.cc +++ b/src/libstore/daemon.cc @@ -964,7 +964,7 @@ static void performOp( case WorkerProto::Op::RegisterDrvOutput: { logger->startWork(); if (GET_PROTOCOL_MINOR(conn.protoVersion) < 31) { - auto outputId = DrvOutput::parse(readString(conn.from)); + auto outputId = WorkerProto::Serialise::read(*store, rconn); auto outputPath = StorePath(readString(conn.from)); store->registerDrvOutput(Realisation{{.outPath = outputPath}, outputId}); } else { @@ -977,7 +977,7 @@ static void performOp( case WorkerProto::Op::QueryRealisation: { logger->startWork(); - auto outputId = DrvOutput::parse(readString(conn.from)); + auto outputId = WorkerProto::Serialise::read(*store, rconn); auto info = store->queryRealisation(outputId); logger->stopWork(); if (GET_PROTOCOL_MINOR(conn.protoVersion) < 31) { diff --git a/src/libstore/remote-store.cc b/src/libstore/remote-store.cc index 949a51f18..b07efc024 100644 --- a/src/libstore/remote-store.cc +++ b/src/libstore/remote-store.cc @@ -493,7 +493,7 @@ void RemoteStore::registerDrvOutput(const Realisation & info) auto conn(getConnection()); conn->to << WorkerProto::Op::RegisterDrvOutput; if (GET_PROTOCOL_MINOR(conn->protoVersion) < 31) { - conn->to << info.id.to_string(); + WorkerProto::write(*this, *conn, info.id); conn->to << std::string(info.outPath.to_string()); } else { WorkerProto::write(*this, *conn, info); @@ -513,7 +513,7 @@ void RemoteStore::queryRealisationUncached( } conn->to << WorkerProto::Op::QueryRealisation; - conn->to << id.to_string(); + WorkerProto::write(*this, *conn, id); conn.processStderr(); auto real = [&]() -> std::shared_ptr { From 504c5e7cf9fcdff1f85a7d90a5d83b4f4d0b1a89 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Mon, 26 Aug 2024 17:37:52 -0400 Subject: [PATCH 08/40] Convert profiles to use `std::filesystem::path` Co-authored-by: Vinayak Goyal Co-authored-by: Eelco Dolstra --- src/libcmd/command.cc | 6 +- src/libexpr/eval-settings.cc | 12 +-- .../include/nix/store/tests/nix_api_store.hh | 3 +- .../build/derivation-building-goal.cc | 2 +- src/libstore/include/nix/store/pathlocks.hh | 12 +-- src/libstore/include/nix/store/profiles.hh | 44 +++++----- src/libstore/include/nix/store/store-api.hh | 6 ++ src/libstore/pathlocks.cc | 2 +- src/libstore/profiles.cc | 87 ++++++++++--------- src/libstore/store-api.cc | 5 ++ src/libstore/unix/pathlocks.cc | 8 +- src/libstore/windows/pathlocks.cc | 13 +-- src/libutil/current-process.cc | 2 +- src/libutil/file-system.cc | 15 +++- src/libutil/include/nix/util/file-system.hh | 9 +- src/libutil/include/nix/util/util.hh | 6 ++ .../nix-collect-garbage.cc | 2 +- src/nix/nix-env/nix-env.cc | 14 +-- src/nix/nix-env/user-env.cc | 4 +- src/nix/profile.cc | 5 +- 20 files changed, 154 insertions(+), 103 deletions(-) diff --git a/src/libcmd/command.cc b/src/libcmd/command.cc index b06d40902..798ef072e 100644 --- a/src/libcmd/command.cc +++ b/src/libcmd/command.cc @@ -297,7 +297,7 @@ void MixProfile::updateProfile(const BuiltPaths & buildables) MixDefaultProfile::MixDefaultProfile() { - profile = getDefaultProfile(); + profile = getDefaultProfile().string(); } MixEnvironment::MixEnvironment() @@ -391,7 +391,7 @@ void createOutLinks(const std::filesystem::path & outLink, const BuiltPaths & bu auto symlink = outLink; if (i) symlink += fmt("-%d", i); - store.addPermRoot(bo.path, absPath(symlink.string())); + store.addPermRoot(bo.path, absPath(symlink).string()); }, [&](const BuiltPath::Built & bfd) { for (auto & output : bfd.outputs) { @@ -400,7 +400,7 @@ void createOutLinks(const std::filesystem::path & outLink, const BuiltPaths & bu symlink += fmt("-%d", i); if (output.first != "out") symlink += fmt("-%s", output.first); - store.addPermRoot(output.second, absPath(symlink.string())); + store.addPermRoot(output.second, absPath(symlink).string()); } }, }, diff --git a/src/libexpr/eval-settings.cc b/src/libexpr/eval-settings.cc index 33c90259f..8e8b6bcd9 100644 --- a/src/libexpr/eval-settings.cc +++ b/src/libexpr/eval-settings.cc @@ -60,18 +60,18 @@ EvalSettings::EvalSettings(bool & readOnlyMode, EvalSettings::LookupPathHooks lo Strings EvalSettings::getDefaultNixPath() { Strings res; - auto add = [&](const Path & p, const std::string & s = std::string()) { + auto add = [&](const std::filesystem::path & p, const std::string & s = std::string()) { if (std::filesystem::exists(p)) { if (s.empty()) { - res.push_back(p); + res.push_back(p.string()); } else { - res.push_back(s + "=" + p); + res.push_back(s + "=" + p.string()); } } }; - add(getNixDefExpr() + "/channels"); - add(rootChannelsDir() + "/nixpkgs", "nixpkgs"); + add(std::filesystem::path{getNixDefExpr()} / "channels"); + add(rootChannelsDir() / "nixpkgs", "nixpkgs"); add(rootChannelsDir()); return res; @@ -108,4 +108,4 @@ Path getNixDefExpr() return settings.useXDGBaseDirectories ? getStateDir() + "/defexpr" : getHome() + "/.nix-defexpr"; } -} // namespace nix \ No newline at end of file +} // namespace nix diff --git a/src/libstore-test-support/include/nix/store/tests/nix_api_store.hh b/src/libstore-test-support/include/nix/store/tests/nix_api_store.hh index 7ecc5603b..829972d84 100644 --- a/src/libstore-test-support/include/nix/store/tests/nix_api_store.hh +++ b/src/libstore-test-support/include/nix/store/tests/nix_api_store.hh @@ -50,7 +50,8 @@ protected: #else // resolve any symlinks in i.e. on macOS /tmp -> /private/tmp // because this is not allowed for a nix store. - auto tmpl = nix::absPath(std::filesystem::path(nix::defaultTempDir()) / "tests_nix-store.XXXXXX", true); + auto tmpl = + nix::absPath(std::filesystem::path(nix::defaultTempDir()) / "tests_nix-store.XXXXXX", std::nullopt, true); nixDir = mkdtemp((char *) tmpl.c_str()); #endif diff --git a/src/libstore/build/derivation-building-goal.cc b/src/libstore/build/derivation-building-goal.cc index b4fc997a5..8221e12c6 100644 --- a/src/libstore/build/derivation-building-goal.cc +++ b/src/libstore/build/derivation-building-goal.cc @@ -307,7 +307,7 @@ Goal::Co DerivationBuildingGoal::tryToBuild() crashes. If we can't acquire the lock, then continue; hopefully some other goal can start a build, and if not, the main loop will sleep a few seconds and then retry this goal. */ - PathSet lockFiles; + std::set lockFiles; /* FIXME: Should lock something like the drv itself so we don't build same CA drv concurrently */ if (auto * localStore = dynamic_cast(&worker.store)) { diff --git a/src/libstore/include/nix/store/pathlocks.hh b/src/libstore/include/nix/store/pathlocks.hh index 05c7e079a..7e27bec4c 100644 --- a/src/libstore/include/nix/store/pathlocks.hh +++ b/src/libstore/include/nix/store/pathlocks.hh @@ -1,6 +1,8 @@ #pragma once ///@file +#include + #include "nix/util/file-descriptor.hh" namespace nix { @@ -10,12 +12,12 @@ namespace nix { * -1 is returned if create is false and the lock could not be opened * because it doesn't exist. Any other error throws an exception. */ -AutoCloseFD openLockFile(const Path & path, bool create); +AutoCloseFD openLockFile(const std::filesystem::path & path, bool create); /** * Delete an open lock file. */ -void deleteLockFile(const Path & path, Descriptor desc); +void deleteLockFile(const std::filesystem::path & path, Descriptor desc); enum LockType { ltRead, ltWrite, ltNone }; @@ -24,14 +26,14 @@ bool lockFile(Descriptor desc, LockType lockType, bool wait); class PathLocks { private: - typedef std::pair FDPair; + typedef std::pair FDPair; std::list fds; bool deletePaths; public: PathLocks(); - PathLocks(const PathSet & paths, const std::string & waitMsg = ""); - bool lockPaths(const PathSet & _paths, const std::string & waitMsg = "", bool wait = true); + PathLocks(const std::set & paths, const std::string & waitMsg = ""); + bool lockPaths(const std::set & _paths, const std::string & waitMsg = "", bool wait = true); ~PathLocks(); void unlock(); void setDeletion(bool deletePaths); diff --git a/src/libstore/include/nix/store/profiles.hh b/src/libstore/include/nix/store/profiles.hh index 75cd11340..1cc306744 100644 --- a/src/libstore/include/nix/store/profiles.hh +++ b/src/libstore/include/nix/store/profiles.hh @@ -7,12 +7,13 @@ * See the manual for additional information. */ -#include "nix/util/types.hh" -#include "nix/store/pathlocks.hh" - +#include #include #include +#include "nix/util/types.hh" +#include "nix/store/pathlocks.hh" + namespace nix { class StorePath; @@ -47,9 +48,9 @@ struct Generation * distinct contents to avoid bloat, but nothing stops two * non-adjacent generations from having the same contents. * - * @todo Use `StorePath` instead of `Path`? + * @todo Use `StorePath` instead of `std::filesystem::path`? */ - Path path; + std::filesystem::path path; /** * When the generation was created. This is extra metadata about the @@ -81,7 +82,7 @@ typedef std::list Generations; * * Note that the current/active generation need not be the latest one. */ -std::pair> findGenerations(Path profile); +std::pair> findGenerations(std::filesystem::path profile); struct LocalFSStore; @@ -96,7 +97,7 @@ struct LocalFSStore; * The behavior of reusing existing generations like this makes this * procedure idempotent. It also avoids clutter. */ -Path createGeneration(LocalFSStore & store, Path profile, StorePath outPath); +std::filesystem::path createGeneration(LocalFSStore & store, std::filesystem::path profile, StorePath outPath); /** * Unconditionally delete a generation @@ -111,7 +112,7 @@ Path createGeneration(LocalFSStore & store, Path profile, StorePath outPath); * * @todo Should we expose this at all? */ -void deleteGeneration(const Path & profile, GenerationNumber gen); +void deleteGeneration(const std::filesystem::path & profile, GenerationNumber gen); /** * Delete the given set of generations. @@ -128,7 +129,8 @@ void deleteGeneration(const Path & profile, GenerationNumber gen); * Trying to delete the currently active generation will fail, and cause * no generations to be deleted. */ -void deleteGenerations(const Path & profile, const std::set & gensToDelete, bool dryRun); +void deleteGenerations( + const std::filesystem::path & profile, const std::set & gensToDelete, bool dryRun); /** * Delete generations older than `max` passed the current generation. @@ -142,7 +144,7 @@ void deleteGenerations(const Path & profile, const std::set & * @param dryRun Log what would be deleted instead of actually doing * so. */ -void deleteGenerationsGreaterThan(const Path & profile, GenerationNumber max, bool dryRun); +void deleteGenerationsGreaterThan(const std::filesystem::path & profile, GenerationNumber max, bool dryRun); /** * Delete all generations other than the current one @@ -153,7 +155,7 @@ void deleteGenerationsGreaterThan(const Path & profile, GenerationNumber max, bo * @param dryRun Log what would be deleted instead of actually doing * so. */ -void deleteOldGenerations(const Path & profile, bool dryRun); +void deleteOldGenerations(const std::filesystem::path & profile, bool dryRun); /** * Delete generations older than `t`, except for the most recent one @@ -165,7 +167,7 @@ void deleteOldGenerations(const Path & profile, bool dryRun); * @param dryRun Log what would be deleted instead of actually doing * so. */ -void deleteGenerationsOlderThan(const Path & profile, time_t t, bool dryRun); +void deleteGenerationsOlderThan(const std::filesystem::path & profile, time_t t, bool dryRun); /** * Parse a temp spec intended for `deleteGenerationsOlderThan()`. @@ -180,19 +182,19 @@ time_t parseOlderThanTimeSpec(std::string_view timeSpec); * * @todo Always use `switchGeneration()` instead, and delete this. */ -void switchLink(Path link, Path target); +void switchLink(std::filesystem::path link, std::filesystem::path target); /** * Roll back a profile to the specified generation, or to the most * recent one older than the current. */ -void switchGeneration(const Path & profile, std::optional dstGen, bool dryRun); +void switchGeneration(const std::filesystem::path & profile, std::optional dstGen, bool dryRun); /** * Ensure exclusive access to a profile. Any command that modifies * the profile first acquires this lock. */ -void lockProfile(PathLocks & lock, const Path & profile); +void lockProfile(PathLocks & lock, const std::filesystem::path & profile); /** * Optimistic locking is used by long-running operations like `nix-env @@ -205,34 +207,34 @@ void lockProfile(PathLocks & lock, const Path & profile); * store. Most of the time, only the user environment has to be * rebuilt. */ -std::string optimisticLockProfile(const Path & profile); +std::string optimisticLockProfile(const std::filesystem::path & profile); /** * Create and return the path to a directory suitable for storing the user’s * profiles. */ -Path profilesDir(); +std::filesystem::path profilesDir(); /** * Return the path to the profile directory for root (but don't try creating it) */ -Path rootProfilesDir(); +std::filesystem::path rootProfilesDir(); /** * Create and return the path to the file used for storing the users's channels */ -Path defaultChannelsDir(); +std::filesystem::path defaultChannelsDir(); /** * Return the path to the channel directory for root (but don't try creating it) */ -Path rootChannelsDir(); +std::filesystem::path rootChannelsDir(); /** * Resolve the default profile (~/.nix-profile by default, * $XDG_STATE_HOME/nix/profile if XDG Base Directory Support is enabled), * and create if doesn't exist */ -Path getDefaultProfile(); +std::filesystem::path getDefaultProfile(); } // namespace nix diff --git a/src/libstore/include/nix/store/store-api.hh b/src/libstore/include/nix/store/store-api.hh index 4c0b156fa..9535227eb 100644 --- a/src/libstore/include/nix/store/store-api.hh +++ b/src/libstore/include/nix/store/store-api.hh @@ -996,6 +996,12 @@ OutputPathMap resolveDerivedPath(Store &, const DerivedPath::Built &, Store * ev */ std::string showPaths(const PathSet & paths); +/** + * Display a set of paths in human-readable form (i.e., between quotes + * and separated by commas). + */ +std::string showPaths(const std::set paths); + std::optional decodeValidPathInfo(const Store & store, std::istream & str, std::optional hashGiven = std::nullopt); diff --git a/src/libstore/pathlocks.cc b/src/libstore/pathlocks.cc index 068c65625..a8e828655 100644 --- a/src/libstore/pathlocks.cc +++ b/src/libstore/pathlocks.cc @@ -13,7 +13,7 @@ PathLocks::PathLocks() { } -PathLocks::PathLocks(const PathSet & paths, const std::string & waitMsg) +PathLocks::PathLocks(const std::set & paths, const std::string & waitMsg) : deletePaths(false) { lockPaths(paths, waitMsg); diff --git a/src/libstore/profiles.cc b/src/libstore/profiles.cc index 3f6fcb6ff..22d3f8f89 100644 --- a/src/libstore/profiles.cc +++ b/src/libstore/profiles.cc @@ -31,12 +31,12 @@ static std::optional parseName(const std::string & profileName return {}; } -std::pair> findGenerations(Path profile) +std::pair> findGenerations(std::filesystem::path profile) { Generations gens; - std::filesystem::path profileDir = dirOf(profile); - auto profileName = std::string(baseNameOf(profile)); + std::filesystem::path profileDir = profile.parent_path(); + auto profileName = profile.filename().string(); for (auto & i : DirectoryIterator{profileDir}) { checkInterrupt(); @@ -48,18 +48,20 @@ std::pair> findGenerations(Path pro gens.sort([](const Generation & a, const Generation & b) { return a.number < b.number; }); - return {gens, pathExists(profile) ? parseName(profileName, readLink(profile)) : std::nullopt}; + return {gens, pathExists(profile) ? parseName(profileName, readLink(profile).string()) : std::nullopt}; } /** * Create a generation name that can be parsed by `parseName()`. */ -static Path makeName(const Path & profile, GenerationNumber num) +static std::filesystem::path makeName(const std::filesystem::path & profile, GenerationNumber num) { - return fmt("%s-%s-link", profile, num); + /* NB std::filesystem::path when put in format strings is + quoted automatically. */ + return fmt("%s-%s-link", profile.string(), num); } -Path createGeneration(LocalFSStore & store, Path profile, StorePath outPath) +std::filesystem::path createGeneration(LocalFSStore & store, std::filesystem::path profile, StorePath outPath) { /* The new generation number should be higher than old the previous ones. */ @@ -90,21 +92,24 @@ Path createGeneration(LocalFSStore & store, Path profile, StorePath outPath) to the permanent roots (of which the GC would have a stale view). If we didn't do it this way, the GC might remove the user environment etc. we've just built. */ - Path generation = makeName(profile, num + 1); - store.addPermRoot(outPath, generation); + auto generation = makeName(profile, num + 1); + store.addPermRoot(outPath, generation.string()); return generation; } -static void removeFile(const Path & path) +static void removeFile(const std::filesystem::path & path) { - if (remove(path.c_str()) == -1) - throw SysError("cannot unlink '%1%'", path); + try { + std::filesystem::remove(path); + } catch (std::filesystem::filesystem_error & e) { + throw SysError("removing file '%1%'", path); + } } -void deleteGeneration(const Path & profile, GenerationNumber gen) +void deleteGeneration(const std::filesystem::path & profile, GenerationNumber gen) { - Path generation = makeName(profile, gen); + std::filesystem::path generation = makeName(profile, gen); removeFile(generation); } @@ -117,7 +122,7 @@ void deleteGeneration(const Path & profile, GenerationNumber gen) * * - We only actually delete if `dryRun` is false. */ -static void deleteGeneration2(const Path & profile, GenerationNumber gen, bool dryRun) +static void deleteGeneration2(const std::filesystem::path & profile, GenerationNumber gen, bool dryRun) { if (dryRun) notice("would remove profile version %1%", gen); @@ -127,7 +132,8 @@ static void deleteGeneration2(const Path & profile, GenerationNumber gen, bool d } } -void deleteGenerations(const Path & profile, const std::set & gensToDelete, bool dryRun) +void deleteGenerations( + const std::filesystem::path & profile, const std::set & gensToDelete, bool dryRun) { PathLocks lock; lockProfile(lock, profile); @@ -153,7 +159,7 @@ static inline void iterDropUntil(Generations & gens, auto && i, auto && cond) ; } -void deleteGenerationsGreaterThan(const Path & profile, GenerationNumber max, bool dryRun) +void deleteGenerationsGreaterThan(const std::filesystem::path & profile, GenerationNumber max, bool dryRun) { if (max == 0) throw Error("Must keep at least one generation, otherwise the current one would be deleted"); @@ -178,7 +184,7 @@ void deleteGenerationsGreaterThan(const Path & profile, GenerationNumber max, bo deleteGeneration2(profile, i->number, dryRun); } -void deleteOldGenerations(const Path & profile, bool dryRun) +void deleteOldGenerations(const std::filesystem::path & profile, bool dryRun) { PathLocks lock; lockProfile(lock, profile); @@ -190,7 +196,7 @@ void deleteOldGenerations(const Path & profile, bool dryRun) deleteGeneration2(profile, i.number, dryRun); } -void deleteGenerationsOlderThan(const Path & profile, time_t t, bool dryRun) +void deleteGenerationsOlderThan(const std::filesystem::path & profile, time_t t, bool dryRun) { PathLocks lock; lockProfile(lock, profile); @@ -238,16 +244,16 @@ time_t parseOlderThanTimeSpec(std::string_view timeSpec) return curTime - *days * 24 * 3600; } -void switchLink(Path link, Path target) +void switchLink(std::filesystem::path link, std::filesystem::path target) { /* Hacky. */ - if (dirOf(target) == dirOf(link)) - target = baseNameOf(target); + if (target.parent_path() == link.parent_path()) + target = target.filename(); replaceSymlink(target, link); } -void switchGeneration(const Path & profile, std::optional dstGen, bool dryRun) +void switchGeneration(const std::filesystem::path & profile, std::optional dstGen, bool dryRun) { PathLocks lock; lockProfile(lock, profile); @@ -274,44 +280,47 @@ void switchGeneration(const Path & profile, std::optional dstG switchLink(profile, dst->path); } -void lockProfile(PathLocks & lock, const Path & profile) +void lockProfile(PathLocks & lock, const std::filesystem::path & profile) { lock.lockPaths({profile}, fmt("waiting for lock on profile '%1%'", profile)); lock.setDeletion(true); } -std::string optimisticLockProfile(const Path & profile) +std::string optimisticLockProfile(const std::filesystem::path & profile) { - return pathExists(profile) ? readLink(profile) : ""; + return pathExists(profile) ? readLink(profile).string() : ""; } -Path profilesDir() +std::filesystem::path profilesDir() { - auto profileRoot = isRootUser() ? rootProfilesDir() : createNixStateDir() + "/profiles"; + auto profileRoot = isRootUser() ? rootProfilesDir() : std::filesystem::path{createNixStateDir()} / "profiles"; createDirs(profileRoot); return profileRoot; } -Path rootProfilesDir() +std::filesystem::path rootProfilesDir() { - return settings.nixStateDir + "/profiles/per-user/root"; + return std::filesystem::path{settings.nixStateDir} / "profiles/per-user/root"; } -Path getDefaultProfile() +std::filesystem::path getDefaultProfile() { - Path profileLink = settings.useXDGBaseDirectories ? createNixStateDir() + "/profile" : getHome() + "/.nix-profile"; + std::filesystem::path profileLink = settings.useXDGBaseDirectories + ? std::filesystem::path{createNixStateDir()} / "profile" + : std::filesystem::path{getHome()} / ".nix-profile"; try { - auto profile = profilesDir() + "/profile"; + auto profile = profilesDir() / "profile"; if (!pathExists(profileLink)) { replaceSymlink(profile, profileLink); } // Backwards compatibility measure: Make root's profile available as // `.../default` as it's what NixOS and most of the init scripts expect - Path globalProfileLink = settings.nixStateDir + "/profiles/default"; + auto globalProfileLink = std::filesystem::path{settings.nixStateDir} / "profiles" / "default"; if (isRootUser() && !pathExists(globalProfileLink)) { replaceSymlink(profile, globalProfileLink); } - return absPath(readLink(profileLink), dirOf(profileLink)); + auto linkDir = profileLink.parent_path(); + return absPath(readLink(profileLink), &linkDir); } catch (Error &) { return profileLink; } catch (std::filesystem::filesystem_error &) { @@ -319,14 +328,14 @@ Path getDefaultProfile() } } -Path defaultChannelsDir() +std::filesystem::path defaultChannelsDir() { - return profilesDir() + "/channels"; + return profilesDir() / "channels"; } -Path rootChannelsDir() +std::filesystem::path rootChannelsDir() { - return rootProfilesDir() + "/channels"; + return rootProfilesDir() / "channels"; } } // namespace nix diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index c292e2e43..52130668c 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -1126,6 +1126,11 @@ std::string StoreDirConfig::showPaths(const StorePathSet & paths) const return s; } +std::string showPaths(const std::set paths) +{ + return concatStringsSep(", ", quoteFSPaths(paths)); +} + std::string showPaths(const PathSet & paths) { return concatStringsSep(", ", quoteStrings(paths)); diff --git a/src/libstore/unix/pathlocks.cc b/src/libstore/unix/pathlocks.cc index e3f411a5d..fc1206012 100644 --- a/src/libstore/unix/pathlocks.cc +++ b/src/libstore/unix/pathlocks.cc @@ -13,7 +13,7 @@ namespace nix { -AutoCloseFD openLockFile(const Path & path, bool create) +AutoCloseFD openLockFile(const std::filesystem::path & path, bool create) { AutoCloseFD fd; @@ -24,7 +24,7 @@ AutoCloseFD openLockFile(const Path & path, bool create) return fd; } -void deleteLockFile(const Path & path, Descriptor desc) +void deleteLockFile(const std::filesystem::path & path, Descriptor desc) { /* Get rid of the lock file. Have to be careful not to introduce races. Write a (meaningless) token to the file to indicate to @@ -69,7 +69,7 @@ bool lockFile(Descriptor desc, LockType lockType, bool wait) return true; } -bool PathLocks::lockPaths(const PathSet & paths, const std::string & waitMsg, bool wait) +bool PathLocks::lockPaths(const std::set & paths, const std::string & waitMsg, bool wait) { assert(fds.empty()); @@ -81,7 +81,7 @@ bool PathLocks::lockPaths(const PathSet & paths, const std::string & waitMsg, bo preventing deadlocks. */ for (auto & path : paths) { checkInterrupt(); - Path lockPath = path + ".lock"; + std::filesystem::path lockPath = path + ".lock"; debug("locking path '%1%'", path); diff --git a/src/libstore/windows/pathlocks.cc b/src/libstore/windows/pathlocks.cc index c4e3a3d39..88fb4e22e 100644 --- a/src/libstore/windows/pathlocks.cc +++ b/src/libstore/windows/pathlocks.cc @@ -13,10 +13,10 @@ namespace nix { using namespace nix::windows; -void deleteLockFile(const Path & path, Descriptor desc) +void deleteLockFile(const std::filesystem::path & path, Descriptor desc) { - int exit = DeleteFileA(path.c_str()); + int exit = DeleteFileW(path.c_str()); if (exit == 0) warn("%s: &s", path, std::to_string(GetLastError())); } @@ -36,9 +36,9 @@ void PathLocks::unlock() fds.clear(); } -AutoCloseFD openLockFile(const Path & path, bool create) +AutoCloseFD openLockFile(const std::filesystem::path & path, bool create) { - AutoCloseFD desc = CreateFileA( + AutoCloseFD desc = CreateFileW( path.c_str(), GENERIC_READ | GENERIC_WRITE, FILE_SHARE_READ | FILE_SHARE_WRITE, @@ -103,13 +103,14 @@ bool lockFile(Descriptor desc, LockType lockType, bool wait) } } -bool PathLocks::lockPaths(const PathSet & paths, const std::string & waitMsg, bool wait) +bool PathLocks::lockPaths(const std::set & paths, const std::string & waitMsg, bool wait) { assert(fds.empty()); for (auto & path : paths) { checkInterrupt(); - Path lockPath = path + ".lock"; + std::filesystem::path lockPath = path; + lockPath += L".lock"; debug("locking path '%1%'", path); AutoCloseFD fd; diff --git a/src/libutil/current-process.cc b/src/libutil/current-process.cc index bc5700803..d2330339e 100644 --- a/src/libutil/current-process.cc +++ b/src/libutil/current-process.cc @@ -109,7 +109,7 @@ std::optional getSelfExe() { static auto cached = []() -> std::optional { #if defined(__linux__) || defined(__GNU__) - return readLink("/proc/self/exe"); + return readLink(std::filesystem::path{"/proc/self/exe"}); #elif defined(__APPLE__) char buf[1024]; uint32_t size = sizeof(buf); diff --git a/src/libutil/file-system.cc b/src/libutil/file-system.cc index fba92dc8e..758173f41 100644 --- a/src/libutil/file-system.cc +++ b/src/libutil/file-system.cc @@ -101,9 +101,11 @@ Path absPath(PathView path, std::optional dir, bool resolveSymlinks) return canonPath(path, resolveSymlinks); } -std::filesystem::path absPath(const std::filesystem::path & path, bool resolveSymlinks) +std::filesystem::path +absPath(const std::filesystem::path & path, const std::filesystem::path * dir_, bool resolveSymlinks) { - return absPath(path.string(), std::nullopt, resolveSymlinks); + std::optional dir = dir_ ? std::optional{dir_->string()} : std::nullopt; + return absPath(PathView{path.string()}, dir.transform([](auto & p) { return PathView(p); }), resolveSymlinks); } Path canonPath(PathView path, bool resolveSymlinks) @@ -242,10 +244,15 @@ bool pathAccessible(const std::filesystem::path & path) } } -Path readLink(const Path & path) +std::filesystem::path readLink(const std::filesystem::path & path) { checkInterrupt(); - return std::filesystem::read_symlink(path).string(); + return std::filesystem::read_symlink(path); +} + +Path readLink(const Path & path) +{ + return readLink(std::filesystem::path{path}).string(); } std::string readFile(const Path & path) diff --git a/src/libutil/include/nix/util/file-system.hh b/src/libutil/include/nix/util/file-system.hh index 98b992472..a203af2aa 100644 --- a/src/libutil/include/nix/util/file-system.hh +++ b/src/libutil/include/nix/util/file-system.hh @@ -55,7 +55,8 @@ inline Path absPath(const Path & path, std::optional dir = {}, bool re return absPath(PathView{path}, dir, resolveSymlinks); } -std::filesystem::path absPath(const std::filesystem::path & path, bool resolveSymlinks = false); +std::filesystem::path +absPath(const std::filesystem::path & path, const std::filesystem::path * dir = nullptr, bool resolveSymlinks = false); /** * Canonicalise a path by removing all `.` or `..` components and @@ -152,6 +153,12 @@ bool pathAccessible(const std::filesystem::path & path); */ Path readLink(const Path & path); +/** + * Read the contents (target) of a symbolic link. The result is not + * in any way canonicalised. + */ +std::filesystem::path readLink(const std::filesystem::path & path); + /** * Open a `Descriptor` with read-only access to the given directory. */ diff --git a/src/libutil/include/nix/util/util.hh b/src/libutil/include/nix/util/util.hh index 2a379fcd3..83419c8c9 100644 --- a/src/libutil/include/nix/util/util.hh +++ b/src/libutil/include/nix/util/util.hh @@ -58,6 +58,12 @@ Strings quoteStrings(const C & c, char quote = '\'') return res; } +inline Strings quoteFSPaths(const std::set & paths, char quote = '\'') +{ + return paths | std::views::transform([&](const auto & p) { return quoteString(p.string(), quote); }) + | std::ranges::to(); +} + /** * Remove trailing whitespace from a string. * diff --git a/src/nix/nix-collect-garbage/nix-collect-garbage.cc b/src/nix/nix-collect-garbage/nix-collect-garbage.cc index 4d6e60bf3..29ca17a5d 100644 --- a/src/nix/nix-collect-garbage/nix-collect-garbage.cc +++ b/src/nix/nix-collect-garbage/nix-collect-garbage.cc @@ -91,7 +91,7 @@ static int main_nix_collect_garbage(int argc, char ** argv) std::set dirsToClean = { profilesDir(), std::filesystem::path{settings.nixStateDir} / "profiles", - std::filesystem::path{getDefaultProfile()}.parent_path(), + getDefaultProfile().parent_path(), }; for (auto & dir : dirsToClean) removeOldGenerations(dir); diff --git a/src/nix/nix-env/nix-env.cc b/src/nix/nix-env/nix-env.cc index 54443a22c..edfccffa6 100644 --- a/src/nix/nix-env/nix-env.cc +++ b/src/nix/nix-env/nix-env.cc @@ -761,7 +761,7 @@ static void opSet(Globals & globals, Strings opFlags, Strings opArgs) globals.state->store->buildPaths(paths, globals.state->repair ? bmRepair : bmNormal); debug("switching to new user environment"); - Path generation = createGeneration(*store2, globals.profile, drv.queryOutPath()); + auto generation = createGeneration(*store2, globals.profile, drv.queryOutPath()); switchLink(globals.profile, generation); } @@ -1407,14 +1407,15 @@ static int main_nix_env(int argc, char ** argv) globals.instSource.type = srcUnknown; globals.instSource.systemFilter = "*"; - Path nixExprPath = getNixDefExpr(); + std::filesystem::path nixExprPath = getNixDefExpr(); if (!pathExists(nixExprPath)) { try { createDirs(nixExprPath); - replaceSymlink(defaultChannelsDir(), nixExprPath + "/channels"); + replaceSymlink(defaultChannelsDir(), nixExprPath / "channels"); if (!isRootUser()) - replaceSymlink(rootChannelsDir(), nixExprPath + "/channels_root"); + replaceSymlink(rootChannelsDir(), nixExprPath / "channels_root"); + } catch (std::filesystem::filesystem_error &) { } catch (Error &) { } } @@ -1511,7 +1512,8 @@ static int main_nix_env(int argc, char ** argv) globals.state->repair = myArgs.repair; globals.instSource.nixExprPath = std::make_shared( - file != "" ? lookupFileArg(*globals.state, file) : globals.state->rootPath(CanonPath(nixExprPath))); + file != "" ? lookupFileArg(*globals.state, file) + : globals.state->rootPath(CanonPath(nixExprPath.string()))); globals.instSource.autoArgs = myArgs.getAutoArgs(*globals.state); @@ -1519,7 +1521,7 @@ static int main_nix_env(int argc, char ** argv) globals.profile = getEnv("NIX_PROFILE").value_or(""); if (globals.profile == "") - globals.profile = getDefaultProfile(); + globals.profile = getDefaultProfile().string(); op(globals, std::move(opFlags), std::move(opArgs)); diff --git a/src/nix/nix-env/user-env.cc b/src/nix/nix-env/user-env.cc index d09365073..5beed78f7 100644 --- a/src/nix/nix-env/user-env.cc +++ b/src/nix/nix-env/user-env.cc @@ -161,14 +161,14 @@ bool createUserEnv( PathLocks lock; lockProfile(lock, profile); - Path lockTokenCur = optimisticLockProfile(profile); + std::filesystem::path lockTokenCur = optimisticLockProfile(profile); if (lockToken != lockTokenCur) { printInfo("profile '%1%' changed while we were busy; restarting", profile); return false; } debug("switching to new user environment"); - Path generation = createGeneration(*store2, profile, topLevelOut); + std::filesystem::path generation = createGeneration(*store2, profile, topLevelOut); switchLink(profile, generation); } diff --git a/src/nix/profile.cc b/src/nix/profile.cc index 9690eacd8..5b0e033e0 100644 --- a/src/nix/profile.cc +++ b/src/nix/profile.cc @@ -849,7 +849,10 @@ struct CmdProfileDiffClosures : virtual StoreCommand, MixDefaultProfile first = false; logger->cout("Version %d -> %d:", prevGen->number, gen.number); printClosureDiff( - store, store->followLinksToStorePath(prevGen->path), store->followLinksToStorePath(gen.path), " "); + store, + store->followLinksToStorePath(prevGen->path.string()), + store->followLinksToStorePath(gen.path.string()), + " "); } prevGen = gen; From 43a183120a2bb6d56d15cecf237f5882466cd6c7 Mon Sep 17 00:00:00 2001 From: Taeer Bar-Yam Date: Mon, 24 Nov 2025 20:23:10 +0100 Subject: [PATCH 09/40] libexpr: factor out functions for moving data to a new allocator --- src/libexpr/include/nix/expr/nixexpr.hh | 2 ++ src/libexpr/nixexpr.cc | 34 +++++++++++++------------ 2 files changed, 20 insertions(+), 16 deletions(-) diff --git a/src/libexpr/include/nix/expr/nixexpr.hh b/src/libexpr/include/nix/expr/nixexpr.hh index e4880a3fb..c7bfb7359 100644 --- a/src/libexpr/include/nix/expr/nixexpr.hh +++ b/src/libexpr/include/nix/expr/nixexpr.hh @@ -438,6 +438,7 @@ struct ExprAttrs : Expr std::shared_ptr bindInheritSources(EvalState & es, const std::shared_ptr & env); Env * buildInheritFromEnv(EvalState & state, Env & up); void showBindings(const SymbolTable & symbols, std::ostream & str) const; + void moveDataToAllocator(std::pmr::polymorphic_allocator & alloc); }; struct ExprList : Expr @@ -622,6 +623,7 @@ struct ExprCall : Expr virtual void resetCursedOr() override; virtual void warnIfCursedOr(const SymbolTable & symbols, const PosTable & positions) override; + void moveDataToAllocator(std::pmr::polymorphic_allocator & alloc); COMMON_METHODS }; diff --git a/src/libexpr/nixexpr.cc b/src/libexpr/nixexpr.cc index 37e22c466..6c9aa26dd 100644 --- a/src/libexpr/nixexpr.cc +++ b/src/libexpr/nixexpr.cc @@ -399,18 +399,19 @@ ExprAttrs::bindInheritSources(EvalState & es, const std::shared_ptr & alloc) +{ + AttrDefs newAttrs{std::move(*attrs), alloc}; + attrs.emplace(std::move(newAttrs), alloc); + DynamicAttrDefs newDynamicAttrs{std::move(*dynamicAttrs), alloc}; + dynamicAttrs.emplace(std::move(newDynamicAttrs), alloc); + if (inheritFromExprs) + inheritFromExprs = std::make_unique>(std::move(*inheritFromExprs), alloc); +} + void ExprAttrs::bindVars(EvalState & es, const std::shared_ptr & env) { - // Move storage into the Exprs arena - { - auto arena = es.mem.exprs.alloc; - AttrDefs newAttrs{std::move(*attrs), arena}; - attrs.emplace(std::move(newAttrs), arena); - DynamicAttrDefs newDynamicAttrs{std::move(*dynamicAttrs), arena}; - dynamicAttrs.emplace(std::move(newDynamicAttrs), arena); - if (inheritFromExprs) - inheritFromExprs = std::make_unique>(std::move(*inheritFromExprs), arena); - } + moveDataToAllocator(es.mem.exprs.alloc); if (es.debugRepl) es.exprEnvs.insert(std::make_pair(this, env)); @@ -484,14 +485,15 @@ void ExprLambda::bindVars(EvalState & es, const std::shared_ptr body->bindVars(es, newEnv); } +void ExprCall::moveDataToAllocator(std::pmr::polymorphic_allocator & alloc) +{ + std::pmr::vector newArgs{std::move(*args), alloc}; + args.emplace(std::move(newArgs), alloc); +} + void ExprCall::bindVars(EvalState & es, const std::shared_ptr & env) { - // Move storage into the Exprs arena - { - auto arena = es.mem.exprs.alloc; - std::pmr::vector newArgs{std::move(*args), arena}; - args.emplace(std::move(newArgs), arena); - } + moveDataToAllocator(es.mem.exprs.alloc); if (es.debugRepl) es.exprEnvs.insert(std::make_pair(this, env)); From 60f09928d1fb8cb6c5e02b66616b170407d511f8 Mon Sep 17 00:00:00 2001 From: Taeer Bar-Yam Date: Mon, 24 Nov 2025 20:25:20 +0100 Subject: [PATCH 10/40] libexpr: move ExprLet::attrs data to arena as well I missed this because I assumed all Exprs were recursed into by bindVars, but ExprLet's ExprAttrs field is not really its own AST node, so it doesn't get recursed into. --- src/libexpr/nixexpr.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/src/libexpr/nixexpr.cc b/src/libexpr/nixexpr.cc index 6c9aa26dd..fc8c71b07 100644 --- a/src/libexpr/nixexpr.cc +++ b/src/libexpr/nixexpr.cc @@ -504,6 +504,7 @@ void ExprCall::bindVars(EvalState & es, const std::shared_ptr & void ExprLet::bindVars(EvalState & es, const std::shared_ptr & env) { + attrs->moveDataToAllocator(es.mem.exprs.alloc); auto newEnv = [&]() -> std::shared_ptr { auto newEnv = std::make_shared(nullptr, env, attrs->attrs->size()); From eb53e61e086efe929887be79f21d4e12832edb13 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Mon, 24 Nov 2025 15:21:22 -0500 Subject: [PATCH 11/40] Fix stray derivation "v3" in manual It's commented out, but we should still update it to "v4" to match the link target. --- doc/manual/source/protocols/json/derivation.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/manual/source/protocols/json/derivation.md b/doc/manual/source/protocols/json/derivation.md index 6eafb255e..13092773f 100644 --- a/doc/manual/source/protocols/json/derivation.md +++ b/doc/manual/source/protocols/json/derivation.md @@ -3,5 +3,5 @@ From b8d32388bc99fd6d537313bdcf2629c20cc87c54 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Mon, 24 Nov 2025 15:20:19 -0500 Subject: [PATCH 12/40] Move derivation JSON doc to `index.md` in dir This prepares for more structure. --- doc/manual/source/SUMMARY.md.in | 2 +- .../protocols/json/{derivation.md => derivation/index.md} | 2 +- .../protocols/json/fixup-json-schema-generated-doc.sed | 8 ++++---- doc/manual/source/release-notes/rl-2.32.md | 2 +- doc/manual/source/store/derivation/index.md | 2 +- src/nix/derivation-add.md | 2 +- src/nix/derivation-show.md | 2 +- 7 files changed, 10 insertions(+), 10 deletions(-) rename doc/manual/source/protocols/json/{derivation.md => derivation/index.md} (75%) diff --git a/doc/manual/source/SUMMARY.md.in b/doc/manual/source/SUMMARY.md.in index 806489bb3..0d32e3628 100644 --- a/doc/manual/source/SUMMARY.md.in +++ b/doc/manual/source/SUMMARY.md.in @@ -126,7 +126,7 @@ - [Content Address](protocols/json/content-address.md) - [Store Path](protocols/json/store-path.md) - [Store Object Info](protocols/json/store-object-info.md) - - [Derivation](protocols/json/derivation.md) + - [Derivation](protocols/json/derivation/index.md) - [Deriving Path](protocols/json/deriving-path.md) - [Build Trace Entry](protocols/json/build-trace-entry.md) - [Build Result](protocols/json/build-result.md) diff --git a/doc/manual/source/protocols/json/derivation.md b/doc/manual/source/protocols/json/derivation/index.md similarity index 75% rename from doc/manual/source/protocols/json/derivation.md rename to doc/manual/source/protocols/json/derivation/index.md index 13092773f..0b15acb8f 100644 --- a/doc/manual/source/protocols/json/derivation.md +++ b/doc/manual/source/protocols/json/derivation/index.md @@ -1,4 +1,4 @@ -{{#include derivation-v4-fixed.md}} +{{#include ../derivation-v4-fixed.md}} diff --git a/doc/manual/source/protocols/json/meson.build b/doc/manual/source/protocols/json/meson.build index 72856a47a..8780d8057 100644 --- a/doc/manual/source/protocols/json/meson.build +++ b/doc/manual/source/protocols/json/meson.build @@ -15,6 +15,7 @@ schemas = [ 'store-path-v1', 'store-object-info-v2', 'derivation-v4', + 'derivation-options-v1', 'deriving-path-v1', 'build-trace-entry-v1', 'build-result-v1', diff --git a/doc/manual/source/protocols/json/schema/derivation-options-v1 b/doc/manual/source/protocols/json/schema/derivation-options-v1 new file mode 120000 index 000000000..9332a5390 --- /dev/null +++ b/doc/manual/source/protocols/json/schema/derivation-options-v1 @@ -0,0 +1 @@ +../../../../../../src/libstore-tests/data/derivation \ No newline at end of file diff --git a/doc/manual/source/protocols/json/schema/derivation-options-v1.yaml b/doc/manual/source/protocols/json/schema/derivation-options-v1.yaml new file mode 100644 index 000000000..58ff07088 --- /dev/null +++ b/doc/manual/source/protocols/json/schema/derivation-options-v1.yaml @@ -0,0 +1,242 @@ +"$schema": "http://json-schema.org/draft-04/schema" +"$id": "https://nix.dev/manual/nix/latest/protocols/json/schema/derivation-options-v1.json" +title: Derivation Options +description: | + JSON representation of Nix's `DerivationOptions` type. + + This schema describes various build-time options and constraints that can be specified for a derivation. + + > **Warning** + > + > This JSON format is currently + > [**experimental**](@docroot@/development/experimental-features.md#xp-feature-nix-command) + > and subject to change. + +type: object +required: + - outputChecks + - unsafeDiscardReferences + - passAsFile + - exportReferencesGraph + - additionalSandboxProfile + - noChroot + - impureHostDeps + - impureEnvVars + - allowLocalNetworking + - requiredSystemFeatures + - preferLocalBuild + - allowSubstitutes +properties: + outputChecks: + type: object + title: Output Check + description: | + Constraints on what the derivation's outputs can and cannot reference. + Can either apply to all outputs or be specified per output. + oneOf: + - title: Output Checks For All Outputs + description: | + Output checks that apply to all outputs of the derivation. + required: + - forAllOutputs + properties: + forAllOutputs: + "$ref": "#/$defs/outputCheckSpec" + additionalProperties: false + + - title: Output Checks Per Output + description: | + Output checks specified individually for each output. + required: + - perOutput + properties: + perOutput: + type: object + additionalProperties: + "$ref": "#/$defs/outputCheckSpec" + additionalProperties: false + + unsafeDiscardReferences: + type: object + title: Unsafe Discard References + description: | + A map specifying which references should be unsafely discarded from each output. + This is generally not recommended and requires special permissions. + additionalProperties: + type: array + items: + type: string + + passAsFile: + type: array + title: Pass As File + description: | + List of environment variable names whose values should be passed as files rather than directly. + items: + type: string + + exportReferencesGraph: + type: object + title: Export References Graph + description: | + Specify paths whose references graph should be exported to files. + additionalProperties: + type: array + items: + "$ref": "deriving-path-v1.yaml" + + additionalSandboxProfile: + type: string + title: Additional Sandbox Profile + description: | + Additional sandbox profile directives (macOS specific). + + noChroot: + type: boolean + title: No Chroot + description: | + Whether to disable the build sandbox, if allowed. + + impureHostDeps: + type: array + title: Impure Host Dependencies + description: | + List of host paths that the build can access. + items: + type: string + + impureEnvVars: + type: array + title: Impure Environment Variables + description: | + List of environment variable names that should be passed through to the build from the calling environment. + items: + type: string + + allowLocalNetworking: + type: boolean + title: Allow Local Networking + description: | + Whether the build should have access to local network (macOS specific). + + requiredSystemFeatures: + type: array + title: Required System Features + description: | + List of system features required to build this derivation (e.g., "kvm", "nixos-test"). + items: + type: string + + preferLocalBuild: + type: boolean + title: Prefer Local Build + description: | + Whether this derivation should preferably be built locally rather than its outputs substituted. + + allowSubstitutes: + type: boolean + title: Allow Substitutes + description: | + Whether substituting from other stores should be allowed for this derivation's outputs. + +additionalProperties: false + +$defs: + + outputCheckSpec: + type: object + title: Output Check Specification + description: | + Constraints on what a specific output can reference. + required: + - ignoreSelfRefs + - maxSize + - maxClosureSize + - allowedReferences + - allowedRequisites + - disallowedReferences + - disallowedRequisites + properties: + ignoreSelfRefs: + type: boolean + title: Ignore Self References + description: | + Whether references from this output to itself should be ignored when checking references. + + maxSize: + type: ["integer", "null"] + title: Maximum Size + description: | + Maximum allowed size of this output in bytes, or null for no limit. + minimum: 0 + + maxClosureSize: + type: ["integer", "null"] + title: Maximum Closure Size + description: | + Maximum allowed size of this output's closure in bytes, or null for no limit. + minimum: 0 + + allowedReferences: + oneOf: + - type: array + items: + "$ref": "#/$defs/drvRef" + - type: "null" + title: Allowed References + description: | + If set, the output can only reference paths in this list. + If null, no restrictions apply. + + allowedRequisites: + oneOf: + - type: array + items: + "$ref": "#/$defs/drvRef" + - type: "null" + title: Allowed Requisites + description: | + If set, the output's closure can only contain paths in this list. + If null, no restrictions apply. + + disallowedReferences: + type: array + title: Disallowed References + description: | + The output must not reference any paths in this list. + items: + "$ref": "#/$defs/drvRef" + + disallowedRequisites: + type: array + title: Disallowed Requisites + description: | + The output's closure must not contain any paths in this list. + items: + "$ref": "#/$defs/drvRef" + additionalProperties: false + + drvRef: + # TODO fix bug in checker, should be `oneOf` + anyOf: + - type: object + title: Current derivation Output Reference + description: | + A reference to a specific output of the current derivation. + required: + - drvPath + - output + properties: + drvPath: + type: string + const: "self" + title: This derivation + description: | + Won't be confused for a deriving path + output: + type: string + title: Output Name + description: | + The name of the output being referenced. + additionalProperties: false + - "$ref": "deriving-path-v1.yaml" diff --git a/src/json-schema-checks/derivation-options b/src/json-schema-checks/derivation-options new file mode 120000 index 000000000..00c6cde65 --- /dev/null +++ b/src/json-schema-checks/derivation-options @@ -0,0 +1 @@ +../libstore-tests/data/derivation \ No newline at end of file diff --git a/src/json-schema-checks/meson.build b/src/json-schema-checks/meson.build index b1a829d38..73be4a47d 100644 --- a/src/json-schema-checks/meson.build +++ b/src/json-schema-checks/meson.build @@ -71,6 +71,18 @@ schemas = [ 'with-signature.json', ], }, + { + 'stem' : 'derivation-options', + 'schema' : schema_dir / 'derivation-options-v1.yaml', + 'files' : [ + 'ia' / 'defaults.json', + 'ia' / 'all_set.json', + 'ia' / 'structuredAttrs_defaults.json', + 'ia' / 'structuredAttrs_all_set.json', + 'ca' / 'all_set.json', + 'ca' / 'structuredAttrs_all_set.json', + ], + }, ] # Derivation and Derivation output From 77990e7cca34f8814f30dbe5dc2fff412323bd65 Mon Sep 17 00:00:00 2001 From: Sergei Zimmerman Date: Mon, 24 Nov 2025 23:14:47 +0300 Subject: [PATCH 14/40] libutil/file-descriptor: Add safer utilities for opening files relative to dirFd Implements a safe no symlink following primitive operation for opening file descriptors. This is unix-only for the time being, since windows doesn't really suffer from symlink races, since they are admin-only. Tested with enosys --syscall openat2 as well. --- src/libutil-tests/file-system.cc | 50 ++++++++ .../include/nix/util/file-descriptor.hh | 58 ++++++++++ src/libutil/unix/file-descriptor.cc | 109 ++++++++++++++++++ 3 files changed, 217 insertions(+) diff --git a/src/libutil-tests/file-system.cc b/src/libutil-tests/file-system.cc index dfdd26088..b85d2613d 100644 --- a/src/libutil-tests/file-system.cc +++ b/src/libutil-tests/file-system.cc @@ -1,3 +1,4 @@ +#include "nix/util/fs-sink.hh" #include "nix/util/util.hh" #include "nix/util/types.hh" #include "nix/util/file-system.hh" @@ -318,4 +319,53 @@ TEST(DirectoryIterator, nonexistent) ASSERT_THROW(DirectoryIterator("/schnitzel/darmstadt/pommes"), SysError); } +/* ---------------------------------------------------------------------------- + * openFileEnsureBeneathNoSymlinks + * --------------------------------------------------------------------------*/ + +#ifndef _WIN32 + +TEST(openFileEnsureBeneathNoSymlinks, works) +{ + std::filesystem::path tmpDir = nix::createTempDir(); + nix::AutoDelete delTmpDir(tmpDir, /*recursive=*/true); + + { + RestoreSink sink(/*startFsync=*/false); + sink.dstPath = tmpDir; + sink.dirFd = openDirectory(tmpDir); + sink.createDirectory(CanonPath("a")); + sink.createDirectory(CanonPath("c")); + sink.createDirectory(CanonPath("c/d")); + sink.createRegularFile(CanonPath("c/d/regular"), [](CreateRegularFileSink & crf) { crf("some contents"); }); + sink.createSymlink(CanonPath("a/absolute_symlink"), tmpDir.string()); + sink.createSymlink(CanonPath("a/relative_symlink"), "../."); + sink.createSymlink(CanonPath("a/broken_symlink"), "./nonexistent"); + } + + AutoCloseFD dirFd = openDirectory(tmpDir); + + using namespace nix::unix; + + auto open = [&](std::string_view path, int flags, mode_t mode = 0) { + return openFileEnsureBeneathNoSymlinks(dirFd.get(), CanonPath(path), flags, mode); + }; + + EXPECT_THROW(open("a/absolute_symlink", O_RDONLY), SymlinkNotAllowed); + EXPECT_THROW(open("a/relative_symlink", O_RDONLY), SymlinkNotAllowed); + EXPECT_THROW(open("a/absolute_symlink/a", O_RDONLY), SymlinkNotAllowed); + EXPECT_THROW(open("a/absolute_symlink/c/d", O_RDONLY), SymlinkNotAllowed); + EXPECT_THROW(open("a/relative_symlink/c", O_RDONLY), SymlinkNotAllowed); + EXPECT_EQ(open("a/broken_symlink", O_CREAT | O_WRONLY | O_EXCL, 0666), INVALID_DESCRIPTOR); + /* Sanity check, no symlink shenanigans and behaves the same as regular openat with O_EXCL | O_CREAT. */ + EXPECT_EQ(errno, EEXIST); + EXPECT_THROW(open("a/absolute_symlink/broken_symlink", O_CREAT | O_WRONLY | O_EXCL, 0666), SymlinkNotAllowed); + EXPECT_EQ(open("c/d/regular/a", O_RDONLY), INVALID_DESCRIPTOR); + EXPECT_EQ(open("c/d/regular", O_RDONLY | O_DIRECTORY), INVALID_DESCRIPTOR); + EXPECT_TRUE(AutoCloseFD{open("c/d/regular", O_RDONLY)}); + EXPECT_TRUE(AutoCloseFD{open("a/regular", O_CREAT | O_WRONLY | O_EXCL, 0666)}); +} + +#endif + } // namespace nix diff --git a/src/libutil/include/nix/util/file-descriptor.hh b/src/libutil/include/nix/util/file-descriptor.hh index 3dd2dd8e6..4a6fdc8a2 100644 --- a/src/libutil/include/nix/util/file-descriptor.hh +++ b/src/libutil/include/nix/util/file-descriptor.hh @@ -1,6 +1,7 @@ #pragma once ///@file +#include "nix/util/canon-path.hh" #include "nix/util/types.hh" #include "nix/util/error.hh" @@ -203,6 +204,26 @@ void closeOnExec(Descriptor fd); } // namespace unix #endif +#ifdef __linux__ +namespace linux { + +/** + * Wrapper around Linux's openat2 syscall introduced in Linux 5.6. + * + * @see https://man7.org/linux/man-pages/man2/openat2.2.html + * @see https://man7.org/linux/man-pages/man2/open_how.2type.html +v* + * @param flags O_* flags + * @param mode Mode for O_{CREAT,TMPFILE} + * @param resolve RESOLVE_* flags + * + * @return nullopt if openat2 is not supported by the kernel. + */ +std::optional openat2(Descriptor dirFd, const char * path, uint64_t flags, uint64_t mode, uint64_t resolve); + +} // namespace linux +#endif + #if defined(_WIN32) && _WIN32_WINNT >= 0x0600 namespace windows { @@ -212,6 +233,43 @@ std::wstring handleToFileName(Descriptor handle); } // namespace windows #endif +#ifndef _WIN32 +namespace unix { + +struct SymlinkNotAllowed : public Error +{ + CanonPath path; + + SymlinkNotAllowed(CanonPath path) + /* Can't provide better error message, since the parent directory is only known to the caller. */ + : Error("relative path '%s' points to a symlink, which is not allowed", path.rel()) + , path(std::move(path)) + { + } +}; + +/** + * Safe(r) function to open \param path file relative to \param dirFd, while + * disallowing escaping from a directory and resolving any symlinks in the + * process. + * + * @note When not on Linux or when openat2 is not available this is implemented + * via openat single path component traversal. Uses RESOLVE_BENEATH with openat2 + * or O_RESOLVE_BENEATH. + * + * @note Since this is Unix-only path is specified as CanonPath, which models + * Unix-style paths and ensures that there are no .. or . components. + * + * @param flags O_* flags + * @param mode Mode for O_{CREAT,TMPFILE} + * + * @throws SymlinkNotAllowed if any path components + */ +Descriptor openFileEnsureBeneathNoSymlinks(Descriptor dirFd, const CanonPath & path, int flags, mode_t mode = 0); + +} // namespace unix +#endif + MakeError(EndOfFile, Error); } // namespace nix diff --git a/src/libutil/unix/file-descriptor.cc b/src/libutil/unix/file-descriptor.cc index 2b612e854..ac699d6f1 100644 --- a/src/libutil/unix/file-descriptor.cc +++ b/src/libutil/unix/file-descriptor.cc @@ -1,3 +1,4 @@ +#include "nix/util/canon-path.hh" #include "nix/util/file-system.hh" #include "nix/util/signals.hh" #include "nix/util/finally.hh" @@ -7,6 +8,14 @@ #include #include +#if defined(__linux__) && defined(__NR_openat2) +# define HAVE_OPENAT2 1 +# include +# include +#else +# define HAVE_OPENAT2 0 +#endif + #include "util-config-private.hh" #include "util-unix-config-private.hh" @@ -223,4 +232,104 @@ void unix::closeOnExec(int fd) throw SysError("setting close-on-exec flag"); } +#ifdef __linux__ + +namespace linux { + +std::optional openat2(Descriptor dirFd, const char * path, uint64_t flags, uint64_t mode, uint64_t resolve) +{ +# if HAVE_OPENAT2 + /* Cache the result of whether openat2 is not supported. */ + static std::atomic_flag unsupported{}; + + if (!unsupported.test()) { + /* No glibc wrapper yet, but there's a patch: + * https://patchwork.sourceware.org/project/glibc/patch/20251029200519.3203914-1-adhemerval.zanella@linaro.org/ + */ + auto how = ::open_how{.flags = flags, .mode = mode, .resolve = resolve}; + auto res = ::syscall(__NR_openat2, dirFd, path, &how, sizeof(how)); + /* Cache that the syscall is not supported. */ + if (res < 0 && errno == ENOSYS) { + unsupported.test_and_set(); + return std::nullopt; + } + + return res; + } +# endif + return std::nullopt; +} + +} // namespace linux + +#endif + +static Descriptor +openFileEnsureBeneathNoSymlinksIterative(Descriptor dirFd, const CanonPath & path, int flags, mode_t mode) +{ + AutoCloseFD parentFd; + auto nrComponents = std::ranges::distance(path); + auto components = std::views::take(path, nrComponents - 1); /* Everything but last component */ + auto getParentFd = [&]() { return parentFd ? parentFd.get() : dirFd; }; + + /* This rather convoluted loop is necessary to avoid TOCTOU when validating that + no inner path component is a symlink. */ + for (auto it = components.begin(); it != components.end(); ++it) { + auto component = std::string(*it); /* Copy into a string to make NUL terminated. */ + assert(component != ".." && !component.starts_with('/')); /* In case invariant is broken somehow.. */ + + AutoCloseFD parentFd2 = ::openat( + getParentFd(), /* First iteration uses dirFd. */ + component.c_str(), + O_DIRECTORY | O_NOFOLLOW | O_CLOEXEC +#ifdef __linux__ + | O_PATH /* Linux-specific optimization. Files are open only for path resolution purposes. */ +#endif +#ifdef __FreeBSD__ + | O_RESOLVE_BENEATH /* Further guard against any possible SNAFUs. */ +#endif + ); + + if (!parentFd2) { + /* Construct the CanonPath for error message. */ + auto path2 = std::ranges::fold_left(components.begin(), ++it, CanonPath::root, [](auto lhs, auto rhs) { + lhs.push(rhs); + return lhs; + }); + + if (errno == ENOTDIR) /* Path component might be a symlink. */ { + struct ::stat st; + if (::fstatat(getParentFd(), component.c_str(), &st, AT_SYMLINK_NOFOLLOW) == 0 && S_ISLNK(st.st_mode)) + throw unix::SymlinkNotAllowed(path2); + errno = ENOTDIR; /* Restore the errno. */ + } else if (errno == ELOOP) { + throw unix::SymlinkNotAllowed(path2); + } + + return INVALID_DESCRIPTOR; + } + + parentFd = std::move(parentFd2); + } + + auto res = ::openat(getParentFd(), std::string(path.baseName().value()).c_str(), flags | O_NOFOLLOW, mode); + if (res < 0 && errno == ELOOP) + throw unix::SymlinkNotAllowed(path); + return res; +} + +Descriptor unix::openFileEnsureBeneathNoSymlinks(Descriptor dirFd, const CanonPath & path, int flags, mode_t mode) +{ +#ifdef __linux__ + auto maybeFd = linux::openat2( + dirFd, path.rel_c_str(), flags, static_cast(mode), RESOLVE_BENEATH | RESOLVE_NO_SYMLINKS); + if (maybeFd) { + if (*maybeFd < 0 && errno == ELOOP) + throw unix::SymlinkNotAllowed(path); + return *maybeFd; + } +#endif + return openFileEnsureBeneathNoSymlinksIterative(dirFd, path, flags, mode); +} + } // namespace nix From 439af1dca10fccb736ae363f2a290a82cc891a20 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Thalheim?= Date: Mon, 24 Nov 2025 21:11:19 +0100 Subject: [PATCH 15/40] feat(libstore): tie AWS CRT logging to Nix verbosity level Map Nix's verbosity levels to AWS CRT log levels so users can debug SSO authentication issues without modifying code: - Default/warn: AWS Warn (errors/warnings only) - Chatty (-vvv): AWS Info (credential provider actions) - Debug (-vvvv): AWS Debug (detailed auth flow) - Vomit (-vvvvv): AWS Trace (full CRT internal tracing) This makes it easy to diagnose SSO issues with: nix copy -vvvv --to s3://bucket?profile=foo ... --- src/libstore/aws-creds.cc | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/src/libstore/aws-creds.cc b/src/libstore/aws-creds.cc index ff7b0f0ef..dfdd81abb 100644 --- a/src/libstore/aws-creds.cc +++ b/src/libstore/aws-creds.cc @@ -79,7 +79,18 @@ class AwsCredentialProviderImpl : public AwsCredentialProvider public: AwsCredentialProviderImpl() { - apiHandle.InitializeLogging(Aws::Crt::LogLevel::Warn, static_cast(nullptr)); + // Map Nix's verbosity to AWS CRT log level + Aws::Crt::LogLevel logLevel; + if (verbosity >= lvlVomit) { + logLevel = Aws::Crt::LogLevel::Trace; + } else if (verbosity >= lvlDebug) { + logLevel = Aws::Crt::LogLevel::Debug; + } else if (verbosity >= lvlChatty) { + logLevel = Aws::Crt::LogLevel::Info; + } else { + logLevel = Aws::Crt::LogLevel::Warn; + } + apiHandle.InitializeLogging(logLevel, stderr); } AwsCredentials getCredentialsRaw(const std::string & profile); From f198e9a0b30fd884943aed454c9c00a7a4dd14bc Mon Sep 17 00:00:00 2001 From: John Ericson Date: Mon, 24 Nov 2025 14:11:54 -0500 Subject: [PATCH 16/40] Document the JSON Schema testing a bit --- doc/manual/source/development/testing.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/doc/manual/source/development/testing.md b/doc/manual/source/development/testing.md index 7c2cbbb5d..dd965862a 100644 --- a/doc/manual/source/development/testing.md +++ b/doc/manual/source/development/testing.md @@ -137,6 +137,12 @@ $ _NIX_TEST_ACCEPT=1 meson test nix-store-tests -v will regenerate the "golden master" expected result for the `libnixstore` characterisation tests. The characterisation tests will mark themselves "skipped" since they regenerated the expected result instead of actually testing anything. +### JSON Schema testing + +In `doc/manual/source/protocols/json/` we have a number of manual pages generated from [JSON Schema](https://json-schema.org/). +That JSON schema is tested against the JSON file test data used in [characterisation tests](#characterisation-testing-unit ) for JSON (de)serialization, in `src/json-schema-checks`. +Between the JSON (de)serialization testing, and this testing of the same data against the schema, we make sure that the manual, the implementation, and a machine-readable schema are are all in sync. + ### Unit test support libraries There are headers and code which are not just used to test the library in question, but also downstream libraries. From d8d75cff9f11735ba5e49e3bf07629e7a2f6f93e Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 24 Nov 2025 22:03:58 +0000 Subject: [PATCH 17/40] build(deps): bump actions/checkout from 5 to 6 Bumps [actions/checkout](https://github.com/actions/checkout) from 5 to 6. - [Release notes](https://github.com/actions/checkout/releases) - [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md) - [Commits](https://github.com/actions/checkout/compare/v5...v6) --- updated-dependencies: - dependency-name: actions/checkout dependency-version: '6' dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] --- .github/workflows/backport.yml | 2 +- .github/workflows/ci.yml | 18 +++++++++--------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/.github/workflows/backport.yml b/.github/workflows/backport.yml index 1413a203c..5ad073785 100644 --- a/.github/workflows/backport.yml +++ b/.github/workflows/backport.yml @@ -20,7 +20,7 @@ jobs: with: app-id: ${{ vars.CI_APP_ID }} private-key: ${{ secrets.CI_APP_PRIVATE_KEY }} - - uses: actions/checkout@v5 + - uses: actions/checkout@v6 with: ref: ${{ github.event.pull_request.head.sha }} # required to find all branches diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 4ab16ab55..fe9d94248 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -24,7 +24,7 @@ jobs: eval: runs-on: ubuntu-24.04 steps: - - uses: actions/checkout@v5 + - uses: actions/checkout@v6 with: fetch-depth: 0 - uses: ./.github/actions/install-nix-action @@ -40,7 +40,7 @@ jobs: name: pre-commit checks runs-on: ubuntu-24.04 steps: - - uses: actions/checkout@v5 + - uses: actions/checkout@v6 - uses: ./.github/actions/install-nix-action with: dogfood: ${{ github.event_name == 'workflow_dispatch' && inputs.dogfood || github.event_name != 'workflow_dispatch' }} @@ -87,7 +87,7 @@ jobs: runs-on: ${{ matrix.runs-on }} timeout-minutes: 60 steps: - - uses: actions/checkout@v5 + - uses: actions/checkout@v6 with: fetch-depth: 0 - uses: ./.github/actions/install-nix-action @@ -162,7 +162,7 @@ jobs: name: installer test ${{ matrix.scenario }} runs-on: ${{ matrix.runs-on }} steps: - - uses: actions/checkout@v5 + - uses: actions/checkout@v6 - name: Download installer tarball uses: actions/download-artifact@v6 with: @@ -227,7 +227,7 @@ jobs: github.ref_name == 'master' runs-on: ubuntu-24.04 steps: - - uses: actions/checkout@v5 + - uses: actions/checkout@v6 with: fetch-depth: 0 - uses: ./.github/actions/install-nix-action @@ -276,14 +276,14 @@ jobs: runs-on: ubuntu-24.04 steps: - name: Checkout nix - uses: actions/checkout@v5 + uses: actions/checkout@v6 - name: Checkout flake-regressions - uses: actions/checkout@v5 + uses: actions/checkout@v6 with: repository: NixOS/flake-regressions path: flake-regressions - name: Checkout flake-regressions-data - uses: actions/checkout@v5 + uses: actions/checkout@v6 with: repository: NixOS/flake-regressions-data path: flake-regressions/tests @@ -303,7 +303,7 @@ jobs: github.event_name == 'push' && github.ref_name == 'master' steps: - - uses: actions/checkout@v5 + - uses: actions/checkout@v6 with: fetch-depth: 0 - uses: ./.github/actions/install-nix-action From f78e88c9730c7718eb919c34579af8a818ff4c58 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Mon, 24 Nov 2025 14:15:06 -0500 Subject: [PATCH 18/40] Add some infrastructure changes for better JSON `ref` impls Also skip a trailing semicolon inside a macro so the caller can use it instead, which is generally nicer to the formatter. --- .../nix/util/tests/json-characterization.hh | 20 ++++++++++++++++ src/libutil/include/nix/util/json-impls.hh | 24 ++++++++++++++----- .../nix/util/memory-source-accessor.hh | 10 ++++---- 3 files changed, 43 insertions(+), 11 deletions(-) diff --git a/src/libutil-test-support/include/nix/util/tests/json-characterization.hh b/src/libutil-test-support/include/nix/util/tests/json-characterization.hh index bd70ba830..6db32c4b6 100644 --- a/src/libutil-test-support/include/nix/util/tests/json-characterization.hh +++ b/src/libutil-test-support/include/nix/util/tests/json-characterization.hh @@ -5,6 +5,7 @@ #include #include "nix/util/types.hh" +#include "nix/util/ref.hh" #include "nix/util/file-system.hh" #include "nix/util/tests/characterization.hh" @@ -39,6 +40,25 @@ void writeJsonTest(CharacterizationTest & test, PathView testStem, const T & val [](const auto & file, const auto & got) { return writeFile(file, got.dump(2) + "\n"); }); } +/** + * Specialization for when we need to do "JSON -> `ref`" in one + * direction, but "`const T &` -> JSON" in the other direction. + * + * We can't just return `const T &`, but it would be wasteful to + * requires a `const ref &` double indirection (and mandatory shared + * pointer), so we break the symmetry as the best remaining option. + */ +template +void writeJsonTest(CharacterizationTest & test, PathView testStem, const ref & value) +{ + using namespace nlohmann; + test.writeTest( + Path{testStem} + ".json", + [&]() -> json { return static_cast(*value); }, + [](const auto & file) { return json::parse(readFile(file)); }, + [](const auto & file, const auto & got) { return writeFile(file, got.dump(2) + "\n"); }); +} + /** * Golden test in the middle of something */ diff --git a/src/libutil/include/nix/util/json-impls.hh b/src/libutil/include/nix/util/json-impls.hh index b55716a6d..26a94472f 100644 --- a/src/libutil/include/nix/util/json-impls.hh +++ b/src/libutil/include/nix/util/json-impls.hh @@ -6,18 +6,30 @@ #include "nix/util/experimental-features.hh" // Following https://github.com/nlohmann/json#how-can-i-use-get-for-non-default-constructiblenon-copyable-types +#define JSON_IMPL_INNER_TO(TYPE) \ + struct adl_serializer \ + { \ + static void to_json(json & json, const TYPE & t); \ + } + +#define JSON_IMPL_INNER_FROM(TYPE) \ + struct adl_serializer \ + { \ + static TYPE from_json(const json & json); \ + } + #define JSON_IMPL_INNER(TYPE) \ struct adl_serializer \ { \ static TYPE from_json(const json & json); \ static void to_json(json & json, const TYPE & t); \ - }; + } -#define JSON_IMPL(TYPE) \ - namespace nlohmann { \ - using namespace nix; \ - template<> \ - JSON_IMPL_INNER(TYPE) \ +#define JSON_IMPL(TYPE) \ + namespace nlohmann { \ + using namespace nix; \ + template<> \ + JSON_IMPL_INNER(TYPE); \ } #define JSON_IMPL_WITH_XP_FEATURES(TYPE) \ diff --git a/src/libutil/include/nix/util/memory-source-accessor.hh b/src/libutil/include/nix/util/memory-source-accessor.hh index 933af600a..fc00f34d9 100644 --- a/src/libutil/include/nix/util/memory-source-accessor.hh +++ b/src/libutil/include/nix/util/memory-source-accessor.hh @@ -188,23 +188,23 @@ using namespace nix; #define ARG fso::Regular template -JSON_IMPL_INNER(ARG) +JSON_IMPL_INNER(ARG); #undef ARG #define ARG fso::DirectoryT template -JSON_IMPL_INNER(ARG) +JSON_IMPL_INNER(ARG); #undef ARG template<> -JSON_IMPL_INNER(fso::Symlink) +JSON_IMPL_INNER(fso::Symlink); template<> -JSON_IMPL_INNER(fso::Opaque) +JSON_IMPL_INNER(fso::Opaque); #define ARG fso::VariantT template -JSON_IMPL_INNER(ARG) +JSON_IMPL_INNER(ARG); #undef ARG } // namespace nlohmann From b0c016ae7d1b04f5bd89ccd54b3698bb3d76d38b Mon Sep 17 00:00:00 2001 From: John Ericson Date: Mon, 24 Nov 2025 15:03:46 -0500 Subject: [PATCH 19/40] `DummyStore` build trace holds `UnkeyedRealisation` by value Otherwise the equality instance we need to add will be messed up. --- src/libstore-tests/dummy-store.cc | 2 +- src/libstore/dummy-store.cc | 7 +++---- src/libstore/include/nix/store/dummy-store-impl.hh | 2 +- 3 files changed, 5 insertions(+), 6 deletions(-) diff --git a/src/libstore-tests/dummy-store.cc b/src/libstore-tests/dummy-store.cc index 3dd8137a3..c87b8d773 100644 --- a/src/libstore-tests/dummy-store.cc +++ b/src/libstore-tests/dummy-store.cc @@ -27,7 +27,7 @@ TEST(DummyStore, realisation_read) .outPath = StorePath{"g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo.drv"}, }; - store->buildTrace.insert({drvHash, {{outputName, make_ref(value)}}}); + store->buildTrace.insert({drvHash, {{outputName, value}}}); auto value2 = store->queryRealisation({drvHash, outputName}); diff --git a/src/libstore/dummy-store.cc b/src/libstore/dummy-store.cc index c45a13cc3..57bbcd231 100644 --- a/src/libstore/dummy-store.cc +++ b/src/libstore/dummy-store.cc @@ -320,9 +320,8 @@ struct DummyStoreImpl : DummyStore void registerDrvOutput(const Realisation & output) override { - auto ref = make_ref(output); - buildTrace.insert_or_visit({output.id.drvHash, {{output.id.outputName, ref}}}, [&](auto & kv) { - kv.second.insert_or_assign(output.id.outputName, make_ref(output)); + buildTrace.insert_or_visit({output.id.drvHash, {{output.id.outputName, output}}}, [&](auto & kv) { + kv.second.insert_or_assign(output.id.outputName, output); }); } @@ -333,7 +332,7 @@ struct DummyStoreImpl : DummyStore buildTrace.cvisit(drvOutput.drvHash, [&](const auto & kv) { if (auto it = kv.second.find(drvOutput.outputName); it != kv.second.end()) { visited = true; - callback(it->second.get_ptr()); + callback(std::make_shared(it->second)); } }); diff --git a/src/libstore/include/nix/store/dummy-store-impl.hh b/src/libstore/include/nix/store/dummy-store-impl.hh index 137f81c9b..095767aaf 100644 --- a/src/libstore/include/nix/store/dummy-store-impl.hh +++ b/src/libstore/include/nix/store/dummy-store-impl.hh @@ -47,7 +47,7 @@ struct DummyStore : virtual Store * outer map for the derivation, and inner maps for the outputs of a * given derivation. */ - boost::concurrent_flat_map>> buildTrace; + boost::concurrent_flat_map> buildTrace; DummyStore(ref config) : Store{*config} From 622a5cd1bf77f97eca53dd453abed03ee370f150 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Mon, 24 Nov 2025 15:07:55 -0500 Subject: [PATCH 20/40] Add `DummyStore::operator==` Will need it for tests. --- src/libstore/dummy-store.cc | 10 ++++++++++ src/libstore/include/nix/store/dummy-store-impl.hh | 4 ++++ 2 files changed, 14 insertions(+) diff --git a/src/libstore/dummy-store.cc b/src/libstore/dummy-store.cc index 57bbcd231..852c38b75 100644 --- a/src/libstore/dummy-store.cc +++ b/src/libstore/dummy-store.cc @@ -16,6 +16,16 @@ std::string DummyStoreConfig::doc() ; } +bool DummyStore::PathInfoAndContents::operator==(const PathInfoAndContents & other) const +{ + return info == other.info && contents->root == other.contents->root; +} + +bool DummyStore::operator==(const DummyStore & other) const +{ + return contents == other.contents && derivations == other.derivations && buildTrace == other.buildTrace; +} + namespace { class WholeStoreViewAccessor : public SourceAccessor diff --git a/src/libstore/include/nix/store/dummy-store-impl.hh b/src/libstore/include/nix/store/dummy-store-impl.hh index 095767aaf..9b078eeaa 100644 --- a/src/libstore/include/nix/store/dummy-store-impl.hh +++ b/src/libstore/include/nix/store/dummy-store-impl.hh @@ -23,6 +23,8 @@ struct DummyStore : virtual Store { UnkeyedValidPathInfo info; ref contents; + + bool operator==(const PathInfoAndContents &) const; }; /** @@ -54,6 +56,8 @@ struct DummyStore : virtual Store , config(config) { } + + bool operator==(const DummyStore &) const; }; } // namespace nix From 0275b64b816223220c357bfc8ac6f0441980692d Mon Sep 17 00:00:00 2001 From: John Ericson Date: Sun, 28 Sep 2025 02:15:20 -0400 Subject: [PATCH 21/40] JSON impl and Schema for `DummyStore` This is the "keystone" that puts most of the other store-layer JSON formats together. Also, add some documentation for JSON testing. --- doc/manual/package.nix | 1 + doc/manual/source/SUMMARY.md.in | 1 + doc/manual/source/protocols/json/meson.build | 1 + .../json/schema/build-trace-entry-v1.yaml | 136 +++++++++++------- .../source/protocols/json/schema/store-v1 | 1 + .../protocols/json/schema/store-v1.yaml | 90 ++++++++++++ doc/manual/source/protocols/json/store.md | 21 +++ src/json-schema-checks/meson.build | 13 ++ src/json-schema-checks/package.nix | 1 + src/json-schema-checks/store | 1 + .../data/dummy-store/empty.json | 8 ++ .../data/dummy-store/one-derivation.json | 22 +++ .../data/dummy-store/one-flat-file.json | 38 +++++ .../data/dummy-store/one-realisation.json | 16 +++ src/libstore-tests/dummy-store.cc | 113 +++++++++++++++ src/libstore/dummy-store.cc | 98 +++++++++++++ .../include/nix/store/dummy-store-impl.hh | 6 + src/libstore/include/nix/store/dummy-store.hh | 30 ++++ 18 files changed, 542 insertions(+), 55 deletions(-) create mode 120000 doc/manual/source/protocols/json/schema/store-v1 create mode 100644 doc/manual/source/protocols/json/schema/store-v1.yaml create mode 100644 doc/manual/source/protocols/json/store.md create mode 120000 src/json-schema-checks/store create mode 100644 src/libstore-tests/data/dummy-store/empty.json create mode 100644 src/libstore-tests/data/dummy-store/one-derivation.json create mode 100644 src/libstore-tests/data/dummy-store/one-flat-file.json create mode 100644 src/libstore-tests/data/dummy-store/one-realisation.json diff --git a/doc/manual/package.nix b/doc/manual/package.nix index b78ccee88..1d7d73ce9 100644 --- a/doc/manual/package.nix +++ b/doc/manual/package.nix @@ -47,6 +47,7 @@ mkMesonDerivation (finalAttrs: { ../../src/libstore-tests/data/path-info ../../src/libstore-tests/data/nar-info ../../src/libstore-tests/data/build-result + ../../src/libstore-tests/data/dummy-store # Too many different types of files to filter for now ../../doc/manual ./. diff --git a/doc/manual/source/SUMMARY.md.in b/doc/manual/source/SUMMARY.md.in index 64bf593f4..a8e286314 100644 --- a/doc/manual/source/SUMMARY.md.in +++ b/doc/manual/source/SUMMARY.md.in @@ -131,6 +131,7 @@ - [Deriving Path](protocols/json/deriving-path.md) - [Build Trace Entry](protocols/json/build-trace-entry.md) - [Build Result](protocols/json/build-result.md) + - [Store](protocols/json/store.md) - [Serving Tarball Flakes](protocols/tarball-fetcher.md) - [Store Path Specification](protocols/store-path.md) - [Nix Archive (NAR) Format](protocols/nix-archive/index.md) diff --git a/doc/manual/source/protocols/json/meson.build b/doc/manual/source/protocols/json/meson.build index 8780d8057..ab9d76d3e 100644 --- a/doc/manual/source/protocols/json/meson.build +++ b/doc/manual/source/protocols/json/meson.build @@ -19,6 +19,7 @@ schemas = [ 'deriving-path-v1', 'build-trace-entry-v1', 'build-result-v1', + 'store-v1', ] schema_files = files() diff --git a/doc/manual/source/protocols/json/schema/build-trace-entry-v1.yaml b/doc/manual/source/protocols/json/schema/build-trace-entry-v1.yaml index cabf2c350..a85738b50 100644 --- a/doc/manual/source/protocols/json/schema/build-trace-entry-v1.yaml +++ b/doc/manual/source/protocols/json/schema/build-trace-entry-v1.yaml @@ -4,71 +4,97 @@ title: Build Trace Entry description: | A record of a successful build outcome for a specific derivation output. - This schema describes the JSON representation of a [build trace entry](@docroot@/store/build-trace.md) entry. + This schema describes the JSON representation of a [build trace entry](@docroot@/store/build-trace.md). > **Warning** > > This JSON format is currently > [**experimental**](@docroot@/development/experimental-features.md#xp-feature-ca-derivations) > and subject to change. - -type: object required: - id - outPath - dependentRealisations - signatures +allOf: + - "$ref": "#/$defs/key" + - "$ref": "#/$defs/value" properties: - id: - type: string - title: Derivation Output ID - pattern: "^sha256:[0-9a-f]{64}![a-zA-Z_][a-zA-Z0-9_-]*$" - description: | - Unique identifier for the derivation output that was built. - - Format: `{hash-quotient-drv}!{output-name}` - - - **hash-quotient-drv**: SHA-256 [hash of the quotient derivation](@docroot@/store/derivation/outputs/input-address.md#hash-quotient-drv). - Begins with `sha256:`. - - - **output-name**: Name of the specific output (e.g., "out", "dev", "doc") - - Example: `"sha256:ba7816bf8f01cfea414140de5dae2223b00361a396177a9cb410ff61f20015ad!foo"` - - outPath: - "$ref": "store-path-v1.yaml" - title: Output Store Path - description: | - The path to the store object that resulted from building this derivation for the given output name. - - dependentRealisations: - type: object - title: Underlying Base Build Trace - description: | - This is for [*derived*](@docroot@/store/build-trace.md#derived) build trace entries to ensure coherence. - - Keys are derivation output IDs (same format as the main `id` field). - Values are the store paths that those dependencies resolved to. - - As described in the linked section on derived build trace traces, derived build trace entries must be kept in addition and not instead of the underlying base build entries. - This is the set of base build trace entries that this derived build trace is derived from. - (The set is also a map since this miniature base build trace must be coherent, mapping each key to a single value.) - - patternProperties: - "^sha256:[0-9a-f]{64}![a-zA-Z_][a-zA-Z0-9_-]*$": - $ref: "store-path-v1.yaml" - title: Dependent Store Path - description: Store path that this dependency resolved to during the build - additionalProperties: false - - signatures: - type: array - title: Build Signatures - description: | - A set of cryptographic signatures attesting to the authenticity of this build trace entry. - items: - type: string - title: Signature - description: A single cryptographic signature - + id: {} + outPath: {} + dependentRealisations: {} + signatures: {} additionalProperties: false + +"$defs": + key: + title: Build Trace Key + description: | + A [build trace entry](@docroot@/store/build-trace.md) is a key-value pair. + This is the "key" part, refering to a derivation and output. + type: object + required: + - id + properties: + id: + type: string + title: Derivation Output ID + pattern: "^sha256:[0-9a-f]{64}![a-zA-Z_][a-zA-Z0-9_-]*$" + description: | + Unique identifier for the derivation output that was built. + + Format: `{hash-quotient-drv}!{output-name}` + + - **hash-quotient-drv**: SHA-256 [hash of the quotient derivation](@docroot@/store/derivation/outputs/input-address.md#hash-quotient-drv). + Begins with `sha256:`. + + - **output-name**: Name of the specific output (e.g., "out", "dev", "doc") + + Example: `"sha256:ba7816bf8f01cfea414140de5dae2223b00361a396177a9cb410ff61f20015ad!foo"` + + value: + title: Build Trace Value + description: | + A [build trace entry](@docroot@/store/build-trace.md) is a key-value pair. + This is the "value" part, describing an output. + type: object + required: + - outPath + - dependentRealisations + - signatures + properties: + outPath: + "$ref": "store-path-v1.yaml" + title: Output Store Path + description: | + The path to the store object that resulted from building this derivation for the given output name. + + dependentRealisations: + type: object + title: Underlying Base Build Trace + description: | + This is for [*derived*](@docroot@/store/build-trace.md#derived) build trace entries to ensure coherence. + + Keys are derivation output IDs (same format as the main `id` field). + Values are the store paths that those dependencies resolved to. + + As described in the linked section on derived build trace traces, derived build trace entries must be kept in addition and not instead of the underlying base build entries. + This is the set of base build trace entries that this derived build trace is derived from. + (The set is also a map since this miniature base build trace must be coherent, mapping each key to a single value.) + + patternProperties: + "^sha256:[0-9a-f]{64}![a-zA-Z_][a-zA-Z0-9_-]*$": + "$ref": "store-path-v1.yaml" + title: Dependent Store Path + description: Store path that this dependency resolved to during the build + additionalProperties: false + + signatures: + type: array + title: Build Signatures + description: | + A set of cryptographic signatures attesting to the authenticity of this build trace entry. + items: + type: string + title: Signature + description: A single cryptographic signature diff --git a/doc/manual/source/protocols/json/schema/store-v1 b/doc/manual/source/protocols/json/schema/store-v1 new file mode 120000 index 000000000..0cb61f962 --- /dev/null +++ b/doc/manual/source/protocols/json/schema/store-v1 @@ -0,0 +1 @@ +../../../../../../src/libstore-tests/data/dummy-store \ No newline at end of file diff --git a/doc/manual/source/protocols/json/schema/store-v1.yaml b/doc/manual/source/protocols/json/schema/store-v1.yaml new file mode 100644 index 000000000..e0c6f8fed --- /dev/null +++ b/doc/manual/source/protocols/json/schema/store-v1.yaml @@ -0,0 +1,90 @@ +"$schema": "http://json-schema.org/draft-04/schema" +"$id": "https://nix.dev/manual/nix/latest/protocols/json/schema/store-v1.json" +title: Store +description: | + Experimental JSON representation of a Nix [Store](@docroot@/store/index.md). + + This schema describes the JSON serialization of a Nix store. + We use it for (de)serializing in-memory "dummy stores" used for testing, but in principle the data represented in this schema could live in any type of store. + + > **Warning** + > + > This JSON format is currently + > [**experimental**](@docroot@/development/experimental-features.md#xp-feature-nix-command) + > and subject to change. + +type: object +required: + - config + - contents + - derivations + - buildTrace +properties: + config: + "$ref": "#/$defs/storeConfig" + + contents: + type: object + title: Store Objects + description: | + Map of [store path](@docroot@/store/store-path.md) base names to [store objects](@docroot@/store/store-object.md). + patternProperties: + "^[0123456789abcdfghijklmnpqrsvwxyz]{32}-.+$": + type: object + title: Store Object + required: + - info + - contents + properties: + info: + "$ref": "./store-object-info-v2.yaml#/$defs/impure" + title: Store Object Info + description: | + Metadata about the [store object](@docroot@/store/store-object.md) including hash, size, references, etc. + contents: + "$ref": "./file-system-object-v1.yaml" + title: File System Object Contents + description: | + The actual [file system object](@docroot@/store/file-system-object.md) contents of this store path. + additionalProperties: false + additionalProperties: false + + derivations: + type: object + title: Derivations + description: | + Map of [store path](@docroot@/store/store-path.md) base names (always ending in `.drv`) to [derivations](@docroot@/store/derivation/index.md). + patternProperties: + "^[0123456789abcdfghijklmnpqrsvwxyz]{32}-.+\\.drv$": + "$ref": "./derivation-v4.yaml" + additionalProperties: false + + buildTrace: + type: object + title: Build Trace + description: | + Map of output hashes (base64 SHA256) to maps of output names to realisations. + Records which outputs have been built and their realisations. + See [Build Trace](@docroot@/store/build-trace.md) for more details. + patternProperties: + "^[A-Za-z0-9+/]{43}=$": + type: object + additionalProperties: + "$ref": "./build-trace-entry-v1.yaml#/$defs/value" + additionalProperties: false + +"$defs": + storeConfig: + title: Store Configuration + description: | + Configuration for the store, including the store directory path. + type: object + required: + - store + properties: + store: + type: string + title: Store Directory + description: | + The store directory path (e.g., `/nix/store`). + additionalProperties: false diff --git a/doc/manual/source/protocols/json/store.md b/doc/manual/source/protocols/json/store.md new file mode 100644 index 000000000..951c1759e --- /dev/null +++ b/doc/manual/source/protocols/json/store.md @@ -0,0 +1,21 @@ +{{#include store-v1-fixed.md}} + +## Examples + +### Empty store + +```json +{{#include schema/store-v1/empty.json}} +``` + +### Store with one file + +```json +{{#include schema/store-v1/one-flat-file.json}} +``` + +### Store with one derivation + +```json +{{#include schema/store-v1/one-derivation.json}} +``` diff --git a/src/json-schema-checks/meson.build b/src/json-schema-checks/meson.build index 73be4a47d..09c8cd048 100644 --- a/src/json-schema-checks/meson.build +++ b/src/json-schema-checks/meson.build @@ -212,6 +212,19 @@ schemas += [ }, ] +# Dummy store +schemas += [ + { + 'stem' : 'store', + 'schema' : schema_dir / 'store-v1.yaml', + 'files' : [ + 'empty.json', + 'one-flat-file.json', + 'one-derivation.json', + ], + }, +] + # Validate each example against the schema foreach schema : schemas stem = schema['stem'] diff --git a/src/json-schema-checks/package.nix b/src/json-schema-checks/package.nix index d9ca880e5..a5ee1f059 100644 --- a/src/json-schema-checks/package.nix +++ b/src/json-schema-checks/package.nix @@ -30,6 +30,7 @@ mkMesonDerivation (finalAttrs: { ../../src/libstore-tests/data/path-info ../../src/libstore-tests/data/nar-info ../../src/libstore-tests/data/build-result + ../../src/libstore-tests/data/dummy-store ./. ]; diff --git a/src/json-schema-checks/store b/src/json-schema-checks/store new file mode 120000 index 000000000..442f0749a --- /dev/null +++ b/src/json-schema-checks/store @@ -0,0 +1 @@ +../../src/libstore-tests/data/dummy-store \ No newline at end of file diff --git a/src/libstore-tests/data/dummy-store/empty.json b/src/libstore-tests/data/dummy-store/empty.json new file mode 100644 index 000000000..93bec5153 --- /dev/null +++ b/src/libstore-tests/data/dummy-store/empty.json @@ -0,0 +1,8 @@ +{ + "buildTrace": {}, + "config": { + "store": "/nix/store" + }, + "contents": {}, + "derivations": {} +} diff --git a/src/libstore-tests/data/dummy-store/one-derivation.json b/src/libstore-tests/data/dummy-store/one-derivation.json new file mode 100644 index 000000000..a3e3391e6 --- /dev/null +++ b/src/libstore-tests/data/dummy-store/one-derivation.json @@ -0,0 +1,22 @@ +{ + "buildTrace": {}, + "config": { + "store": "/nix/store" + }, + "contents": {}, + "derivations": { + "rlqjbbb65ggcx9hy577hvnn929wz1aj0-foo.drv": { + "args": [], + "builder": "", + "env": {}, + "inputs": { + "drvs": {}, + "srcs": [] + }, + "name": "foo", + "outputs": {}, + "system": "", + "version": 4 + } + } +} diff --git a/src/libstore-tests/data/dummy-store/one-flat-file.json b/src/libstore-tests/data/dummy-store/one-flat-file.json new file mode 100644 index 000000000..d572b4c4f --- /dev/null +++ b/src/libstore-tests/data/dummy-store/one-flat-file.json @@ -0,0 +1,38 @@ +{ + "buildTrace": {}, + "config": { + "store": "/nix/store" + }, + "contents": { + "5hizn7xyyrhxr0k2magvxl5ccvk0ci9n-my-file": { + "contents": { + "contents": "asdf", + "executable": false, + "type": "regular" + }, + "info": { + "ca": { + "hash": { + "algorithm": "sha256", + "format": "base64", + "hash": "f1eduuSIYC1BofXA1tycF79Ai2NSMJQtUErx5DxLYSU=" + }, + "method": "nar" + }, + "deriver": null, + "narHash": { + "algorithm": "sha256", + "format": "base64", + "hash": "f1eduuSIYC1BofXA1tycF79Ai2NSMJQtUErx5DxLYSU=" + }, + "narSize": 120, + "references": [], + "registrationTime": null, + "signatures": [], + "ultimate": false, + "version": 2 + } + } + }, + "derivations": {} +} diff --git a/src/libstore-tests/data/dummy-store/one-realisation.json b/src/libstore-tests/data/dummy-store/one-realisation.json new file mode 100644 index 000000000..b5c8b8c56 --- /dev/null +++ b/src/libstore-tests/data/dummy-store/one-realisation.json @@ -0,0 +1,16 @@ +{ + "buildTrace": { + "ungWv48Bz+pBQUDeXa4iI7ADYaOWF3qctBD/YfIAFa0=": { + "out": { + "dependentRealisations": {}, + "outPath": "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo", + "signatures": [] + } + } + }, + "config": { + "store": "/nix/store" + }, + "contents": {}, + "derivations": {} +} diff --git a/src/libstore-tests/dummy-store.cc b/src/libstore-tests/dummy-store.cc index c87b8d773..4a12dcf78 100644 --- a/src/libstore-tests/dummy-store.cc +++ b/src/libstore-tests/dummy-store.cc @@ -1,11 +1,32 @@ #include +#include +#include "nix/util/memory-source-accessor.hh" #include "nix/store/dummy-store-impl.hh" #include "nix/store/globals.hh" #include "nix/store/realisation.hh" +#include "nix/util/tests/json-characterization.hh" + namespace nix { +class DummyStoreTest : public virtual CharacterizationTest +{ + std::filesystem::path unitTestData = getUnitTestData() / "dummy-store"; + +public: + + std::filesystem::path goldenMaster(std::string_view testStem) const override + { + return unitTestData / testStem; + } + + static void SetUpTestSuite() + { + initLibStore(false); + } +}; + TEST(DummyStore, realisation_read) { initLibStore(/*loadConfig=*/false); @@ -35,4 +56,96 @@ TEST(DummyStore, realisation_read) EXPECT_EQ(*value2, value); } +/* ---------------------------------------------------------------------------- + * JSON + * --------------------------------------------------------------------------*/ + +using nlohmann::json; + +struct DummyStoreJsonTest : DummyStoreTest, + JsonCharacterizationTest>, + ::testing::WithParamInterface>> +{}; + +TEST_P(DummyStoreJsonTest, from_json) +{ + auto & [name, expected] = GetParam(); + using namespace nlohmann; + /* Cannot use `readJsonTest` because need to dereference the stores + for equality. */ + readTest(Path{name} + ".json", [&](const auto & encodedRaw) { + auto encoded = json::parse(encodedRaw); + ref decoded = adl_serializer>::from_json(encoded); + ASSERT_EQ(*decoded, *expected); + }); +} + +TEST_P(DummyStoreJsonTest, to_json) +{ + auto & [name, value] = GetParam(); + writeJsonTest(name, value); +} + +INSTANTIATE_TEST_SUITE_P(DummyStoreJSON, DummyStoreJsonTest, [] { + initLibStore(false); + auto writeCfg = make_ref(DummyStore::Config::Params{}); + writeCfg->readOnly = false; + return ::testing::Values( + std::pair{ + "empty", + make_ref(DummyStore::Config::Params{})->openDummyStore(), + }, + std::pair{ + "one-flat-file", + [&] { + auto store = writeCfg->openDummyStore(); + store->addToStore( + "my-file", + SourcePath{ + [] { + auto sc = make_ref(); + sc->root = MemorySourceAccessor::File{MemorySourceAccessor::File::Regular{ + .executable = false, + .contents = "asdf", + }}; + return sc; + }(), + }, + ContentAddressMethod::Raw::NixArchive, + HashAlgorithm::SHA256); + return store; + }(), + }, + std::pair{ + "one-derivation", + [&] { + auto store = writeCfg->openDummyStore(); + Derivation drv; + drv.name = "foo"; + store->writeDerivation(drv); + return store; + }(), + }, + std::pair{ + "one-realisation", + [&] { + auto store = writeCfg->openDummyStore(); + store->buildTrace.insert_or_assign( + Hash::parseExplicitFormatUnprefixed( + "ba7816bf8f01cfea414140de5dae2223b00361a396177a9cb410ff61f20015ad", + HashAlgorithm::SHA256, + HashFormat::Base16), + std::map{ + { + "out", + UnkeyedRealisation{ + .outPath = StorePath{"g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo"}, + }, + }, + }); + return store; + }(), + }); +}()); + } // namespace nix diff --git a/src/libstore/dummy-store.cc b/src/libstore/dummy-store.cc index 852c38b75..aa763a679 100644 --- a/src/libstore/dummy-store.cc +++ b/src/libstore/dummy-store.cc @@ -2,6 +2,7 @@ #include "nix/util/archive.hh" #include "nix/util/callback.hh" #include "nix/util/memory-source-accessor.hh" +#include "nix/util/json-utils.hh" #include "nix/store/dummy-store-impl.hh" #include "nix/store/realisation.hh" @@ -386,3 +387,100 @@ ref DummyStore::Config::openDummyStore() const static RegisterStoreImplementation regDummyStore; } // namespace nix + +namespace nlohmann { + +using namespace nix; + +DummyStore::PathInfoAndContents adl_serializer::from_json(const json & json) +{ + auto & obj = getObject(json); + return DummyStore::PathInfoAndContents{ + .info = valueAt(obj, "info"), + .contents = make_ref(valueAt(obj, "contents")), + }; +} + +void adl_serializer::to_json(json & json, const DummyStore::PathInfoAndContents & val) +{ + json = { + {"info", val.info}, + {"contents", *val.contents}, + }; +} + +ref adl_serializer>::from_json(const json & json) +{ + auto & obj = getObject(json); + auto cfg = make_ref(DummyStore::Config::Params{}); + const_cast(cfg->storeDir_).set(getString(valueAt(obj, "store"))); + cfg->readOnly = true; + return cfg; +} + +void adl_serializer::to_json(json & json, const DummyStoreConfig & val) +{ + json = { + {"store", val.storeDir}, + }; +} + +ref adl_serializer>::from_json(const json & json) +{ + auto & obj = getObject(json); + ref res = adl_serializer>::from_json(valueAt(obj, "config"))->openDummyStore(); + for (auto & [k, v] : getObject(valueAt(obj, "contents"))) + res->contents.insert({StorePath{k}, v}); + for (auto & [k, v] : getObject(valueAt(obj, "derivations"))) + res->derivations.insert({StorePath{k}, v}); + for (auto & [k0, v] : getObject(valueAt(obj, "buildTrace"))) { + for (auto & [k1, v2] : getObject(v)) { + UnkeyedRealisation realisation = v2; + res->buildTrace.insert_or_visit( + { + Hash::parseExplicitFormatUnprefixed(k0, HashAlgorithm::SHA256, HashFormat::Base64), + {{k1, realisation}}, + }, + [&](auto & kv) { kv.second.insert_or_assign(k1, realisation); }); + } + } + return res; +} + +void adl_serializer::to_json(json & json, const DummyStore & val) +{ + json = { + {"config", *val.config}, + {"contents", + [&] { + auto obj = json::object(); + val.contents.cvisit_all([&](const auto & kv) { + auto & [k, v] = kv; + obj[k.to_string()] = v; + }); + return obj; + }()}, + {"derivations", + [&] { + auto obj = json::object(); + val.derivations.cvisit_all([&](const auto & kv) { + auto & [k, v] = kv; + obj[k.to_string()] = v; + }); + return obj; + }()}, + {"buildTrace", + [&] { + auto obj = json::object(); + val.buildTrace.cvisit_all([&](const auto & kv) { + auto & [k, v] = kv; + auto & obj2 = obj[k.to_string(HashFormat::Base64, false)] = json::object(); + for (auto & [k2, v2] : kv.second) + obj2[k2] = v2; + }); + return obj; + }()}, + }; +} + +} // namespace nlohmann diff --git a/src/libstore/include/nix/store/dummy-store-impl.hh b/src/libstore/include/nix/store/dummy-store-impl.hh index 9b078eeaa..ac7ab9c68 100644 --- a/src/libstore/include/nix/store/dummy-store-impl.hh +++ b/src/libstore/include/nix/store/dummy-store-impl.hh @@ -60,4 +60,10 @@ struct DummyStore : virtual Store bool operator==(const DummyStore &) const; }; +template<> +struct json_avoids_null : std::true_type +{}; + } // namespace nix + +JSON_IMPL(nix::DummyStore::PathInfoAndContents) diff --git a/src/libstore/include/nix/store/dummy-store.hh b/src/libstore/include/nix/store/dummy-store.hh index d371c4e51..febf351c9 100644 --- a/src/libstore/include/nix/store/dummy-store.hh +++ b/src/libstore/include/nix/store/dummy-store.hh @@ -2,6 +2,7 @@ ///@file #include "nix/store/store-api.hh" +#include "nix/util/json-impls.hh" #include @@ -65,4 +66,33 @@ struct DummyStoreConfig : public std::enable_shared_from_this, } }; +template<> +struct json_avoids_null : std::true_type +{}; + +template<> +struct json_avoids_null> : std::true_type +{}; + +template<> +struct json_avoids_null : std::true_type +{}; + +template<> +struct json_avoids_null> : std::true_type +{}; + } // namespace nix + +namespace nlohmann { + +template<> +JSON_IMPL_INNER_TO(nix::DummyStoreConfig); +template<> +JSON_IMPL_INNER_FROM(nix::ref); +template<> +JSON_IMPL_INNER_TO(nix::DummyStore); +template<> +JSON_IMPL_INNER_FROM(nix::ref); + +} // namespace nlohmann From 3a9be9fd2fa6503697af06547785af99e08746b8 Mon Sep 17 00:00:00 2001 From: Sergei Zimmerman Date: Tue, 25 Nov 2025 01:10:35 +0300 Subject: [PATCH 22/40] libutil: Use openFileEnsureBeneathNoSymlinks in RestoreSink::createDirectory Starts using the new function. --- src/libutil-tests/file-system.cc | 13 +++++++++++-- src/libutil/fs-sink.cc | 3 ++- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/src/libutil-tests/file-system.cc b/src/libutil-tests/file-system.cc index b85d2613d..3a54ac55b 100644 --- a/src/libutil-tests/file-system.cc +++ b/src/libutil-tests/file-system.cc @@ -329,6 +329,7 @@ TEST(openFileEnsureBeneathNoSymlinks, works) { std::filesystem::path tmpDir = nix::createTempDir(); nix::AutoDelete delTmpDir(tmpDir, /*recursive=*/true); + using namespace nix::unix; { RestoreSink sink(/*startFsync=*/false); @@ -341,12 +342,19 @@ TEST(openFileEnsureBeneathNoSymlinks, works) sink.createSymlink(CanonPath("a/absolute_symlink"), tmpDir.string()); sink.createSymlink(CanonPath("a/relative_symlink"), "../."); sink.createSymlink(CanonPath("a/broken_symlink"), "./nonexistent"); + sink.createDirectory(CanonPath("a/b"), [](FileSystemObjectSink & dirSink, const CanonPath & relPath) { + dirSink.createDirectory(CanonPath("d")); + dirSink.createSymlink(CanonPath("c"), "./d"); + }); + sink.createDirectory(CanonPath("a/b/c/e")); // FIXME: This still follows symlinks + ASSERT_THROW( + sink.createDirectory( + CanonPath("a/b/c/f"), [](FileSystemObjectSink & dirSink, const CanonPath & relPath) {}), + SymlinkNotAllowed); } AutoCloseFD dirFd = openDirectory(tmpDir); - using namespace nix::unix; - auto open = [&](std::string_view path, int flags, mode_t mode = 0) { return openFileEnsureBeneathNoSymlinks(dirFd.get(), CanonPath(path), flags, mode); }; @@ -356,6 +364,7 @@ TEST(openFileEnsureBeneathNoSymlinks, works) EXPECT_THROW(open("a/absolute_symlink/a", O_RDONLY), SymlinkNotAllowed); EXPECT_THROW(open("a/absolute_symlink/c/d", O_RDONLY), SymlinkNotAllowed); EXPECT_THROW(open("a/relative_symlink/c", O_RDONLY), SymlinkNotAllowed); + EXPECT_THROW(open("a/b/c/d", O_RDONLY), SymlinkNotAllowed); EXPECT_EQ(open("a/broken_symlink", O_CREAT | O_WRONLY | O_EXCL, 0666), INVALID_DESCRIPTOR); /* Sanity check, no symlink shenanigans and behaves the same as regular openat with O_EXCL | O_CREAT. */ EXPECT_EQ(errno, EEXIST); diff --git a/src/libutil/fs-sink.cc b/src/libutil/fs-sink.cc index a78fe1af4..87bdaa339 100644 --- a/src/libutil/fs-sink.cc +++ b/src/libutil/fs-sink.cc @@ -84,7 +84,8 @@ void RestoreSink::createDirectory(const CanonPath & path, DirectoryCreatedCallba RestoreSink dirSink{startFsync}; dirSink.dstPath = append(dstPath, path); - dirSink.dirFd = ::openat(dirFd.get(), path.rel_c_str(), O_RDONLY | O_DIRECTORY | O_NOFOLLOW | O_CLOEXEC); + dirSink.dirFd = + unix::openFileEnsureBeneathNoSymlinks(dirFd.get(), path, O_RDONLY | O_DIRECTORY | O_NOFOLLOW | O_CLOEXEC); if (!dirSink.dirFd) throw SysError("opening directory '%s'", dirSink.dstPath.string()); From 84079e10cfde4e0187decdef98fd052bfa5976ef Mon Sep 17 00:00:00 2001 From: John Ericson Date: Wed, 18 Sep 2024 12:07:15 -0400 Subject: [PATCH 23/40] No more `Path` in `libnixcmd` Co-authored-by: Vinayak Goyal --- src/libcmd/common-eval-args.cc | 9 +++++---- src/libcmd/include/nix/cmd/command.hh | 4 ++-- .../include/nix/cmd/common-eval-args.hh | 2 +- .../include/nix/cmd/installable-value.hh | 2 +- src/libcmd/include/nix/cmd/repl.hh | 11 ++++++++++- src/libcmd/installables.cc | 18 ++++++++++-------- src/libcmd/repl.cc | 19 ++++++++++--------- src/libutil/args.cc | 4 ++-- src/libutil/include/nix/util/args.hh | 2 +- src/libutil/include/nix/util/args/root.hh | 4 ++-- src/nix/app.cc | 6 +++--- src/nix/formatter.cc | 4 ++-- src/nix/repl.cc | 2 +- src/nix/run.cc | 4 ++-- 14 files changed, 52 insertions(+), 39 deletions(-) diff --git a/src/libcmd/common-eval-args.cc b/src/libcmd/common-eval-args.cc index 00b09f4ea..30e76b245 100644 --- a/src/libcmd/common-eval-args.cc +++ b/src/libcmd/common-eval-args.cc @@ -165,7 +165,7 @@ Bindings * MixEvalArgs::getAutoArgs(EvalState & state) state.parseExprFromString( arg.expr, compatibilitySettings.nixShellShebangArgumentsRelativeToScript - ? state.rootPath(absPath(getCommandBaseDir())) + ? state.rootPath(absPath(getCommandBaseDir()).string()) : state.rootPath("."))); }, [&](const AutoArgString & arg) { v->mkString(arg.s, state.mem); }, @@ -177,7 +177,7 @@ Bindings * MixEvalArgs::getAutoArgs(EvalState & state) return res.finish(); } -SourcePath lookupFileArg(EvalState & state, std::string_view s, const Path * baseDir) +SourcePath lookupFileArg(EvalState & state, std::string_view s, const std::filesystem::path * baseDir) { if (EvalSettings::isPseudoUrl(s)) { auto accessor = fetchers::downloadTarball(*state.store, state.fetchSettings, EvalSettings::resolvePseudoUrl(s)); @@ -197,12 +197,13 @@ SourcePath lookupFileArg(EvalState & state, std::string_view s, const Path * bas } else if (s.size() > 2 && s.at(0) == '<' && s.at(s.size() - 1) == '>') { - Path p(s.substr(1, s.size() - 2)); + // Should perhaps be a `CanonPath`? + std::string p(s.substr(1, s.size() - 2)); return state.findFile(p); } else - return state.rootPath(baseDir ? absPath(s, *baseDir) : absPath(s)); + return state.rootPath(absPath(std::filesystem::path{s}, baseDir).string()); } } // namespace nix diff --git a/src/libcmd/include/nix/cmd/command.hh b/src/libcmd/include/nix/cmd/command.hh index 2f97b30da..d1b528e24 100644 --- a/src/libcmd/include/nix/cmd/command.hh +++ b/src/libcmd/include/nix/cmd/command.hh @@ -134,7 +134,7 @@ struct MixFlakeOptions : virtual Args, EvalCommand struct SourceExprCommand : virtual Args, MixFlakeOptions { - std::optional file; + std::optional file; std::optional expr; SourceExprCommand(); @@ -310,7 +310,7 @@ static RegisterCommand registerCommand2(std::vector && name) struct MixProfile : virtual StoreCommand { - std::optional profile; + std::optional profile; MixProfile(); diff --git a/src/libcmd/include/nix/cmd/common-eval-args.hh b/src/libcmd/include/nix/cmd/common-eval-args.hh index 62518ba0e..67cb07148 100644 --- a/src/libcmd/include/nix/cmd/common-eval-args.hh +++ b/src/libcmd/include/nix/cmd/common-eval-args.hh @@ -84,6 +84,6 @@ private: /** * @param baseDir Optional [base directory](https://nix.dev/manual/nix/development/glossary#gloss-base-directory) */ -SourcePath lookupFileArg(EvalState & state, std::string_view s, const Path * baseDir = nullptr); +SourcePath lookupFileArg(EvalState & state, std::string_view s, const std::filesystem::path * baseDir = nullptr); } // namespace nix diff --git a/src/libcmd/include/nix/cmd/installable-value.hh b/src/libcmd/include/nix/cmd/installable-value.hh index 3521a4154..27a1fb981 100644 --- a/src/libcmd/include/nix/cmd/installable-value.hh +++ b/src/libcmd/include/nix/cmd/installable-value.hh @@ -17,7 +17,7 @@ class AttrCursor; struct App { std::vector context; - Path program; + std::filesystem::path program; // FIXME: add args, sandbox settings, metadata, ... }; diff --git a/src/libcmd/include/nix/cmd/repl.hh b/src/libcmd/include/nix/cmd/repl.hh index a2c905f86..b72a9b7d1 100644 --- a/src/libcmd/include/nix/cmd/repl.hh +++ b/src/libcmd/include/nix/cmd/repl.hh @@ -19,7 +19,16 @@ struct AbstractNixRepl typedef std::vector> AnnotatedValues; - using RunNix = void(Path program, const Strings & args, const std::optional & input); + /** + * Run a nix executable + * + * @todo this is a layer violation + * + * @param programName Name of the command, e.g. `nix` or `nix-env`. + * @param args aguments to the command. + */ + using RunNix = + void(const std::string & programName, const Strings & args, const std::optional & input); /** * @param runNix Function to run the nix CLI to support various diff --git a/src/libcmd/installables.cc b/src/libcmd/installables.cc index 36777fb9c..2fa2280ea 100644 --- a/src/libcmd/installables.cc +++ b/src/libcmd/installables.cc @@ -132,7 +132,7 @@ MixFlakeOptions::MixFlakeOptions() lockFlags.writeLockFile = false; lockFlags.inputOverrides.insert_or_assign( flake::parseInputAttrPath(inputAttrPath), - parseFlakeRef(fetchSettings, flakeRef, absPath(getCommandBaseDir()), true)); + parseFlakeRef(fetchSettings, flakeRef, absPath(getCommandBaseDir()).string(), true)); }}, .completer = {[&](AddCompletions & completions, size_t n, std::string_view prefix) { if (n == 0) { @@ -173,7 +173,7 @@ MixFlakeOptions::MixFlakeOptions() auto flake = flake::lockFlake( flakeSettings, *evalState, - parseFlakeRef(fetchSettings, flakeRef, absPath(getCommandBaseDir())), + parseFlakeRef(fetchSettings, flakeRef, absPath(getCommandBaseDir()).string()), {.writeLockFile = false}); for (auto & [inputName, input] : flake.lockFile.root->inputs) { auto input2 = flake.lockFile.findInput({inputName}); // resolve 'follows' nodes @@ -263,7 +263,7 @@ void SourceExprCommand::completeInstallable(AddCompletions & completions, std::s evalSettings.pureEval = false; auto state = getEvalState(); - auto e = state->parseExprFromFile(resolveExprPath(lookupFileArg(*state, *file))); + auto e = state->parseExprFromFile(resolveExprPath(lookupFileArg(*state, file->string()))); Value root; state->eval(e, root); @@ -465,10 +465,10 @@ Installables SourceExprCommand::parseInstallables(ref store, std::vector< state->eval(e, *vFile); } else if (file) { auto dir = absPath(getCommandBaseDir()); - state->evalFile(lookupFileArg(*state, *file, &dir), *vFile); + state->evalFile(lookupFileArg(*state, file->string(), &dir), *vFile); } else { - Path dir = absPath(getCommandBaseDir()); - auto e = state->parseExprFromString(*expr, state->rootPath(dir)); + auto dir = absPath(getCommandBaseDir()); + auto e = state->parseExprFromString(*expr, state->rootPath(dir.string())); state->eval(e, *vFile); } @@ -801,7 +801,8 @@ std::vector RawInstallablesCommand::getFlakeRefsForCompletion() std::vector res; res.reserve(rawInstallables.size()); for (const auto & i : rawInstallables) - res.push_back(parseFlakeRefWithFragment(fetchSettings, expandTilde(i), absPath(getCommandBaseDir())).first); + res.push_back( + parseFlakeRefWithFragment(fetchSettings, expandTilde(i), absPath(getCommandBaseDir()).string()).first); return res; } @@ -820,7 +821,8 @@ void RawInstallablesCommand::run(ref store) std::vector InstallableCommand::getFlakeRefsForCompletion() { - return {parseFlakeRefWithFragment(fetchSettings, expandTilde(_installable), absPath(getCommandBaseDir())).first}; + return {parseFlakeRefWithFragment(fetchSettings, expandTilde(_installable), absPath(getCommandBaseDir()).string()) + .first}; } void InstallablesCommand::run(ref store, std::vector && rawInstallables) diff --git a/src/libcmd/repl.cc b/src/libcmd/repl.cc index 162f048af..38a0da0f8 100644 --- a/src/libcmd/repl.cc +++ b/src/libcmd/repl.cc @@ -58,8 +58,7 @@ struct NixRepl : AbstractNixRepl, detail::ReplCompleterMixin, gc { size_t debugTraceIndex; - // Arguments passed to :load, saved so they can be reloaded with :reload - Strings loadedFiles; + std::list loadedFiles; // Arguments passed to :load-flake, saved so they can be reloaded with :reload Strings loadedFlakes; std::function getValues; @@ -73,7 +72,7 @@ struct NixRepl : AbstractNixRepl, detail::ReplCompleterMixin, gc RunNix * runNixPtr; - void runNix(Path program, const Strings & args, const std::optional & input = {}); + void runNix(const std::string & program, const Strings & args, const std::optional & input = {}); std::unique_ptr interacter; @@ -92,7 +91,7 @@ struct NixRepl : AbstractNixRepl, detail::ReplCompleterMixin, gc StorePath getDerivationPath(Value & v); ProcessLineResult processLine(std::string line); - void loadFile(const Path & path); + void loadFile(const std::filesystem::path & path); void loadFlake(const std::string & flakeRef); void loadFiles(); void loadFlakes(); @@ -539,7 +538,9 @@ ProcessLineResult NixRepl::processLine(std::string line) Value v; evalString(arg, v); StorePath drvPath = getDerivationPath(v); - Path drvPathRaw = state->store->printStorePath(drvPath); + // N.B. This need not be a local / native file path. For + // example, we might be using an SSH store to a different OS. + std::string drvPathRaw = state->store->printStorePath(drvPath); if (command == ":b" || command == ":bl") { state->store->buildPaths({ @@ -712,12 +713,12 @@ ProcessLineResult NixRepl::processLine(std::string line) return ProcessLineResult::PromptAgain; } -void NixRepl::loadFile(const Path & path) +void NixRepl::loadFile(const std::filesystem::path & path) { loadedFiles.remove(path); loadedFiles.push_back(path); Value v, v2; - state->evalFile(lookupFileArg(*state, path), v); + state->evalFile(lookupFileArg(*state, path.string()), v); state->autoCallFunction(*autoArgs, v, v2); addAttrsToScope(v2); } @@ -790,7 +791,7 @@ void NixRepl::reloadFilesAndFlakes() void NixRepl::loadFiles() { - Strings old = loadedFiles; + decltype(loadedFiles) old = loadedFiles; loadedFiles.clear(); for (auto & i : old) { @@ -888,7 +889,7 @@ void NixRepl::evalString(std::string s, Value & v) state->forceValue(v, v.determinePos(noPos)); } -void NixRepl::runNix(Path program, const Strings & args, const std::optional & input) +void NixRepl::runNix(const std::string & program, const Strings & args, const std::optional & input) { if (runNixPtr) (*runNixPtr)(program, args, input); diff --git a/src/libutil/args.cc b/src/libutil/args.cc index 05b5a25c7..c6d450a0b 100644 --- a/src/libutil/args.cc +++ b/src/libutil/args.cc @@ -371,13 +371,13 @@ void RootArgs::parseCmdline(const Strings & _cmdline, bool allowShebang) d.completer(*completions, d.n, d.prefix); } -Path Args::getCommandBaseDir() const +std::filesystem::path Args::getCommandBaseDir() const { assert(parent); return parent->getCommandBaseDir(); } -Path RootArgs::getCommandBaseDir() const +std::filesystem::path RootArgs::getCommandBaseDir() const { return commandBaseDir; } diff --git a/src/libutil/include/nix/util/args.hh b/src/libutil/include/nix/util/args.hh index 99f6e23e8..d79341124 100644 --- a/src/libutil/include/nix/util/args.hh +++ b/src/libutil/include/nix/util/args.hh @@ -59,7 +59,7 @@ public: * * This only returns the correct value after parseCmdline() has run. */ - virtual Path getCommandBaseDir() const; + virtual std::filesystem::path getCommandBaseDir() const; protected: diff --git a/src/libutil/include/nix/util/args/root.hh b/src/libutil/include/nix/util/args/root.hh index 86b677be4..15919a7ac 100644 --- a/src/libutil/include/nix/util/args/root.hh +++ b/src/libutil/include/nix/util/args/root.hh @@ -38,7 +38,7 @@ protected: * * @see getCommandBaseDir() */ - Path commandBaseDir = "."; + std::filesystem::path commandBaseDir = "."; public: /** Parse the command line, throwing a UsageError if something goes @@ -48,7 +48,7 @@ public: std::shared_ptr completions; - Path getCommandBaseDir() const override; + std::filesystem::path getCommandBaseDir() const override; protected: diff --git a/src/nix/app.cc b/src/nix/app.cc index f1937bc23..634db04f3 100644 --- a/src/nix/app.cc +++ b/src/nix/app.cc @@ -140,9 +140,9 @@ App UnresolvedApp::resolve(ref evalStore, ref store) auto res = unresolved; auto builtContext = build(evalStore, store); - res.program = resolveString(*store, unresolved.program, builtContext); - if (!store->isInStore(res.program)) - throw Error("app program '%s' is not in the Nix store", res.program); + res.program = resolveString(*store, unresolved.program.string(), builtContext); + if (!store->isInStore(res.program.string())) + throw Error("app program '%s' is not in the Nix store", res.program.string()); return res; } diff --git a/src/nix/formatter.cc b/src/nix/formatter.cc index f5eb966d6..2c0b5c62b 100644 --- a/src/nix/formatter.cc +++ b/src/nix/formatter.cc @@ -84,7 +84,7 @@ struct CmdFormatterRun : MixFormatter, MixJSON assert(maybeFlakeDir.has_value()); auto flakeDir = maybeFlakeDir.value(); - Strings programArgs{app.program}; + Strings programArgs{app.program.string()}; // Propagate arguments from the CLI for (auto & i : args) { @@ -103,7 +103,7 @@ struct CmdFormatterRun : MixFormatter, MixJSON execProgramInStore( store, UseLookupPath::DontUse, - app.program, + app.program.string(), programArgs, std::nullopt, // Use default system env); diff --git a/src/nix/repl.cc b/src/nix/repl.cc index 5dd53e932..19f02e759 100644 --- a/src/nix/repl.cc +++ b/src/nix/repl.cc @@ -11,7 +11,7 @@ namespace nix { -void runNix(Path program, const Strings & args, const std::optional & input = {}) +void runNix(const std::string & program, const Strings & args, const std::optional & input = {}) { auto subprocessEnv = getEnv(); subprocessEnv["NIX_CONFIG"] = globalConfig.toKeyValue(); diff --git a/src/nix/run.cc b/src/nix/run.cc index 368a5ed57..324b736a6 100644 --- a/src/nix/run.cc +++ b/src/nix/run.cc @@ -160,7 +160,7 @@ struct CmdRun : InstallableValueCommand, MixEnvironment lockFlags.applyNixConfig = true; auto app = installable->toApp(*state).resolve(getEvalStore(), store); - Strings allArgs{app.program}; + Strings allArgs{app.program.string()}; for (auto & i : args) allArgs.push_back(i); @@ -170,7 +170,7 @@ struct CmdRun : InstallableValueCommand, MixEnvironment setEnviron(); - execProgramInStore(store, UseLookupPath::DontUse, app.program, allArgs); + execProgramInStore(store, UseLookupPath::DontUse, app.program.string(), allArgs); } }; From d7b6afecdb8b8ed4f714c15c55833e82b96b443d Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Tue, 25 Nov 2025 13:29:29 +0100 Subject: [PATCH 24/40] LambdaSink: Allow passing a destructor callback --- src/libutil/include/nix/util/serialise.hh | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/src/libutil/include/nix/util/serialise.hh b/src/libutil/include/nix/util/serialise.hh index 09b33bf95..5db55c60d 100644 --- a/src/libutil/include/nix/util/serialise.hh +++ b/src/libutil/include/nix/util/serialise.hh @@ -447,18 +447,27 @@ struct LengthSource : Source */ struct LambdaSink : Sink { - typedef std::function lambda_t; + typedef std::function data_t; + typedef std::function cleanup_t; - lambda_t lambda; + data_t dataFun; + cleanup_t cleanupFun; - LambdaSink(const lambda_t & lambda) - : lambda(lambda) + LambdaSink( + const data_t & dataFun, const cleanup_t & cleanupFun = []() {}) + : dataFun(dataFun) + , cleanupFun(cleanupFun) { } + ~LambdaSink() + { + cleanupFun(); + } + void operator()(std::string_view data) override { - lambda(data); + dataFun(data); } }; From 7ba84437be28ef6bd7ada581e0edd0c4199f9d0c Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Tue, 25 Nov 2025 14:19:32 +0100 Subject: [PATCH 25/40] BinaryCacheStore::narFromPath(): Fix unreachable code When this function is called as a coroutine (e.g. when it's called by `copyStorePath()`), the code after `decompressor->finish()` is never reached because the coroutine is destroyed when the caller reaches the end of the NAR. So put that code in a `LambdaSink` destructor. --- src/libstore/binary-cache-store.cc | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/src/libstore/binary-cache-store.cc b/src/libstore/binary-cache-store.cc index caae72479..e1f1d24c6 100644 --- a/src/libstore/binary-cache-store.cc +++ b/src/libstore/binary-cache-store.cc @@ -418,10 +418,20 @@ void BinaryCacheStore::narFromPath(const StorePath & storePath, Sink & sink) { auto info = queryPathInfo(storePath).cast(); - LengthSink narSize; - TeeSink tee{sink, narSize}; + uint64_t narSize = 0; - auto decompressor = makeDecompressionSink(info->compression, tee); + LambdaSink uncompressedSink{ + [&](std::string_view data) { + narSize += data.size(); + sink(data); + }, + [&]() { + stats.narRead++; + // stats.narReadCompressedBytes += nar->size(); // FIXME + stats.narReadBytes += narSize; + }}; + + auto decompressor = makeDecompressionSink(info->compression, uncompressedSink); try { getFile(info->url, *decompressor); @@ -431,9 +441,7 @@ void BinaryCacheStore::narFromPath(const StorePath & storePath, Sink & sink) decompressor->finish(); - stats.narRead++; - // stats.narReadCompressedBytes += nar->size(); // FIXME - stats.narReadBytes += narSize.length; + // Note: don't do anything here because it's never reached if we're called as a coroutine. } void BinaryCacheStore::queryPathInfoUncached( From ab58d2720c3ed10ec09af1f36f9f8216fdc71130 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Tue, 25 Nov 2025 11:11:55 -0500 Subject: [PATCH 26/40] Make `nix-shell.sh` functional test debuggable Without this change, when one runs wit with `meson test --interactive`, that command will block waiting on standard input to be closed. --- tests/functional/nix-shell.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/functional/nix-shell.sh b/tests/functional/nix-shell.sh index cf650e2c3..562cc252e 100755 --- a/tests/functional/nix-shell.sh +++ b/tests/functional/nix-shell.sh @@ -175,7 +175,7 @@ cat >"$TEST_ROOT"/marco/polo/default.nix < Date: Tue, 25 Nov 2025 10:44:32 -0500 Subject: [PATCH 27/40] Test `nix develop` on fixed-output derivations It half works today, we should fix this but also not regress it! --- tests/functional/nix-shell.sh | 6 ++++++ tests/functional/shell.nix | 10 ++++++++++ 2 files changed, 16 insertions(+) diff --git a/tests/functional/nix-shell.sh b/tests/functional/nix-shell.sh index 562cc252e..2ff994c99 100755 --- a/tests/functional/nix-shell.sh +++ b/tests/functional/nix-shell.sh @@ -279,3 +279,9 @@ assert (!(args ? inNixShell)); (import $shellDotNix { }).shellDrv EOF nix-shell "$TEST_ROOT"/shell-ellipsis.nix --run "true" + +if [[ -z ${NIX_TESTS_CA_BY_DEFAULT:-} ]]; then + # `nix develop` should also work with fixed-output derivations + # shellcheck disable=SC2016 + nix develop -f "$shellDotNix" fixed -c bash -c '[[ -n $stdenv ]]' +fi diff --git a/tests/functional/shell.nix b/tests/functional/shell.nix index 5e9f48818..267b0c8f0 100644 --- a/tests/functional/shell.nix +++ b/tests/functional/shell.nix @@ -84,6 +84,16 @@ let ''; }; + # Shells should also work with fixed-output derivations + fixed = mkDerivation { + name = "fixed"; + FOO = "was a fixed-output derivation"; + outputHash = "1ixr6yd3297ciyp9im522dfxpqbkhcw0pylkb2aab915278fqaik"; + outputHashMode = "recursive"; + outputHashAlgo = "sha256"; + outputs = [ "out" ]; + }; + # Used by nix-shell -p runCommand = name: args: buildCommand: From 801cb161319148bca4c6d4e9daea0ccea1953b2e Mon Sep 17 00:00:00 2001 From: John Ericson Date: Mon, 24 Nov 2025 23:27:00 -0500 Subject: [PATCH 28/40] Simplify `nix develop` "gathering derivation environment" Before, had some funny logic with an unnecessary is CA enabled branch, and erroneous use of the comma operator. Now, take advantage of the new `Derivation::fillInOutputPaths` to fill in input addresses (and output path env vars) in a much-more lightweight manner. Also, fix `nix develop` on fixed-output derivations so that weird things don't happen when we have that experimental feature enabled. As a slight behavior change, if the original derivation was content-addressing this one will be too, but I really don't think that matters --- if anything, it is a slight improvement for users that have already opted into content-addressing anyways. --- src/nix/develop.cc | 36 +++++++++++++++++------------------ tests/functional/nix-shell.sh | 8 +++----- 2 files changed, 20 insertions(+), 24 deletions(-) diff --git a/src/nix/develop.cc b/src/nix/develop.cc index 1eff735da..68ff3fcf9 100644 --- a/src/nix/develop.cc +++ b/src/nix/develop.cc @@ -272,26 +272,24 @@ static StorePath getDerivationEnvironment(ref store, ref evalStore drv.name += "-env"; drv.env.emplace("name", drv.name); drv.inputSrcs.insert(std::move(getEnvShPath)); - if (experimentalFeatureSettings.isEnabled(Xp::CaDerivations)) { - for (auto & output : drv.outputs) { - output.second = DerivationOutput::Deferred{}, drv.env[output.first] = hashPlaceholder(output.first); - } - } else { - for (auto & output : drv.outputs) { - output.second = DerivationOutput::Deferred{}; - drv.env[output.first] = ""; - } - auto hashesModulo = hashDerivationModulo(*evalStore, drv, true); - - for (auto & output : drv.outputs) { - Hash h = hashesModulo.hashes.at(output.first); - auto outPath = store->makeOutputPath(output.first, h, drv.name); - output.second = DerivationOutput::InputAddressed{ - .path = outPath, - }; - drv.env[output.first] = store->printStorePath(outPath); - } + for (auto & [outputName, output] : drv.outputs) { + std::visit( + overloaded{ + [&](const DerivationOutput::InputAddressed &) { + output = DerivationOutput::Deferred{}; + drv.env[outputName] = ""; + }, + [&](const DerivationOutput::CAFixed &) { + output = DerivationOutput::Deferred{}; + drv.env[outputName] = ""; + }, + [&](const auto &) { + // Do nothing for other types (CAFloating, Deferred, Impure) + }, + }, + output.raw); } + drv.fillInOutputPaths(*evalStore); auto shellDrvPath = writeDerivation(*evalStore, drv); diff --git a/tests/functional/nix-shell.sh b/tests/functional/nix-shell.sh index 2ff994c99..418857987 100755 --- a/tests/functional/nix-shell.sh +++ b/tests/functional/nix-shell.sh @@ -280,8 +280,6 @@ assert (!(args ? inNixShell)); EOF nix-shell "$TEST_ROOT"/shell-ellipsis.nix --run "true" -if [[ -z ${NIX_TESTS_CA_BY_DEFAULT:-} ]]; then - # `nix develop` should also work with fixed-output derivations - # shellcheck disable=SC2016 - nix develop -f "$shellDotNix" fixed -c bash -c '[[ -n $stdenv ]]' -fi +# `nix develop` should also work with fixed-output derivations +# shellcheck disable=SC2016 +nix develop -f "$shellDotNix" fixed -c bash -c '[[ -n $stdenv ]]' From 6f33f64ce5764ada8e53275e9fc01cac63dbed45 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Mon, 24 Nov 2025 18:25:43 -0500 Subject: [PATCH 29/40] C API: Need to try-catch around `new` Per https://en.cppreference.com/w/cpp/memory/new/operator_new.html, it can throw if the allocation fails. --- src/libstore-c/nix_api_store.cc | 6 +++++- src/libutil-c/nix_api_util.cc | 6 +++++- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/src/libstore-c/nix_api_store.cc b/src/libstore-c/nix_api_store.cc index b95e5b749..0869fc91d 100644 --- a/src/libstore-c/nix_api_store.cc +++ b/src/libstore-c/nix_api_store.cc @@ -215,7 +215,11 @@ void nix_derivation_free(nix_derivation * drv) StorePath * nix_store_path_clone(const StorePath * p) { - return new StorePath{p->path}; + try { + return new StorePath{p->path}; + } catch (...) { + return nullptr; + } } nix_derivation * nix_derivation_from_json(nix_c_context * context, Store * store, const char * json) diff --git a/src/libutil-c/nix_api_util.cc b/src/libutil-c/nix_api_util.cc index a2f7710bc..f28a9168e 100644 --- a/src/libutil-c/nix_api_util.cc +++ b/src/libutil-c/nix_api_util.cc @@ -13,7 +13,11 @@ extern "C" { nix_c_context * nix_c_context_create() { - return new nix_c_context(); + try { + return new nix_c_context(); + } catch (...) { + return nullptr; + } } void nix_c_context_free(nix_c_context * context) From 1c10ce6047f0a338b01c4ade0a542275696e8b13 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Mon, 17 Nov 2025 12:14:39 -0500 Subject: [PATCH 30/40] libstore-c: Add new derivation and store path functions Add several new functions to the C API: StorePath operations: - nix_store_path_hash: Extract the hash part from a store path - nix_store_create_from_parts: Construct a store path from hash and name Derivation operations: - nix_derivation_clone: Clone a derivation - nix_derivation_to_json: Serialize a derivation to JSON Store operations: - nix_store_drv_from_store_path: Load a derivation from a store path Test the new functions, and improve documentation of some existing functions to better distinguish them, also. Co-authored-by: Tristan Ross Co-authored-by: Robert Hensing --- src/libstore-c/nix_api_store.cc | 82 +++++++++++ src/libstore-c/nix_api_store.h | 19 ++- src/libstore-c/nix_api_store/derivation.h | 19 +++ src/libstore-c/nix_api_store/store_path.h | 42 ++++++ src/libstore-tests/nix_api_store.cc | 159 ++++++++++++++++++++++ 5 files changed, 320 insertions(+), 1 deletion(-) diff --git a/src/libstore-c/nix_api_store.cc b/src/libstore-c/nix_api_store.cc index 0869fc91d..4f71d0a3c 100644 --- a/src/libstore-c/nix_api_store.cc +++ b/src/libstore-c/nix_api_store.cc @@ -1,3 +1,6 @@ +#include +#include + #include "nix_api_store.h" #include "nix_api_store_internal.h" #include "nix_api_util.h" @@ -8,6 +11,7 @@ #include "nix/store/store-open.hh" #include "nix/store/build-result.hh" #include "nix/store/local-fs-store.hh" +#include "nix/util/base-nix-32.hh" #include "nix/store/globals.hh" @@ -222,6 +226,60 @@ StorePath * nix_store_path_clone(const StorePath * p) } } +} // extern "C" + +template +static auto to_cpp_array(const uint8_t (&r)[S]) +{ + return reinterpret_cast &>(r); +} + +extern "C" { + +nix_err +nix_store_path_hash(nix_c_context * context, const StorePath * store_path, nix_store_path_hash_part * hash_part_out) +{ + try { + auto hashPart = store_path->path.hashPart(); + // Decode from Nix32 (base32) encoding to raw bytes + auto decoded = nix::BaseNix32::decode(hashPart); + + assert(decoded.size() == sizeof(hash_part_out->bytes)); + std::memcpy(hash_part_out->bytes, decoded.data(), sizeof(hash_part_out->bytes)); + return NIX_OK; + } + NIXC_CATCH_ERRS +} + +StorePath * nix_store_create_from_parts( + nix_c_context * context, const nix_store_path_hash_part * hash, const char * name, size_t name_len) +{ + if (context) + context->last_err_code = NIX_OK; + try { + // Encode the 20 raw bytes to Nix32 (base32) format + auto hashStr = nix::BaseNix32::encode(std::span{to_cpp_array(hash->bytes)}); + + // Construct the store path basename: - + std::string baseName; + baseName += hashStr; + baseName += "-"; + baseName += std::string_view{name, name_len}; + + return new StorePath{nix::StorePath(std::move(baseName))}; + } + NIXC_CATCH_ERRS_NULL +} + +nix_derivation * nix_derivation_clone(const nix_derivation * d) +{ + try { + return new nix_derivation{d->drv}; + } catch (...) { + return nullptr; + } +} + nix_derivation * nix_derivation_from_json(nix_c_context * context, Store * store, const char * json) { if (context) @@ -232,6 +290,20 @@ nix_derivation * nix_derivation_from_json(nix_c_context * context, Store * store NIXC_CATCH_ERRS_NULL } +nix_err nix_derivation_to_json( + nix_c_context * context, const nix_derivation * drv, nix_get_string_callback callback, void * userdata) +{ + if (context) + context->last_err_code = NIX_OK; + try { + auto result = static_cast(drv->drv).dump(); + if (callback) { + callback(result.data(), result.size(), userdata); + } + } + NIXC_CATCH_ERRS +} + StorePath * nix_add_derivation(nix_c_context * context, Store * store, nix_derivation * derivation) { if (context) @@ -256,4 +328,14 @@ nix_err nix_store_copy_closure(nix_c_context * context, Store * srcStore, Store NIXC_CATCH_ERRS } +nix_derivation * nix_store_drv_from_store_path(nix_c_context * context, Store * store, const StorePath * path) +{ + if (context) + context->last_err_code = NIX_OK; + try { + return new nix_derivation{store->ptr->derivationFromPath(path->path)}; + } + NIXC_CATCH_ERRS_NULL +} + } // extern "C" diff --git a/src/libstore-c/nix_api_store.h b/src/libstore-c/nix_api_store.h index 9eaa61a92..761fdf3c8 100644 --- a/src/libstore-c/nix_api_store.h +++ b/src/libstore-c/nix_api_store.h @@ -106,7 +106,7 @@ nix_err nix_store_get_storedir(nix_c_context * context, Store * store, nix_get_string_callback callback, void * user_data); /** - * @brief Parse a Nix store path into a StorePath + * @brief Parse a Nix store path that includes the store dir into a StorePath * * @note Don't forget to free this path using nix_store_path_free()! * @param[out] context Optional, stores error information @@ -188,9 +188,16 @@ nix_store_get_version(nix_c_context * context, Store * store, nix_get_string_cal /** * @brief Create a `nix_derivation` from a JSON representation of that derivation. * + * @note Unlike `nix_derivation_to_json`, this needs a `Store`. This is because + * over time we expect the internal representation of derivations in Nix to + * differ from accepted derivation formats. The store argument is here to help + * any logic needed to convert from JSON to the internal representation, in + * excess of just parsing. + * * @param[out] context Optional, stores error information. * @param[in] store nix store reference. * @param[in] json JSON of the derivation as a string. + * @return A new derivation, or NULL on error. Free with `nix_derivation_free` when done using the `nix_derivation`. */ nix_derivation * nix_derivation_from_json(nix_c_context * context, Store * store, const char * json); @@ -242,6 +249,16 @@ nix_err nix_store_get_fs_closure( void * userdata, void (*callback)(nix_c_context * context, void * userdata, const StorePath * store_path)); +/** + * @brief Returns the derivation associated with the store path + * + * @param[out] context Optional, stores error information + * @param[in] store The nix store + * @param[in] path The nix store path + * @return A new derivation, or NULL on error. Free with `nix_derivation_free` when done using the `nix_derivation`. + */ +nix_derivation * nix_store_drv_from_store_path(nix_c_context * context, Store * store, const StorePath * path); + // cffi end #ifdef __cplusplus } diff --git a/src/libstore-c/nix_api_store/derivation.h b/src/libstore-c/nix_api_store/derivation.h index 9c42cfd60..239ffd52f 100644 --- a/src/libstore-c/nix_api_store/derivation.h +++ b/src/libstore-c/nix_api_store/derivation.h @@ -20,6 +20,14 @@ extern "C" { /** @brief Nix Derivation */ typedef struct nix_derivation nix_derivation; +/** + * @brief Copy a `nix_derivation` + * + * @param[in] d the derivation to copy + * @return a new `nix_derivation` + */ +nix_derivation * nix_derivation_clone(const nix_derivation * d); + /** * @brief Deallocate a `nix_derivation` * @@ -28,6 +36,17 @@ typedef struct nix_derivation nix_derivation; */ void nix_derivation_free(nix_derivation * drv); +/** + * @brief Gets the derivation as a JSON string + * + * @param[out] context Optional, stores error information + * @param[in] drv The derivation + * @param[in] callback Called with the JSON string + * @param[in] userdata Arbitrary data passed to the callback + */ +nix_err nix_derivation_to_json( + nix_c_context * context, const nix_derivation * drv, nix_get_string_callback callback, void * userdata); + // cffi end #ifdef __cplusplus } diff --git a/src/libstore-c/nix_api_store/store_path.h b/src/libstore-c/nix_api_store/store_path.h index 9f3717aea..1aa9bcac7 100644 --- a/src/libstore-c/nix_api_store/store_path.h +++ b/src/libstore-c/nix_api_store/store_path.h @@ -10,6 +10,9 @@ * @brief Store path operations */ +#include +#include + #include "nix_api_util.h" #ifdef __cplusplus @@ -44,6 +47,45 @@ void nix_store_path_free(StorePath * p); */ void nix_store_path_name(const StorePath * store_path, nix_get_string_callback callback, void * user_data); +/** + * @brief A store path hash + * + * Once decoded from "nix32" encoding, a store path hash is 20 raw bytes. + */ +typedef struct nix_store_path_hash_part +{ + uint8_t bytes[20]; +} nix_store_path_hash_part; + +/** + * @brief Get the path hash (e.g. "" in /nix/store/-) + * + * The hash is returned as raw bytes, decoded from "nix32" encoding. + * + * @param[out] context Optional, stores error information + * @param[in] store_path the path to get the hash from + * @param[out] hash_part_out the decoded hash as 20 raw bytes + * @return NIX_OK on success, error code on failure + */ +nix_err +nix_store_path_hash(nix_c_context * context, const StorePath * store_path, nix_store_path_hash_part * hash_part_out); + +/** + * @brief Create a StorePath from its constituent parts (hash and name) + * + * This function constructs a store path from a hash and name, without needing + * a Store reference or the store directory prefix. + * + * @note Don't forget to free this path using nix_store_path_free()! + * @param[out] context Optional, stores error information + * @param[in] hash The store path hash (20 raw bytes) + * @param[in] name The store path name (the part after the hash) + * @param[in] name_len Length of the name string + * @return owned store path, NULL on error + */ +StorePath * nix_store_create_from_parts( + nix_c_context * context, const nix_store_path_hash_part * hash, const char name[/*name_len*/], size_t name_len); + // cffi end #ifdef __cplusplus } diff --git a/src/libstore-tests/nix_api_store.cc b/src/libstore-tests/nix_api_store.cc index bf411053a..a7fa4d8d8 100644 --- a/src/libstore-tests/nix_api_store.cc +++ b/src/libstore-tests/nix_api_store.cc @@ -1,5 +1,7 @@ #include +#include + #include "nix_api_util.h" #include "nix_api_store.h" @@ -92,6 +94,70 @@ TEST_F(nix_api_store_test, DoesNotCrashWhenContextIsNull) nix_store_path_free(path); } +// Verify it's 20 bytes +static_assert(sizeof(nix_store_path_hash_part::bytes) == 20); +static_assert(sizeof(nix_store_path_hash_part::bytes) == sizeof(nix_store_path_hash_part)); + +TEST_F(nix_api_store_test, nix_store_path_hash) +{ + StorePath * path = nix_store_parse_path(ctx, store, (nixStoreDir + PATH_SUFFIX).c_str()); + ASSERT_NE(path, nullptr); + + nix_store_path_hash_part hash; + auto ret = nix_store_path_hash(ctx, path, &hash); + assert_ctx_ok(); + ASSERT_EQ(ret, NIX_OK); + + // The hash should be non-zero + bool allZero = true; + for (size_t i = 0; i < sizeof(hash.bytes); i++) { + if (hash.bytes[i] != 0) { + allZero = false; + break; + } + } + ASSERT_FALSE(allZero); + + nix_store_path_free(path); +} + +TEST_F(nix_api_store_test, nix_store_create_from_parts_roundtrip) +{ + // Parse a path + StorePath * original = nix_store_parse_path(ctx, store, (nixStoreDir + PATH_SUFFIX).c_str()); + EXPECT_NE(original, nullptr); + + // Get its hash + nix_store_path_hash_part hash; + auto ret = nix_store_path_hash(ctx, original, &hash); + assert_ctx_ok(); + ASSERT_EQ(ret, NIX_OK); + + // Get its name + std::string name; + nix_store_path_name(original, OBSERVE_STRING(name)); + + // Reconstruct from parts + StorePath * reconstructed = nix_store_create_from_parts(ctx, &hash, name.c_str(), name.size()); + assert_ctx_ok(); + ASSERT_NE(reconstructed, nullptr); + + // Should be equal + EXPECT_EQ(original->path, reconstructed->path); + + nix_store_path_free(original); + nix_store_path_free(reconstructed); +} + +TEST_F(nix_api_store_test, nix_store_create_from_parts_invalid_name) +{ + nix_store_path_hash_part hash = {}; + // Invalid name with spaces + StorePath * path = nix_store_create_from_parts(ctx, &hash, "invalid name", 12); + ASSERT_EQ(path, nullptr); + ASSERT_EQ(nix_err_code(ctx), NIX_ERR_NIX_ERROR); +} + TEST_F(nix_api_store_test, get_version) { std::string str; @@ -795,4 +861,97 @@ TEST_F(NixApiStoreTestWithRealisedPath, nix_store_get_fs_closure_error_propagati ASSERT_EQ(call_count, 1); // Should have been called exactly once, then aborted } +/** + * @brief Helper function to load JSON from a test data file + * + * @param filename Relative path from _NIX_TEST_UNIT_DATA + * @return JSON string contents of the file + */ +static std::string load_json_from_test_data(const char * filename) +{ + std::filesystem::path unitTestData{getenv("_NIX_TEST_UNIT_DATA")}; + std::ifstream t{unitTestData / filename}; + std::stringstream buffer; + buffer << t.rdbuf(); + return buffer.str(); +} + +TEST_F(nix_api_store_test, nix_derivation_to_json_roundtrip) +{ + // Load JSON from test data + auto originalJson = load_json_from_test_data("derivation/invariants/filled-in-deferred-empty-env-var-pre.json"); + + // Parse to derivation + auto * drv = nix_derivation_from_json(ctx, store, originalJson.c_str()); + assert_ctx_ok(); + ASSERT_NE(drv, nullptr); + + // Convert back to JSON + std::string convertedJson; + auto ret = nix_derivation_to_json(ctx, drv, OBSERVE_STRING(convertedJson)); + assert_ctx_ok(); + ASSERT_EQ(ret, NIX_OK); + ASSERT_FALSE(convertedJson.empty()); + + // Parse both JSON strings to compare (ignoring whitespace differences) + auto originalParsed = nlohmann::json::parse(originalJson); + auto convertedParsed = nlohmann::json::parse(convertedJson); + + // Remove parts that will be different due to filling-in. + originalParsed.at("outputs").erase("out"); + originalParsed.at("env").erase("out"); + convertedParsed.at("outputs").erase("out"); + convertedParsed.at("env").erase("out"); + + // They should be equivalent + ASSERT_EQ(originalParsed, convertedParsed); + + nix_derivation_free(drv); +} + +TEST_F(nix_api_store_test, nix_derivation_store_round_trip) +{ + // Load a derivation from JSON + auto json = load_json_from_test_data("derivation/invariants/filled-in-deferred-empty-env-var-pre.json"); + auto * drv = nix_derivation_from_json(ctx, store, json.c_str()); + assert_ctx_ok(); + ASSERT_NE(drv, nullptr); + + // Add to store + auto * drvPath = nix_add_derivation(ctx, store, drv); + assert_ctx_ok(); + ASSERT_NE(drvPath, nullptr); + + // Retrieve from store + auto * drv2 = nix_store_drv_from_store_path(ctx, store, drvPath); + assert_ctx_ok(); + ASSERT_NE(drv2, nullptr); + + // The round trip should make the same derivation + ASSERT_EQ(drv->drv, drv2->drv); + + nix_store_path_free(drvPath); + nix_derivation_free(drv); + nix_derivation_free(drv2); +} + +TEST_F(nix_api_store_test, nix_derivation_clone) +{ + // Load a derivation from JSON + auto json = load_json_from_test_data("derivation/invariants/filled-in-deferred-empty-env-var-pre.json"); + auto * drv = nix_derivation_from_json(ctx, store, json.c_str()); + assert_ctx_ok(); + ASSERT_NE(drv, nullptr); + + // Clone the derivation + auto * drv2 = nix_derivation_clone(drv); + ASSERT_NE(drv2, nullptr); + + // The clone should be equal + ASSERT_EQ(drv->drv, drv2->drv); + + nix_derivation_free(drv); + nix_derivation_free(drv2); +} + } // namespace nixC From 6a4a1e9f724a4e9c928f379efeb85125dcc3dc07 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Tue, 25 Nov 2025 13:35:03 -0500 Subject: [PATCH 31/40] Skip new part of functional test on NixOS It's very weird it doesn't work here, but I don't mind not debugging this now as I just added this part of the functional test --- it's already better than it was before. --- tests/functional/nix-shell.sh | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/tests/functional/nix-shell.sh b/tests/functional/nix-shell.sh index 418857987..cdeea3268 100755 --- a/tests/functional/nix-shell.sh +++ b/tests/functional/nix-shell.sh @@ -280,6 +280,9 @@ assert (!(args ? inNixShell)); EOF nix-shell "$TEST_ROOT"/shell-ellipsis.nix --run "true" -# `nix develop` should also work with fixed-output derivations -# shellcheck disable=SC2016 -nix develop -f "$shellDotNix" fixed -c bash -c '[[ -n $stdenv ]]' +# FIXME unclear why this (newly made) test is failing in this case. +if ! isTestOnNixOS; then + # `nix develop` should also work with fixed-output derivations + # shellcheck disable=SC2016 + nix develop -f "$shellDotNix" fixed -c bash -c '[[ -n $stdenv ]]' +fi From 97abcda9cc8db9218fe679969fa52efaac9c086d Mon Sep 17 00:00:00 2001 From: Taeer Bar-Yam Date: Tue, 25 Nov 2025 17:35:35 +0100 Subject: [PATCH 32/40] parser.y: correctly abstract over to-be-constructed ExprString Fixes the regression from eab467ecfb829182548276df7d56a4d1c525057a with dynamic attributes that a simple string expressions. Co-authored-by: Sergei Zimmerman --- src/libexpr/include/nix/expr/parser-state.hh | 73 ++++++++++++++++++++ src/libexpr/parser.y | 43 +++++------- 2 files changed, 92 insertions(+), 24 deletions(-) diff --git a/src/libexpr/include/nix/expr/parser-state.hh b/src/libexpr/include/nix/expr/parser-state.hh index c2a49a3d3..5a94f62e8 100644 --- a/src/libexpr/include/nix/expr/parser-state.hh +++ b/src/libexpr/include/nix/expr/parser-state.hh @@ -47,6 +47,79 @@ struct ParserLocation } }; +/** + * This represents a string-like parse that possibly has yet to be constructed. + * + * Examples: + * "foo" + * ${"foo" + "bar"} + * "foo.bar" + * "foo-${a}" + * + * Using this type allows us to avoid construction altogether in cases where what we actually need is the string + * contents. For example in foo."bar.baz", there is no need to construct an AST node for "bar.baz", but we don't know + * that until we bubble the value up during parsing and see that it's a node in an AttrPath. + */ +class ToBeStringyExpr +{ +private: + using Raw = std::variant; + Raw raw; + +public: + ToBeStringyExpr() = default; + + ToBeStringyExpr(std::string_view v) + : raw(v) + { + } + + ToBeStringyExpr(Expr * expr) + : raw(expr) + { + assert(expr); + } + + /** + * Visits the expression and invokes an overloaded functor object \ref f. + * If the underlying Expr has a dynamic type of ExprString the overload taking std::string_view + * is invoked. + * + * Used to consistently handle simple StringExpr ${"string"} as non-dynamic attributes. + * @see https://github.com/NixOS/nix/issues/14642 + */ + template + void visit(F && f) + { + std::visit( + overloaded{ + [&](std::string_view str) { f(str); }, + [&](Expr * expr) { + ExprString * str = dynamic_cast(expr); + if (str) + f(str->v.string_view()); + else + f(expr); + }, + [](std::monostate) { unreachable(); }}, + raw); + } + + /** + * Get or create an Expr from either an existing Expr or from a string. + * Delays the allocation or an AST node in case the parser only cares about string contents. + */ + Expr * toExpr(Exprs & exprs) + { + return std::visit( + overloaded{ + [&](std::string_view str) -> Expr * { return exprs.add(exprs.alloc, str); }, + [&](Expr * expr) { return expr; }, + [](std::monostate) -> Expr * { unreachable(); }}, + raw); + } +}; + struct LexerState { /** diff --git a/src/libexpr/parser.y b/src/libexpr/parser.y index a9166c5b5..520086e28 100644 --- a/src/libexpr/parser.y +++ b/src/libexpr/parser.y @@ -138,7 +138,7 @@ static Expr * makeCall(Exprs & exprs, PosIdx pos, Expr * fn, Expr * arg) { %type >> string_parts_interpolated %type >>> ind_string_parts %type path_start -%type > string_parts string_attr +%type string_parts string_attr %type attr %token ID %token STR IND_STR @@ -297,12 +297,7 @@ expr_simple } | INT_LIT { $$ = state->exprs.add($1); } | FLOAT_LIT { $$ = state->exprs.add($1); } - | '"' string_parts '"' { - std::visit(overloaded{ - [&](std::string_view str) { $$ = state->exprs.add(state->exprs.alloc, str); }, - [&](Expr * expr) { $$ = expr; }}, - $2); - } + | '"' string_parts '"' { $$ = $2.toExpr(state->exprs); } | IND_STRING_OPEN ind_string_parts IND_STRING_CLOSE { $$ = state->stripIndentation(CUR_POS, $2); } @@ -342,9 +337,9 @@ expr_simple ; string_parts - : STR { $$ = $1; } - | string_parts_interpolated { $$ = state->exprs.add(state->exprs.alloc, CUR_POS, true, $1); } - | { $$ = std::string_view(); } + : STR { $$ = {$1}; } + | string_parts_interpolated { $$ = {state->exprs.add(state->exprs.alloc, CUR_POS, true, $1)}; } + | { $$ = {std::string_view()}; } ; string_parts_interpolated @@ -447,15 +442,15 @@ attrs : attrs attr { $$ = std::move($1); $$.emplace_back(state->symbols.create($2), state->at(@2)); } | attrs string_attr { $$ = std::move($1); - std::visit(overloaded { + $2.visit(overloaded{ [&](std::string_view str) { $$.emplace_back(state->symbols.create(str), state->at(@2)); }, [&](Expr * expr) { - throw ParseError({ - .msg = HintFmt("dynamic attributes not allowed in inherit"), - .pos = state->positions[state->at(@2)] - }); - } - }, $2); + throw ParseError({ + .msg = HintFmt("dynamic attributes not allowed in inherit"), + .pos = state->positions[state->at(@2)] + }); + }} + ); } | { } ; @@ -464,17 +459,17 @@ attrpath : attrpath '.' attr { $$ = std::move($1); $$.emplace_back(state->symbols.create($3)); } | attrpath '.' string_attr { $$ = std::move($1); - std::visit(overloaded { + $3.visit(overloaded{ [&](std::string_view str) { $$.emplace_back(state->symbols.create(str)); }, - [&](Expr * expr) { $$.emplace_back(expr); } - }, std::move($3)); + [&](Expr * expr) { $$.emplace_back(expr); }} + ); } | attr { $$.emplace_back(state->symbols.create($1)); } | string_attr - { std::visit(overloaded { + { $1.visit(overloaded{ [&](std::string_view str) { $$.emplace_back(state->symbols.create(str)); }, - [&](Expr * expr) { $$.emplace_back(expr); } - }, std::move($1)); + [&](Expr * expr) { $$.emplace_back(expr); }} + ); } ; @@ -485,7 +480,7 @@ attr string_attr : '"' string_parts '"' { $$ = std::move($2); } - | DOLLAR_CURLY expr '}' { $$ = $2; } + | DOLLAR_CURLY expr '}' { $$ = {$2}; } ; list From 0c0a41a81a7865cb822b4fd242cac07c873debda Mon Sep 17 00:00:00 2001 From: Taeer Bar-Yam Date: Tue, 25 Nov 2025 17:46:35 +0100 Subject: [PATCH 33/40] tests: add tests for dynamic attribute in let and inherit Regression tests for the previous commit. Co-authored-by: Sergei Zimmerman Co-authored-by: piegames --- maintainers/flake-module.nix | 2 ++ .../lang/eval-fail-dynamic-attrs-inherit-2.err.exp | 6 ++++++ .../lang/eval-fail-dynamic-attrs-inherit-2.nix | 6 ++++++ .../lang/eval-fail-dynamic-attrs-inherit.err.exp | 6 ++++++ .../lang/eval-fail-dynamic-attrs-inherit.nix | 6 ++++++ .../lang/eval-fail-dynamic-attrs-let-2.err.exp | 5 +++++ .../lang/eval-fail-dynamic-attrs-let-2.nix | 4 ++++ .../lang/eval-fail-dynamic-attrs-let-3.err.exp | 5 +++++ .../lang/eval-fail-dynamic-attrs-let-3.nix | 4 ++++ .../lang/eval-fail-dynamic-attrs-let.err.exp | 5 +++++ .../lang/eval-fail-dynamic-attrs-let.nix | 4 ++++ .../functional/lang/eval-okay-dynamic-attrs-3.exp | 1 + .../functional/lang/eval-okay-dynamic-attrs-3.nix | 14 ++++++++++++++ 13 files changed, 68 insertions(+) create mode 100644 tests/functional/lang/eval-fail-dynamic-attrs-inherit-2.err.exp create mode 100644 tests/functional/lang/eval-fail-dynamic-attrs-inherit-2.nix create mode 100644 tests/functional/lang/eval-fail-dynamic-attrs-inherit.err.exp create mode 100644 tests/functional/lang/eval-fail-dynamic-attrs-inherit.nix create mode 100644 tests/functional/lang/eval-fail-dynamic-attrs-let-2.err.exp create mode 100644 tests/functional/lang/eval-fail-dynamic-attrs-let-2.nix create mode 100644 tests/functional/lang/eval-fail-dynamic-attrs-let-3.err.exp create mode 100644 tests/functional/lang/eval-fail-dynamic-attrs-let-3.nix create mode 100644 tests/functional/lang/eval-fail-dynamic-attrs-let.err.exp create mode 100644 tests/functional/lang/eval-fail-dynamic-attrs-let.nix create mode 100644 tests/functional/lang/eval-okay-dynamic-attrs-3.exp create mode 100644 tests/functional/lang/eval-okay-dynamic-attrs-3.nix diff --git a/maintainers/flake-module.nix b/maintainers/flake-module.nix index 8dcff9c63..414e6c570 100644 --- a/maintainers/flake-module.nix +++ b/maintainers/flake-module.nix @@ -79,6 +79,8 @@ # Not supported by nixfmt ''^tests/functional/lang/eval-okay-deprecate-cursed-or\.nix$'' ''^tests/functional/lang/eval-okay-attrs5\.nix$'' + ''^tests/functional/lang/eval-fail-dynamic-attrs-inherit\.nix$'' + ''^tests/functional/lang/eval-fail-dynamic-attrs-inherit-2\.nix$'' # More syntax tests # These tests, or parts of them, should have been parse-* test cases. diff --git a/tests/functional/lang/eval-fail-dynamic-attrs-inherit-2.err.exp b/tests/functional/lang/eval-fail-dynamic-attrs-inherit-2.err.exp new file mode 100644 index 000000000..e71fc23b5 --- /dev/null +++ b/tests/functional/lang/eval-fail-dynamic-attrs-inherit-2.err.exp @@ -0,0 +1,6 @@ +error: dynamic attributes not allowed in inherit + at /pwd/lang/eval-fail-dynamic-attrs-inherit-2.nix:5:15: + 4| { + 5| inherit (a) ${"b" + ""}; + | ^ + 6| } diff --git a/tests/functional/lang/eval-fail-dynamic-attrs-inherit-2.nix b/tests/functional/lang/eval-fail-dynamic-attrs-inherit-2.nix new file mode 100644 index 000000000..7af9685fe --- /dev/null +++ b/tests/functional/lang/eval-fail-dynamic-attrs-inherit-2.nix @@ -0,0 +1,6 @@ +let + a.b = 1; +in +{ + inherit (a) ${"b" + ""}; +} diff --git a/tests/functional/lang/eval-fail-dynamic-attrs-inherit.err.exp b/tests/functional/lang/eval-fail-dynamic-attrs-inherit.err.exp new file mode 100644 index 000000000..b08b0e201 --- /dev/null +++ b/tests/functional/lang/eval-fail-dynamic-attrs-inherit.err.exp @@ -0,0 +1,6 @@ +error: dynamic attributes not allowed in inherit + at /pwd/lang/eval-fail-dynamic-attrs-inherit.nix:5:11: + 4| { + 5| inherit ${"a" + ""}; + | ^ + 6| } diff --git a/tests/functional/lang/eval-fail-dynamic-attrs-inherit.nix b/tests/functional/lang/eval-fail-dynamic-attrs-inherit.nix new file mode 100644 index 000000000..3a9b68410 --- /dev/null +++ b/tests/functional/lang/eval-fail-dynamic-attrs-inherit.nix @@ -0,0 +1,6 @@ +let + a = 1; +in +{ + inherit ${"a" + ""}; +} diff --git a/tests/functional/lang/eval-fail-dynamic-attrs-let-2.err.exp b/tests/functional/lang/eval-fail-dynamic-attrs-let-2.err.exp new file mode 100644 index 000000000..2eb7f04a7 --- /dev/null +++ b/tests/functional/lang/eval-fail-dynamic-attrs-let-2.err.exp @@ -0,0 +1,5 @@ +error: dynamic attributes not allowed in let + at /pwd/lang/eval-fail-dynamic-attrs-let-2.nix:1:1: + 1| let + | ^ + 2| ${"${"a"}"} = 1; diff --git a/tests/functional/lang/eval-fail-dynamic-attrs-let-2.nix b/tests/functional/lang/eval-fail-dynamic-attrs-let-2.nix new file mode 100644 index 000000000..bcec33ddf --- /dev/null +++ b/tests/functional/lang/eval-fail-dynamic-attrs-let-2.nix @@ -0,0 +1,4 @@ +let + ${"${"a"}"} = 1; +in +a diff --git a/tests/functional/lang/eval-fail-dynamic-attrs-let-3.err.exp b/tests/functional/lang/eval-fail-dynamic-attrs-let-3.err.exp new file mode 100644 index 000000000..0f44e25dd --- /dev/null +++ b/tests/functional/lang/eval-fail-dynamic-attrs-let-3.err.exp @@ -0,0 +1,5 @@ +error: dynamic attributes not allowed in let + at /pwd/lang/eval-fail-dynamic-attrs-let-3.nix:1:1: + 1| let + | ^ + 2| "${"a"}" = 1; diff --git a/tests/functional/lang/eval-fail-dynamic-attrs-let-3.nix b/tests/functional/lang/eval-fail-dynamic-attrs-let-3.nix new file mode 100644 index 000000000..37453c530 --- /dev/null +++ b/tests/functional/lang/eval-fail-dynamic-attrs-let-3.nix @@ -0,0 +1,4 @@ +let + "${"a"}" = 1; +in +a diff --git a/tests/functional/lang/eval-fail-dynamic-attrs-let.err.exp b/tests/functional/lang/eval-fail-dynamic-attrs-let.err.exp new file mode 100644 index 000000000..ca3192133 --- /dev/null +++ b/tests/functional/lang/eval-fail-dynamic-attrs-let.err.exp @@ -0,0 +1,5 @@ +error: dynamic attributes not allowed in let + at /pwd/lang/eval-fail-dynamic-attrs-let.nix:1:1: + 1| let + | ^ + 2| ${"a" + ""} = 1; diff --git a/tests/functional/lang/eval-fail-dynamic-attrs-let.nix b/tests/functional/lang/eval-fail-dynamic-attrs-let.nix new file mode 100644 index 000000000..fca32ae4f --- /dev/null +++ b/tests/functional/lang/eval-fail-dynamic-attrs-let.nix @@ -0,0 +1,4 @@ +let + ${"a" + ""} = 1; +in +a diff --git a/tests/functional/lang/eval-okay-dynamic-attrs-3.exp b/tests/functional/lang/eval-okay-dynamic-attrs-3.exp new file mode 100644 index 000000000..9d27f872c --- /dev/null +++ b/tests/functional/lang/eval-okay-dynamic-attrs-3.exp @@ -0,0 +1 @@ +{ a = 1; attrs = { b = 1; c = 1; d = 1; }; b = 1; c = 1; d = 1; } diff --git a/tests/functional/lang/eval-okay-dynamic-attrs-3.nix b/tests/functional/lang/eval-okay-dynamic-attrs-3.nix new file mode 100644 index 000000000..d55ed82f8 --- /dev/null +++ b/tests/functional/lang/eval-okay-dynamic-attrs-3.nix @@ -0,0 +1,14 @@ +# dynamic attrs are not generally allowed in `let`, and inherit, but they are if they only contain a string +let + ${"a"} = 1; + attrs = rec { + b = c; + ${"c"} = d; + d = a; + }; +in +{ + inherit ${"a"}; + inherit attrs; + inherit (attrs) ${"b"} ${"c"} d; +} From 4031343e445dd1f4476c5e527ce7ffd654412711 Mon Sep 17 00:00:00 2001 From: Sergei Zimmerman Date: Wed, 26 Nov 2025 01:22:38 +0300 Subject: [PATCH 34/40] libmain: Fix download progress rendering This was broken in https://github.com/NixOS/nix/pull/14423 accidentally. Add [[nodiscard]] to prevent such mistakes in the future. --- src/libmain/progress-bar.cc | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/src/libmain/progress-bar.cc b/src/libmain/progress-bar.cc index 6cefae6be..a973102f9 100644 --- a/src/libmain/progress-bar.cc +++ b/src/libmain/progress-bar.cc @@ -470,7 +470,8 @@ public: std::string res; auto renderActivity = - [&](ActivityType type, const std::string & itemFmt, const std::string & numberFmt = "%d", double unit = 1) { + [&] [[nodiscard]] ( + ActivityType type, const std::string & itemFmt, const std::string & numberFmt = "%d", double unit = 1) { auto & act = state.activitiesByType[type]; uint64_t done = act.done, expected = act.done, running = 0, failed = act.failed; for (auto & j : act.its) { @@ -514,7 +515,7 @@ public: return s; }; - auto renderSizeActivity = [&](ActivityType type, const std::string & itemFmt = "%s") { + auto renderSizeActivity = [&] [[nodiscard]] (ActivityType type, const std::string & itemFmt = "%s") { auto & act = state.activitiesByType[type]; uint64_t done = act.done, expected = act.done, running = 0, failed = act.failed; for (auto & j : act.its) { @@ -573,14 +574,17 @@ public: return s; }; + auto maybeAppendToResult = [&](std::string_view s) { + if (s.empty()) + return; + if (!res.empty()) + res += ", "; + res += s; + }; + auto showActivity = [&](ActivityType type, const std::string & itemFmt, const std::string & numberFmt = "%d", double unit = 1) { - auto s = renderActivity(type, itemFmt, numberFmt, unit); - if (s.empty()) - return; - if (!res.empty()) - res += ", "; - res += s; + maybeAppendToResult(renderActivity(type, itemFmt, numberFmt, unit)); }; showActivity(actBuilds, "%s built"); @@ -602,7 +606,7 @@ public: } } - renderSizeActivity(actFileTransfer, "%s DL"); + maybeAppendToResult(renderSizeActivity(actFileTransfer, "%s DL")); { auto s = renderActivity(actOptimiseStore, "%s paths optimised"); From 697b068756922da6384e073683f49fa4413f0b39 Mon Sep 17 00:00:00 2001 From: Ubuntu Date: Wed, 26 Nov 2025 00:22:26 +0000 Subject: [PATCH 35/40] Use std::filesystem::path instead of Path in libexpr. --- src/libexpr/eval-settings.cc | 4 +- src/libexpr/include/nix/expr/eval-settings.hh | 2 +- src/libutil/include/nix/util/users.hh | 18 ++++---- src/libutil/include/nix/util/util.hh | 1 + src/libutil/unix/users.cc | 6 +-- src/libutil/users.cc | 41 ++++++++++--------- src/libutil/windows/users.cc | 6 +-- 7 files changed, 41 insertions(+), 37 deletions(-) diff --git a/src/libexpr/eval-settings.cc b/src/libexpr/eval-settings.cc index 8e8b6bcd9..04c619388 100644 --- a/src/libexpr/eval-settings.cc +++ b/src/libexpr/eval-settings.cc @@ -103,9 +103,9 @@ const std::string & EvalSettings::getCurrentSystem() const return evalSystem != "" ? evalSystem : settings.thisSystem.get(); } -Path getNixDefExpr() +std::filesystem::path getNixDefExpr() { - return settings.useXDGBaseDirectories ? getStateDir() + "/defexpr" : getHome() + "/.nix-defexpr"; + return settings.useXDGBaseDirectories ? getStateDir() / "defexpr" : getHome() / ".nix-defexpr"; } } // namespace nix diff --git a/src/libexpr/include/nix/expr/eval-settings.hh b/src/libexpr/include/nix/expr/eval-settings.hh index 250c2cddf..5dbef9272 100644 --- a/src/libexpr/include/nix/expr/eval-settings.hh +++ b/src/libexpr/include/nix/expr/eval-settings.hh @@ -366,6 +366,6 @@ struct EvalSettings : Config /** * Conventionally part of the default nix path in impure mode. */ -Path getNixDefExpr(); +std::filesystem::path getNixDefExpr(); } // namespace nix diff --git a/src/libutil/include/nix/util/users.hh b/src/libutil/include/nix/util/users.hh index f2c6caecf..7a556fa8b 100644 --- a/src/libutil/include/nix/util/users.hh +++ b/src/libutil/include/nix/util/users.hh @@ -1,6 +1,8 @@ #pragma once ///@file +#include + #include "nix/util/types.hh" #ifndef _WIN32 @@ -15,43 +17,43 @@ std::string getUserName(); /** * @return the given user's home directory from /etc/passwd. */ -Path getHomeOf(uid_t userId); +std::filesystem::path getHomeOf(uid_t userId); #endif /** * @return $HOME or the user's home directory from /etc/passwd. */ -Path getHome(); +std::filesystem::path getHome(); /** * @return $NIX_CACHE_HOME or $XDG_CACHE_HOME/nix or $HOME/.cache/nix. */ -Path getCacheDir(); +std::filesystem::path getCacheDir(); /** * @return $NIX_CONFIG_HOME or $XDG_CONFIG_HOME/nix or $HOME/.config/nix. */ -Path getConfigDir(); +std::filesystem::path getConfigDir(); /** * @return the directories to search for user configuration files */ -std::vector getConfigDirs(); +std::vector getConfigDirs(); /** * @return $NIX_DATA_HOME or $XDG_DATA_HOME/nix or $HOME/.local/share/nix. */ -Path getDataDir(); +std::filesystem::path getDataDir(); /** * @return $NIX_STATE_HOME or $XDG_STATE_HOME/nix or $HOME/.local/state/nix. */ -Path getStateDir(); +std::filesystem::path getStateDir(); /** * Create the Nix state directory and return the path to it. */ -Path createNixStateDir(); +std::filesystem::path createNixStateDir(); /** * Perform tilde expansion on a path, replacing tilde with the user's diff --git a/src/libutil/include/nix/util/util.hh b/src/libutil/include/nix/util/util.hh index 83419c8c9..7556663cd 100644 --- a/src/libutil/include/nix/util/util.hh +++ b/src/libutil/include/nix/util/util.hh @@ -6,6 +6,7 @@ #include "nix/util/logging.hh" #include "nix/util/strings.hh" +#include #include #include #include diff --git a/src/libutil/unix/users.cc b/src/libutil/unix/users.cc index 09b38be5e..870bbe376 100644 --- a/src/libutil/unix/users.cc +++ b/src/libutil/unix/users.cc @@ -18,7 +18,7 @@ std::string getUserName() return name; } -Path getHomeOf(uid_t userId) +std::filesystem::path getHomeOf(uid_t userId) { std::vector buf(16384); struct passwd pwbuf; @@ -28,9 +28,9 @@ Path getHomeOf(uid_t userId) return pw->pw_dir; } -Path getHome() +std::filesystem::path getHome() { - static Path homeDir = []() { + static std::filesystem::path homeDir = []() { std::optional unownedUserHomeDir = {}; auto homeDir = getEnv("HOME"); if (homeDir) { diff --git a/src/libutil/users.cc b/src/libutil/users.cc index f19a5d39c..e07b5535e 100644 --- a/src/libutil/users.cc +++ b/src/libutil/users.cc @@ -5,7 +5,7 @@ namespace nix { -Path getCacheDir() +std::filesystem::path getCacheDir() { auto dir = getEnv("NIX_CACHE_HOME"); if (dir) { @@ -13,14 +13,14 @@ Path getCacheDir() } else { auto xdgDir = getEnv("XDG_CACHE_HOME"); if (xdgDir) { - return *xdgDir + "/nix"; + return std::filesystem::path{*xdgDir} / "nix"; } else { - return getHome() + "/.cache/nix"; + return getHome() / ".cache" / "nix"; } } } -Path getConfigDir() +std::filesystem::path getConfigDir() { auto dir = getEnv("NIX_CONFIG_HOME"); if (dir) { @@ -28,26 +28,27 @@ Path getConfigDir() } else { auto xdgDir = getEnv("XDG_CONFIG_HOME"); if (xdgDir) { - return *xdgDir + "/nix"; + return std::filesystem::path{*xdgDir} / "nix"; } else { - return getHome() + "/.config/nix"; + return getHome() / ".config" / "nix"; } } } -std::vector getConfigDirs() +std::vector getConfigDirs() { - Path configHome = getConfigDir(); + std::filesystem::path configHome = getConfigDir(); auto configDirs = getEnv("XDG_CONFIG_DIRS").value_or("/etc/xdg"); - std::vector result = tokenizeString>(configDirs, ":"); - for (auto & p : result) { - p += "/nix"; + auto tokens = tokenizeString>(configDirs, ":"); + std::vector result; + result.push_back(configHome); + for (auto & token : tokens) { + result.push_back(std::filesystem::path{token} / "nix"); } - result.insert(result.begin(), configHome); return result; } -Path getDataDir() +std::filesystem::path getDataDir() { auto dir = getEnv("NIX_DATA_HOME"); if (dir) { @@ -55,14 +56,14 @@ Path getDataDir() } else { auto xdgDir = getEnv("XDG_DATA_HOME"); if (xdgDir) { - return *xdgDir + "/nix"; + return std::filesystem::path{*xdgDir} / "nix"; } else { - return getHome() + "/.local/share/nix"; + return getHome() / ".local" / "share" / "nix"; } } } -Path getStateDir() +std::filesystem::path getStateDir() { auto dir = getEnv("NIX_STATE_HOME"); if (dir) { @@ -70,16 +71,16 @@ Path getStateDir() } else { auto xdgDir = getEnv("XDG_STATE_HOME"); if (xdgDir) { - return *xdgDir + "/nix"; + return std::filesystem::path{*xdgDir} / "nix"; } else { - return getHome() + "/.local/state/nix"; + return getHome() / ".local" / "state" / "nix"; } } } -Path createNixStateDir() +std::filesystem::path createNixStateDir() { - Path dir = getStateDir(); + std::filesystem::path dir = getStateDir(); createDirs(dir); return dir; } diff --git a/src/libutil/windows/users.cc b/src/libutil/windows/users.cc index 6cc753cec..cbeadcb81 100644 --- a/src/libutil/windows/users.cc +++ b/src/libutil/windows/users.cc @@ -35,10 +35,10 @@ std::string getUserName() return name; } -Path getHome() +std::filesystem::path getHome() { - static Path homeDir = []() { - Path homeDir = getEnv("USERPROFILE").value_or("C:\\Users\\Default"); + static std::filesystem::path homeDir = []() { + std::filesystem::path homeDir = getEnv("USERPROFILE").value_or("C:\\Users\\Default"); assert(!homeDir.empty()); return canonPath(homeDir); }(); From 3716bd9a62aa9d0f4c11e8f5ecadb42274a64fa6 Mon Sep 17 00:00:00 2001 From: Sergei Zimmerman Date: Wed, 26 Nov 2025 03:22:46 +0300 Subject: [PATCH 36/40] libstore: Fix double quotes in debug logs for pathlocks This is now using std::filesystem which gets double-quoted. --- src/libstore/unix/pathlocks.cc | 14 +++++++------- src/libstore/windows/pathlocks.cc | 12 ++++++------ 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/src/libstore/unix/pathlocks.cc b/src/libstore/unix/pathlocks.cc index fc1206012..6117b82c8 100644 --- a/src/libstore/unix/pathlocks.cc +++ b/src/libstore/unix/pathlocks.cc @@ -19,7 +19,7 @@ AutoCloseFD openLockFile(const std::filesystem::path & path, bool create) fd = open(path.c_str(), O_CLOEXEC | O_RDWR | (create ? O_CREAT : 0), 0600); if (!fd && (create || errno != ENOENT)) - throw SysError("opening lock file '%1%'", path); + throw SysError("opening lock file %1%", path); return fd; } @@ -83,7 +83,7 @@ bool PathLocks::lockPaths(const std::set & paths, const s checkInterrupt(); std::filesystem::path lockPath = path + ".lock"; - debug("locking path '%1%'", path); + debug("locking path %1%", path); AutoCloseFD fd; @@ -106,19 +106,19 @@ bool PathLocks::lockPaths(const std::set & paths, const s } } - debug("lock acquired on '%1%'", lockPath); + debug("lock acquired on %1%", lockPath); /* Check that the lock file hasn't become stale (i.e., hasn't been unlinked). */ struct stat st; if (fstat(fd.get(), &st) == -1) - throw SysError("statting lock file '%1%'", lockPath); + throw SysError("statting lock file %1%", lockPath); if (st.st_size != 0) /* This lock file has been unlinked, so we're holding a lock on a deleted file. This means that other processes may create and acquire a lock on `lockPath', and proceed. So we must retry. */ - debug("open lock file '%1%' has become stale", lockPath); + debug("open lock file %1% has become stale", lockPath); else break; } @@ -137,9 +137,9 @@ void PathLocks::unlock() deleteLockFile(i.second, i.first); if (close(i.first) == -1) - printError("error (ignored): cannot close lock file on '%1%'", i.second); + printError("error (ignored): cannot close lock file on %1%", i.second); - debug("lock released on '%1%'", i.second); + debug("lock released on %1%", i.second); } fds.clear(); diff --git a/src/libstore/windows/pathlocks.cc b/src/libstore/windows/pathlocks.cc index 88fb4e22e..32d9e7c0f 100644 --- a/src/libstore/windows/pathlocks.cc +++ b/src/libstore/windows/pathlocks.cc @@ -28,9 +28,9 @@ void PathLocks::unlock() deleteLockFile(i.second, i.first); if (CloseHandle(i.first) == -1) - printError("error (ignored): cannot close lock file on '%1%'", i.second); + printError("error (ignored): cannot close lock file on %1%", i.second); - debug("lock released on '%1%'", i.second); + debug("lock released on %1%", i.second); } fds.clear(); @@ -111,7 +111,7 @@ bool PathLocks::lockPaths(const std::set & paths, const s checkInterrupt(); std::filesystem::path lockPath = path; lockPath += L".lock"; - debug("locking path '%1%'", path); + debug("locking path %1%", path); AutoCloseFD fd; @@ -128,13 +128,13 @@ bool PathLocks::lockPaths(const std::set & paths, const s } } - debug("lock acquired on '%1%'", lockPath); + debug("lock acquired on %1%", lockPath); struct _stat st; if (_fstat(fromDescriptorReadOnly(fd.get()), &st) == -1) - throw SysError("statting lock file '%1%'", lockPath); + throw SysError("statting lock file %1%", lockPath); if (st.st_size != 0) - debug("open lock file '%1%' has become stale", lockPath); + debug("open lock file %1% has become stale", lockPath); else break; } From 0778b861a93eb47850ad8f48cfc4153bf31deba6 Mon Sep 17 00:00:00 2001 From: Sergei Zimmerman Date: Wed, 26 Nov 2025 03:47:42 +0300 Subject: [PATCH 37/40] libutil: Use openFileEnsureBeneathNoSymlinks in RestoreSink::createRegularFile Add more assertions for preconditions of openFileEnsureBeneathNoSymlinks to prevent misuse. Also start using it for regular file creation as well. --- src/libutil-tests/file-system.cc | 4 ++++ src/libutil/fs-sink.cc | 2 +- src/libutil/include/nix/util/file-descriptor.hh | 2 ++ src/libutil/unix/file-descriptor.cc | 3 +++ 4 files changed, 10 insertions(+), 1 deletion(-) diff --git a/src/libutil-tests/file-system.cc b/src/libutil-tests/file-system.cc index 3a54ac55b..8ea081c51 100644 --- a/src/libutil-tests/file-system.cc +++ b/src/libutil-tests/file-system.cc @@ -351,6 +351,10 @@ TEST(openFileEnsureBeneathNoSymlinks, works) sink.createDirectory( CanonPath("a/b/c/f"), [](FileSystemObjectSink & dirSink, const CanonPath & relPath) {}), SymlinkNotAllowed); + ASSERT_THROW( + sink.createRegularFile( + CanonPath("a/b/c/regular"), [](CreateRegularFileSink & crf) { crf("some contents"); }), + SymlinkNotAllowed); } AutoCloseFD dirFd = openDirectory(tmpDir); diff --git a/src/libutil/fs-sink.cc b/src/libutil/fs-sink.cc index 87bdaa339..521a10c9a 100644 --- a/src/libutil/fs-sink.cc +++ b/src/libutil/fs-sink.cc @@ -170,7 +170,7 @@ void RestoreSink::createRegularFile(const CanonPath & path, std::function= 1); auto components = std::views::take(path, nrComponents - 1); /* Everything but last component */ auto getParentFd = [&]() { return parentFd ? parentFd.get() : dirFd; }; @@ -320,6 +321,8 @@ openFileEnsureBeneathNoSymlinksIterative(Descriptor dirFd, const CanonPath & pat Descriptor unix::openFileEnsureBeneathNoSymlinks(Descriptor dirFd, const CanonPath & path, int flags, mode_t mode) { + assert(!path.rel().starts_with('/')); /* Just in case the invariant is somehow broken. */ + assert(!path.isRoot()); #ifdef __linux__ auto maybeFd = linux::openat2( dirFd, path.rel_c_str(), flags, static_cast(mode), RESOLVE_BENEATH | RESOLVE_NO_SYMLINKS); From e761a9fb6d02d7071bde4e87ea705cbe526df5c4 Mon Sep 17 00:00:00 2001 From: Ubuntu Date: Wed, 26 Nov 2025 00:22:26 +0000 Subject: [PATCH 38/40] Use std::filesystem::path instead of Path in libexpr. --- src/libexpr/parser.y | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libexpr/parser.y b/src/libexpr/parser.y index 520086e28..c9ad30407 100644 --- a/src/libexpr/parser.y +++ b/src/libexpr/parser.y @@ -384,7 +384,7 @@ path_start std::string_view($1.p, $1.l) ); } - Path path(getHome() + std::string($1.p + 1, $1.l - 1)); + Path path(getHome().string() + std::string($1.p + 1, $1.l - 1)); $$ = state->exprs.add(state->exprs.alloc, ref(state->rootFS), path); } ; From f0390758dd89806b1c1bb6e1d9620ca27e64493f Mon Sep 17 00:00:00 2001 From: Ubuntu Date: Wed, 26 Nov 2025 03:59:02 +0000 Subject: [PATCH 39/40] Replace Path with std::filesystem::path in libfetchers. --- src/libexpr-tests/nix_api_expr.cc | 8 +++---- src/libfetchers/git-lfs-fetch.cc | 8 +++---- src/libfetchers/git-utils.cc | 2 +- src/libfetchers/git.cc | 14 +++++------ .../include/nix/fetchers/registry.hh | 6 ++--- src/libfetchers/mercurial.cc | 24 +++++++++---------- src/libfetchers/registry.cc | 22 ++++++++--------- src/libflake-tests/nix_api_flake.cc | 22 +++++++++-------- src/libstore-tests/nix_api_store.cc | 9 +++---- src/libstore/globals.cc | 2 +- src/libstore/include/nix/store/globals.hh | 4 ++-- src/libstore/local-store.cc | 2 +- .../unix/build/darwin-derivation-builder.cc | 2 +- src/libutil/file-system.cc | 10 ++++---- src/libutil/include/nix/util/file-system.hh | 7 +++--- 15 files changed, 72 insertions(+), 70 deletions(-) diff --git a/src/libexpr-tests/nix_api_expr.cc b/src/libexpr-tests/nix_api_expr.cc index 3e5cd0d43..026c89598 100644 --- a/src/libexpr-tests/nix_api_expr.cc +++ b/src/libexpr-tests/nix_api_expr.cc @@ -18,13 +18,13 @@ TEST_F(nix_api_expr_test, nix_eval_state_lookup_path) { auto tmpDir = nix::createTempDir(); auto delTmpDir = std::make_unique(tmpDir, true); - auto nixpkgs = tmpDir + "/pkgs"; - auto nixos = tmpDir + "/cfg"; + auto nixpkgs = tmpDir / "pkgs"; + auto nixos = tmpDir / "cfg"; std::filesystem::create_directories(nixpkgs); std::filesystem::create_directories(nixos); - std::string nixpkgsEntry = "nixpkgs=" + nixpkgs; - std::string nixosEntry = "nixos-config=" + nixos; + std::string nixpkgsEntry = "nixpkgs=" + nixpkgs.string(); + std::string nixosEntry = "nixos-config=" + nixos.string(); const char * lookupPath[] = {nixpkgsEntry.c_str(), nixosEntry.c_str(), nullptr}; auto builder = nix_eval_state_builder_new(ctx, store); diff --git a/src/libfetchers/git-lfs-fetch.cc b/src/libfetchers/git-lfs-fetch.cc index f1982c314..e2b2c2e7d 100644 --- a/src/libfetchers/git-lfs-fetch.cc +++ b/src/libfetchers/git-lfs-fetch.cc @@ -268,10 +268,10 @@ void Fetch::fetch( return; } - Path cacheDir = getCacheDir() + "/git-lfs"; + std::filesystem::path cacheDir = getCacheDir() / "git-lfs"; std::string key = hashString(HashAlgorithm::SHA256, pointerFilePath.rel()).to_string(HashFormat::Base16, false) + "/" + pointer->oid; - Path cachePath = cacheDir + "/" + key; + std::filesystem::path cachePath = cacheDir / key; if (pathExists(cachePath)) { debug("using cache entry %s -> %s", key, cachePath); sink(readFile(cachePath)); @@ -302,8 +302,8 @@ void Fetch::fetch( downloadToSink(ourl, authHeader, sink, sha256, size); debug("creating cache entry %s -> %s", key, cachePath); - if (!pathExists(dirOf(cachePath))) - createDirs(dirOf(cachePath)); + if (!pathExists(cachePath.parent_path())) + createDirs(cachePath.parent_path()); writeFile(cachePath, sink.s); debug("%s fetched with git-lfs", pointerFilePath); diff --git a/src/libfetchers/git-utils.cc b/src/libfetchers/git-utils.cc index 4a7fc3d0d..37f776b11 100644 --- a/src/libfetchers/git-utils.cc +++ b/src/libfetchers/git-utils.cc @@ -206,7 +206,7 @@ static void initRepoAtomically(std::filesystem::path & path, bool bare) if (pathExists(path.string())) return; - Path tmpDir = createTempDir(os_string_to_string(PathViewNG{std::filesystem::path(path).parent_path()})); + std::filesystem::path tmpDir = createTempDir(path.parent_path()); AutoDelete delTmpDir(tmpDir, true); Repository tmpRepo; diff --git a/src/libfetchers/git.cc b/src/libfetchers/git.cc index 75e3f1214..0e6a6cb69 100644 --- a/src/libfetchers/git.cc +++ b/src/libfetchers/git.cc @@ -42,10 +42,10 @@ bool isCacheFileWithinTtl(time_t now, const struct stat & st) return st.st_mtime + static_cast(settings.tarballTtl) > now; } -Path getCachePath(std::string_view key, bool shallow) +std::filesystem::path getCachePath(std::string_view key, bool shallow) { - return getCacheDir() + "/gitv3/" + hashString(HashAlgorithm::SHA256, key).to_string(HashFormat::Nix32, false) - + (shallow ? "-shallow" : ""); + return getCacheDir() / "gitv3" + / (hashString(HashAlgorithm::SHA256, key).to_string(HashFormat::Nix32, false) + (shallow ? "-shallow" : "")); } // Returns the name of the HEAD branch. @@ -55,7 +55,7 @@ Path getCachePath(std::string_view key, bool shallow) // // ref: refs/heads/main HEAD // ... -std::optional readHead(const Path & path) +std::optional readHead(const std::filesystem::path & path) { auto [status, output] = runProgram( RunOptions{ @@ -86,7 +86,7 @@ std::optional readHead(const Path & path) // Persist the HEAD ref from the remote repo in the local cached repo. bool storeCachedHead(const std::string & actualUrl, bool shallow, const std::string & headRef) { - Path cacheDir = getCachePath(actualUrl, shallow); + std::filesystem::path cacheDir = getCachePath(actualUrl, shallow); try { runProgram("git", true, {"-C", cacheDir, "--git-dir", ".", "symbolic-ref", "--", "HEAD", headRef}); } catch (ExecError & e) { @@ -109,8 +109,8 @@ std::optional readHeadCached(const std::string & actualUrl, bool sh { // Create a cache path to store the branch of the HEAD ref. Append something // in front of the URL to prevent collision with the repository itself. - Path cacheDir = getCachePath(actualUrl, shallow); - Path headRefFile = cacheDir + "/HEAD"; + std::filesystem::path cacheDir = getCachePath(actualUrl, shallow); + std::filesystem::path headRefFile = cacheDir / "HEAD"; time_t now = time(0); struct stat st; diff --git a/src/libfetchers/include/nix/fetchers/registry.hh b/src/libfetchers/include/nix/fetchers/registry.hh index c9c8b162f..dc7e3edb5 100644 --- a/src/libfetchers/include/nix/fetchers/registry.hh +++ b/src/libfetchers/include/nix/fetchers/registry.hh @@ -39,7 +39,7 @@ struct Registry static std::shared_ptr read(const Settings & settings, const SourcePath & path, RegistryType type); - void write(const Path & path); + void write(const std::filesystem::path & path); void add(const Input & from, const Input & to, const Attrs & extraAttrs); @@ -50,9 +50,9 @@ typedef std::vector> Registries; std::shared_ptr getUserRegistry(const Settings & settings); -std::shared_ptr getCustomRegistry(const Settings & settings, const Path & p); +std::shared_ptr getCustomRegistry(const Settings & settings, const std::filesystem::path & p); -Path getUserRegistryPath(); +std::filesystem::path getUserRegistryPath(); Registries getRegistries(const Settings & settings, Store & store); diff --git a/src/libfetchers/mercurial.cc b/src/libfetchers/mercurial.cc index 87e18133d..65999497c 100644 --- a/src/libfetchers/mercurial.cc +++ b/src/libfetchers/mercurial.cc @@ -213,11 +213,11 @@ struct MercurialInputScheme : InputScheme runHg({"status", "-R", actualUrl, "--clean", "--modified", "--added", "--no-status", "--print0"}), "\0"s); - Path actualPath(absPath(actualUrl)); + std::filesystem::path actualPath(absPath(actualUrl)); PathFilter filter = [&](const Path & p) -> bool { - assert(hasPrefix(p, actualPath)); - std::string file(p, actualPath.size() + 1); + assert(hasPrefix(p, actualPath.string())); + std::string file(p, actualPath.string().size() + 1); auto st = lstat(p); @@ -232,7 +232,7 @@ struct MercurialInputScheme : InputScheme auto storePath = store.addToStore( input.getName(), - {getFSSourceAccessor(), CanonPath(actualPath)}, + {getFSSourceAccessor(), CanonPath(actualPath.string())}, ContentAddressMethod::Raw::NixArchive, HashAlgorithm::SHA256, {}, @@ -275,10 +275,8 @@ struct MercurialInputScheme : InputScheme return makeResult(res->value, res->storePath); } - Path cacheDir = - fmt("%s/hg/%s", - getCacheDir(), - hashString(HashAlgorithm::SHA256, actualUrl).to_string(HashFormat::Nix32, false)); + std::filesystem::path cacheDir = + getCacheDir() / "hg" / hashString(HashAlgorithm::SHA256, actualUrl).to_string(HashFormat::Nix32, false); /* If this is a commit hash that we already have, we don't have to pull again. */ @@ -292,7 +290,7 @@ struct MercurialInputScheme : InputScheme try { runHg({"pull", "-R", cacheDir, "--", actualUrl}); } catch (ExecError & e) { - auto transJournal = cacheDir + "/.hg/store/journal"; + auto transJournal = cacheDir / ".hg" / "store" / "journal"; /* hg throws "abandoned transaction" error only if this file exists */ if (pathExists(transJournal)) { runHg({"recover", "-R", cacheDir}); @@ -302,7 +300,7 @@ struct MercurialInputScheme : InputScheme } } } else { - createDirs(dirOf(cacheDir)); + createDirs(dirOf(cacheDir.string())); runHg({"clone", "--noupdate", "--", actualUrl, cacheDir}); } } @@ -328,14 +326,14 @@ struct MercurialInputScheme : InputScheme if (auto res = settings.getCache()->lookupStorePath(revInfoKey(rev), store)) return makeResult(res->value, res->storePath); - Path tmpDir = createTempDir(); + std::filesystem::path tmpDir = createTempDir(); AutoDelete delTmpDir(tmpDir, true); runHg({"archive", "-R", cacheDir, "-r", rev.gitRev(), tmpDir}); - deletePath(tmpDir + "/.hg_archival.txt"); + deletePath(tmpDir / ".hg_archival.txt"); - auto storePath = store.addToStore(name, {getFSSourceAccessor(), CanonPath(tmpDir)}); + auto storePath = store.addToStore(name, {getFSSourceAccessor(), CanonPath(tmpDir.string())}); Attrs infoAttrs({ {"revCount", (uint64_t) revCount}, diff --git a/src/libfetchers/registry.cc b/src/libfetchers/registry.cc index cb360f03c..c81eb6b53 100644 --- a/src/libfetchers/registry.cc +++ b/src/libfetchers/registry.cc @@ -56,7 +56,7 @@ std::shared_ptr Registry::read(const Settings & settings, const Source return registry; } -void Registry::write(const Path & path) +void Registry::write(const std::filesystem::path & path) { nlohmann::json arr; for (auto & entry : entries) { @@ -74,7 +74,7 @@ void Registry::write(const Path & path) json["version"] = 2; json["flakes"] = std::move(arr); - createDirs(dirOf(path)); + createDirs(path.parent_path()); writeFile(path, json.dump(2)); } @@ -90,38 +90,38 @@ void Registry::remove(const Input & input) entries.end()); } -static Path getSystemRegistryPath() +static std::filesystem::path getSystemRegistryPath() { - return settings.nixConfDir + "/registry.json"; + return settings.nixConfDir / "registry.json"; } static std::shared_ptr getSystemRegistry(const Settings & settings) { static auto systemRegistry = Registry::read( settings, - SourcePath{getFSSourceAccessor(), CanonPath{getSystemRegistryPath()}}.resolveSymlinks(), + SourcePath{getFSSourceAccessor(), CanonPath{getSystemRegistryPath().string()}}.resolveSymlinks(), Registry::System); return systemRegistry; } -Path getUserRegistryPath() +std::filesystem::path getUserRegistryPath() { - return getConfigDir() + "/registry.json"; + return getConfigDir() / "registry.json"; } std::shared_ptr getUserRegistry(const Settings & settings) { static auto userRegistry = Registry::read( settings, - SourcePath{getFSSourceAccessor(), CanonPath{getUserRegistryPath()}}.resolveSymlinks(), + SourcePath{getFSSourceAccessor(), CanonPath{getUserRegistryPath().string()}}.resolveSymlinks(), Registry::User); return userRegistry; } -std::shared_ptr getCustomRegistry(const Settings & settings, const Path & p) +std::shared_ptr getCustomRegistry(const Settings & settings, const std::filesystem::path & p) { - static auto customRegistry = - Registry::read(settings, SourcePath{getFSSourceAccessor(), CanonPath{p}}.resolveSymlinks(), Registry::Custom); + static auto customRegistry = Registry::read( + settings, SourcePath{getFSSourceAccessor(), CanonPath{p.string()}}.resolveSymlinks(), Registry::Custom); return customRegistry; } diff --git a/src/libflake-tests/nix_api_flake.cc b/src/libflake-tests/nix_api_flake.cc index da7f01401..84ac8eb40 100644 --- a/src/libflake-tests/nix_api_flake.cc +++ b/src/libflake-tests/nix_api_flake.cc @@ -86,7 +86,7 @@ TEST_F(nix_api_store_test, nix_api_load_flake) auto tmpDir = nix::createTempDir(); nix::AutoDelete delTmpDir(tmpDir, true); - nix::writeFile(tmpDir + "/flake.nix", R"( + nix::writeFile(tmpDir / "flake.nix", R"( { outputs = { ... }: { hello = "potato"; @@ -121,7 +121,8 @@ TEST_F(nix_api_store_test, nix_api_load_flake) assert_ctx_ok(); ASSERT_NE(nullptr, parseFlags); - auto r0 = nix_flake_reference_parse_flags_set_base_directory(ctx, parseFlags, tmpDir.c_str(), tmpDir.size()); + auto r0 = + nix_flake_reference_parse_flags_set_base_directory(ctx, parseFlags, tmpDir.c_str(), tmpDir.string().size()); assert_ctx_ok(); ASSERT_EQ(NIX_OK, r0); @@ -177,8 +178,8 @@ TEST_F(nix_api_store_test, nix_api_load_flake_with_flags) auto tmpDir = nix::createTempDir(); nix::AutoDelete delTmpDir(tmpDir, true); - nix::createDirs(tmpDir + "/b"); - nix::writeFile(tmpDir + "/b/flake.nix", R"( + nix::createDirs(tmpDir / "b"); + nix::writeFile(tmpDir / "b" / "flake.nix", R"( { outputs = { ... }: { hello = "BOB"; @@ -186,18 +187,18 @@ TEST_F(nix_api_store_test, nix_api_load_flake_with_flags) } )"); - nix::createDirs(tmpDir + "/a"); - nix::writeFile(tmpDir + "/a/flake.nix", R"( + nix::createDirs(tmpDir / "a"); + nix::writeFile(tmpDir / "a" / "flake.nix", R"( { - inputs.b.url = ")" + tmpDir + R"(/b"; + inputs.b.url = ")" + tmpDir.string() + R"(/b"; outputs = { b, ... }: { hello = b.hello; }; } )"); - nix::createDirs(tmpDir + "/c"); - nix::writeFile(tmpDir + "/c/flake.nix", R"( + nix::createDirs(tmpDir / "c"); + nix::writeFile(tmpDir / "c" / "flake.nix", R"( { outputs = { ... }: { hello = "Claire"; @@ -230,7 +231,8 @@ TEST_F(nix_api_store_test, nix_api_load_flake_with_flags) assert_ctx_ok(); ASSERT_NE(nullptr, parseFlags); - auto r0 = nix_flake_reference_parse_flags_set_base_directory(ctx, parseFlags, tmpDir.c_str(), tmpDir.size()); + auto r0 = + nix_flake_reference_parse_flags_set_base_directory(ctx, parseFlags, tmpDir.c_str(), tmpDir.string().size()); assert_ctx_ok(); ASSERT_EQ(NIX_OK, r0); diff --git a/src/libstore-tests/nix_api_store.cc b/src/libstore-tests/nix_api_store.cc index a7fa4d8d8..142f648d3 100644 --- a/src/libstore-tests/nix_api_store.cc +++ b/src/libstore-tests/nix_api_store.cc @@ -212,9 +212,9 @@ TEST_F(nix_api_store_test, nix_store_real_path) TEST_F(nix_api_util_context, nix_store_real_path_relocated) { auto tmp = nix::createTempDir(); - std::string storeRoot = tmp + "/store"; - std::string stateDir = tmp + "/state"; - std::string logDir = tmp + "/log"; + std::string storeRoot = tmp / "store"; + std::string stateDir = tmp / "state"; + std::string logDir = tmp / "log"; const char * rootkv[] = {"root", storeRoot.c_str()}; const char * statekv[] = {"state", stateDir.c_str()}; const char * logkv[] = {"log", logDir.c_str()}; @@ -250,7 +250,8 @@ TEST_F(nix_api_util_context, nix_store_real_path_relocated) TEST_F(nix_api_util_context, nix_store_real_path_binary_cache) { - Store * store = nix_store_open(ctx, nix::fmt("file://%s/binary-cache", nix::createTempDir()).c_str(), nullptr); + Store * store = + nix_store_open(ctx, nix::fmt("file://%s/binary-cache", nix::createTempDir().string()).c_str(), nullptr); assert_ctx_ok(); ASSERT_NE(store, nullptr); diff --git a/src/libstore/globals.cc b/src/libstore/globals.cc index 8c542b686..9e72f8577 100644 --- a/src/libstore/globals.cc +++ b/src/libstore/globals.cc @@ -486,7 +486,7 @@ void initLibStore(bool loadConfig) /* On macOS, don't use the per-session TMPDIR (as set e.g. by sshd). This breaks build users because they don't have access to the TMPDIR, in particular in ‘nix-store --serve’. */ - if (hasPrefix(defaultTempDir(), "/var/folders/")) + if (hasPrefix(defaultTempDir().string(), "/var/folders/")) unsetenv("TMPDIR"); #endif diff --git a/src/libstore/include/nix/store/globals.hh b/src/libstore/include/nix/store/globals.hh index 5ddfbee30..fa4f7abbd 100644 --- a/src/libstore/include/nix/store/globals.hh +++ b/src/libstore/include/nix/store/globals.hh @@ -101,7 +101,7 @@ public: /** * The directory where system configuration files are stored. */ - Path nixConfDir; + std::filesystem::path nixConfDir; /** * A list of user configuration files to load. @@ -292,7 +292,7 @@ public: Setting builders{ this, - "@" + nixConfDir + "/machines", + "@" + nixConfDir.string() + "/machines", "builders", R"( A semicolon- or newline-separated list of build machines. diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index a849576f6..ab242bd84 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -1332,7 +1332,7 @@ std::pair LocalStore::createTempDirInStore() /* There is a slight possibility that `tmpDir' gets deleted by the GC between createTempDir() and when we acquire a lock on it. We'll repeat until 'tmpDir' exists and we've locked it. */ - tmpDirFn = createTempDir(config->realStoreDir, "tmp"); + tmpDirFn = createTempDir(std::filesystem::path{config->realStoreDir.get()}, "tmp"); tmpDirFd = openDirectory(tmpDirFn); if (!tmpDirFd) { continue; diff --git a/src/libstore/unix/build/darwin-derivation-builder.cc b/src/libstore/unix/build/darwin-derivation-builder.cc index 613ec6d54..24329bffc 100644 --- a/src/libstore/unix/build/darwin-derivation-builder.cc +++ b/src/libstore/unix/build/darwin-derivation-builder.cc @@ -174,7 +174,7 @@ struct DarwinDerivationBuilder : DerivationBuilderImpl /* The tmpDir in scope points at the temporary build directory for our derivation. Some packages try different mechanisms to find temporary directories, so we want to open up a broader place for them to put their files, if needed. */ - Path globalTmpDir = canonPath(defaultTempDir(), true); + Path globalTmpDir = canonPath(defaultTempDir().string(), true); /* They don't like trailing slashes on subpath directives */ while (!globalTmpDir.empty() && globalTmpDir.back() == '/') diff --git a/src/libutil/file-system.cc b/src/libutil/file-system.cc index 758173f41..6b145b343 100644 --- a/src/libutil/file-system.cc +++ b/src/libutil/file-system.cc @@ -676,16 +676,16 @@ void AutoUnmount::cancel() ////////////////////////////////////////////////////////////////////// -std::string defaultTempDir() +std::filesystem::path defaultTempDir() { return getEnvNonEmpty("TMPDIR").value_or("/tmp"); } -Path createTempDir(const Path & tmpRoot, const Path & prefix, mode_t mode) +std::filesystem::path createTempDir(const std::filesystem::path & tmpRoot, const std::string & prefix, mode_t mode) { while (1) { checkInterrupt(); - Path tmpDir = makeTempPath(tmpRoot, prefix); + std::filesystem::path tmpDir = makeTempPath(tmpRoot, prefix); if (mkdir( tmpDir.c_str() #ifndef _WIN32 // TODO abstract mkdir perms for Windows @@ -727,11 +727,11 @@ std::pair createTempFile(const Path & prefix) return {std::move(fd), tmpl}; } -Path makeTempPath(const Path & root, const Path & suffix) +std::filesystem::path makeTempPath(const std::filesystem::path & root, const std::string & suffix) { // start the counter at a random value to minimize issues with preexisting temp paths static std::atomic counter(std::random_device{}()); - auto tmpRoot = canonPath(root.empty() ? defaultTempDir() : root, true); + auto tmpRoot = canonPath(root.empty() ? defaultTempDir().string() : root.string(), true); return fmt("%1%/%2%-%3%-%4%", tmpRoot, suffix, getpid(), counter.fetch_add(1, std::memory_order_relaxed)); } diff --git a/src/libutil/include/nix/util/file-system.hh b/src/libutil/include/nix/util/file-system.hh index a203af2aa..7673c24fd 100644 --- a/src/libutil/include/nix/util/file-system.hh +++ b/src/libutil/include/nix/util/file-system.hh @@ -334,7 +334,8 @@ typedef std::unique_ptr AutoCloseDir; /** * Create a temporary directory. */ -Path createTempDir(const Path & tmpRoot = "", const Path & prefix = "nix", mode_t mode = 0755); +std::filesystem::path +createTempDir(const std::filesystem::path & tmpRoot = "", const std::string & prefix = "nix", mode_t mode = 0755); /** * Create a temporary file, returning a file handle and its path. @@ -344,7 +345,7 @@ std::pair createTempFile(const Path & prefix = "nix"); /** * Return `TMPDIR`, or the default temporary directory if unset or empty. */ -Path defaultTempDir(); +std::filesystem::path defaultTempDir(); /** * Interpret `exe` as a location in the ambient file system and return @@ -358,7 +359,7 @@ bool isExecutableFileAmbient(const std::filesystem::path & exe); * The constructed path looks like `--`. To create a * path nested in a directory, provide a suffix starting with `/`. */ -Path makeTempPath(const Path & root, const Path & suffix = ".tmp"); +std::filesystem::path makeTempPath(const std::filesystem::path & root, const std::string & suffix = ".tmp"); /** * Used in various places. From e7f95783dbc0035827397d4a634ec4654ed15673 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Wed, 26 Nov 2025 19:38:42 +0100 Subject: [PATCH 40/40] Move GitHub input attribute validation into inputFromAttrs() Previously inputFromAttrs() didn't do any validation. inputFromURL() now calls inputFromAttrs(), so we only need to validate in one place. Fixes #14655. --- src/libfetchers/github.cc | 63 ++++++++++++++++++++------------------- 1 file changed, 33 insertions(+), 30 deletions(-) diff --git a/src/libfetchers/github.cc b/src/libfetchers/github.cc index cd7ce1b4e..b86fa926a 100644 --- a/src/libfetchers/github.cc +++ b/src/libfetchers/github.cc @@ -41,18 +41,16 @@ struct GitArchiveInputScheme : InputScheme /* This ignores empty path segments for back-compat. Older versions used a tokenizeString here. */ auto path = url.pathSegments(/*skipEmpty=*/true) | std::ranges::to>(); - std::optional rev; + std::optional rev; std::optional ref; std::optional host_url; auto size = path.size(); if (size == 3) { if (std::regex_match(path[2], revRegex)) - rev = Hash::parseAny(path[2], HashAlgorithm::SHA1); - else if (isLegalRefName(path[2])) - ref = path[2]; + rev = path[2]; else - throw BadURL("in URL '%s', '%s' is not a commit hash or branch/tag name", url, path[2]); + ref = path[2]; } else if (size > 3) { std::string rs; for (auto i = std::next(path.begin(), 2); i != path.end(); i++) { @@ -61,12 +59,7 @@ struct GitArchiveInputScheme : InputScheme rs += "/"; } } - - if (isLegalRefName(rs)) { - ref = rs; - } else { - throw BadURL("in URL '%s', '%s' is not a branch/tag name", url, rs); - } + ref = rs; } else if (size < 2) throw BadURL("URL '%s' is invalid", url); @@ -74,40 +67,32 @@ struct GitArchiveInputScheme : InputScheme if (name == "rev") { if (rev) throw BadURL("URL '%s' contains multiple commit hashes", url); - rev = Hash::parseAny(value, HashAlgorithm::SHA1); + rev = value; } else if (name == "ref") { - if (!isLegalRefName(value)) - throw BadURL("URL '%s' contains an invalid branch/tag name", url); if (ref) throw BadURL("URL '%s' contains multiple branch/tag names", url); ref = value; - } else if (name == "host") { - if (!std::regex_match(value, hostRegex)) - throw BadURL("URL '%s' contains an invalid instance host", url); + } else if (name == "host") host_url = value; - } // FIXME: barf on unsupported attributes } - if (ref && rev) - throw BadURL("URL '%s' contains both a commit hash and a branch/tag name %s %s", url, *ref, rev->gitRev()); - - Input input{}; - input.attrs.insert_or_assign("type", std::string{schemeName()}); - input.attrs.insert_or_assign("owner", path[0]); - input.attrs.insert_or_assign("repo", path[1]); + Attrs attrs; + attrs.insert_or_assign("type", std::string{schemeName()}); + attrs.insert_or_assign("owner", path[0]); + attrs.insert_or_assign("repo", path[1]); if (rev) - input.attrs.insert_or_assign("rev", rev->gitRev()); + attrs.insert_or_assign("rev", *rev); if (ref) - input.attrs.insert_or_assign("ref", *ref); + attrs.insert_or_assign("ref", *ref); if (host_url) - input.attrs.insert_or_assign("host", *host_url); + attrs.insert_or_assign("host", *host_url); auto narHash = url.query.find("narHash"); if (narHash != url.query.end()) - input.attrs.insert_or_assign("narHash", narHash->second); + attrs.insert_or_assign("narHash", narHash->second); - return input; + return inputFromAttrs(settings, attrs); } const std::map & allowedAttrs() const override @@ -154,6 +139,24 @@ struct GitArchiveInputScheme : InputScheme getStrAttr(attrs, "owner"); getStrAttr(attrs, "repo"); + auto ref = maybeGetStrAttr(attrs, "ref"); + auto rev = maybeGetStrAttr(attrs, "rev"); + if (ref && rev) + throw BadURL( + "input %s contains both a commit hash ('%s') and a branch/tag name ('%s')", + attrsToJSON(attrs), + *rev, + *ref); + + if (rev) + Hash::parseAny(*rev, HashAlgorithm::SHA1); + + if (ref && !isLegalRefName(*ref)) + throw BadURL("input %s contains an invalid branch/tag name", attrsToJSON(attrs)); + + if (auto host = maybeGetStrAttr(attrs, "host"); host && !std::regex_match(*host, hostRegex)) + throw BadURL("input %s contains an invalid instance host", attrsToJSON(attrs)); + Input input{}; input.attrs = attrs; return input;