From 668377f217c0fa4053d746f7094dfe887e07887c Mon Sep 17 00:00:00 2001 From: John Ericson Date: Mon, 17 Apr 2023 19:02:45 -0400 Subject: [PATCH] `TextHashMethod` -> `TextIngestionMethod`, gate with XP feature I suppose we can use `dynamic-derivations` for the few things we neeed. --- src/libexpr/primops.cc | 13 ++++++++++--- src/libstore/build/local-derivation-goal.cc | 4 ++-- src/libstore/content-address.cc | 14 +++++++------- src/libstore/content-address.hh | 4 ++-- src/libstore/daemon.cc | 2 +- src/libstore/derivations.cc | 2 ++ src/libstore/derivations.hh | 14 ++++++++------ src/libstore/remote-store.cc | 4 ++-- src/libutil/experimental-features.cc | 12 +++++++++++- src/libutil/experimental-features.hh | 1 + tests/ca/text-hashed-output.sh | 12 +++++++----- 11 files changed, 53 insertions(+), 29 deletions(-) diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index 2476b7e73..fc397db33 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -1105,8 +1105,10 @@ drvName, Bindings * attrs, Value & v) auto handleHashMode = [&](const std::string_view s) { if (s == "recursive") ingestionMethod = FileIngestionMethod::Recursive; else if (s == "flat") ingestionMethod = FileIngestionMethod::Flat; - else if (s == "text") ingestionMethod = TextHashMethod {}; - else + else if (s == "text") { + experimentalFeatureSettings.require(Xp::DynamicDerivations); + ingestionMethod = TextIngestionMethod {}; + } else state.debugThrowLastTrace(EvalError({ .msg = hintfmt("invalid value '%s' for 'outputHashMode' attribute", s), .errPos = state.positions[noPos] @@ -1274,11 +1276,16 @@ drvName, Bindings * attrs, Value & v) })); /* Check whether the derivation name is valid. */ - if (isDerivation(drvName) && ingestionMethod != ContentAddressMethod { TextHashMethod { } }) + if (isDerivation(drvName) && + !(ingestionMethod == ContentAddressMethod { TextIngestionMethod { } } && + outputs.size() == 1 && + *(outputs.begin()) == "out")) + { state.debugThrowLastTrace(EvalError({ .msg = hintfmt("derivation names are allowed to end in '%s' only if they produce a single derivation file", drvExtension), .errPos = state.positions[noPos] })); + } if (outputHash) { /* Handle fixed-output derivations. diff --git a/src/libstore/build/local-derivation-goal.cc b/src/libstore/build/local-derivation-goal.cc index 0a208750e..7a424fbfc 100644 --- a/src/libstore/build/local-derivation-goal.cc +++ b/src/libstore/build/local-derivation-goal.cc @@ -2427,7 +2427,7 @@ SingleDrvOutputs LocalDerivationGoal::registerOutputs() "output path %1% without valid stats info", actualPath); if (outputHash.method == ContentAddressMethod { FileIngestionMethod::Flat } || - outputHash.method == ContentAddressMethod { TextHashMethod {} }) + outputHash.method == ContentAddressMethod { TextIngestionMethod {} }) { /* The output path should be a regular file without execute permission. */ if (!S_ISREG(st->st_mode) || (st->st_mode & S_IXUSR) != 0) @@ -2441,7 +2441,7 @@ SingleDrvOutputs LocalDerivationGoal::registerOutputs() std::string oldHashPart { scratchPath->hashPart() }; HashModuloSink caSink { outputHash.hashType, oldHashPart }; std::visit(overloaded { - [&](const TextHashMethod &) { + [&](const TextIngestionMethod &) { readFile(actualPath, caSink); }, [&](const FileIngestionMethod & m2) { diff --git a/src/libstore/content-address.cc b/src/libstore/content-address.cc index 8c04dd285..2bde23e79 100644 --- a/src/libstore/content-address.cc +++ b/src/libstore/content-address.cc @@ -23,7 +23,7 @@ std::string makeFileIngestionPrefix(FileIngestionMethod m) std::string ContentAddressMethod::renderPrefix() const { return std::visit(overloaded { - [](TextHashMethod) -> std::string { return "text:"; }, + [](TextIngestionMethod) -> std::string { return "text:"; }, [](FileIngestionMethod m2) { /* Not prefixed for back compat with things that couldn't produce text before. */ return makeFileIngestionPrefix(m2); @@ -37,7 +37,7 @@ ContentAddressMethod ContentAddressMethod::parsePrefix(std::string_view & m) if (splitPrefix(m, "r:")) method = FileIngestionMethod::Recursive; else if (splitPrefix(m, "text:")) - method = TextHashMethod {}; + method = TextIngestionMethod {}; return method; } @@ -59,7 +59,7 @@ std::string ContentAddress::render() const std::string ContentAddressMethod::render(HashType ht) const { return std::visit(overloaded { - [&](const TextHashMethod & th) { + [&](const TextIngestionMethod & th) { return std::string{"text:"} + printHashType(ht); }, [&](const FileIngestionMethod & fim) { @@ -96,7 +96,7 @@ static std::pair parseContentAddressMethodPrefix // No parsing of the ingestion method, "text" only support flat. HashType hashType = parseHashType_(); return { - TextHashMethod {}, + TextIngestionMethod {}, std::move(hashType), }; } else if (prefix == "fixed") { @@ -120,7 +120,7 @@ ContentAddress ContentAddress::parse(std::string_view rawCa) { auto hashType = hashType_; // work around clang bug return std::visit(overloaded { - [&](TextHashMethod &) { + [&](TextIngestionMethod &) { return ContentAddress(TextHash { .hash = Hash::parseNonSRIUnprefixed(rest, hashType) }); @@ -158,7 +158,7 @@ ContentAddressWithReferences ContentAddressWithReferences::fromParts( ContentAddressMethod method, Hash hash, StoreReferences refs) { return std::visit(overloaded { - [&](TextHashMethod _) -> ContentAddressWithReferences { + [&](TextIngestionMethod _) -> ContentAddressWithReferences { if (refs.self) throw UsageError("Cannot have a self reference with text hashing scheme"); return TextInfo { @@ -182,7 +182,7 @@ ContentAddressMethod ContentAddressWithReferences::getMethod() const { return std::visit(overloaded { [](const TextInfo & th) -> ContentAddressMethod { - return TextHashMethod {}; + return TextIngestionMethod {}; }, [](const FixedOutputInfo & fsh) -> ContentAddressMethod { return fsh.hash.method; diff --git a/src/libstore/content-address.hh b/src/libstore/content-address.hh index 962b63e83..8668acacf 100644 --- a/src/libstore/content-address.hh +++ b/src/libstore/content-address.hh @@ -22,7 +22,7 @@ namespace nix { * Somewhat obscure, used by \ref Derivation derivations and * `builtins.toFile` currently. */ -struct TextHashMethod : std::monostate { }; +struct TextIngestionMethod : std::monostate { }; /** * An enumeration of the main ways we can serialize file system @@ -57,7 +57,7 @@ std::string makeFileIngestionPrefix(FileIngestionMethod m); struct ContentAddressMethod { typedef std::variant< - TextHashMethod, + TextIngestionMethod, FileIngestionMethod > Raw; diff --git a/src/libstore/daemon.cc b/src/libstore/daemon.cc index d3b9988c9..31e2e2af5 100644 --- a/src/libstore/daemon.cc +++ b/src/libstore/daemon.cc @@ -406,7 +406,7 @@ static void performOp(TunnelLogger * logger, ref store, FramedSource source(from); // TODO this is essentially RemoteStore::addCAToStore. Move it up to Store. return std::visit(overloaded { - [&](const TextHashMethod &) { + [&](const TextIngestionMethod &) { if (hashType != htSHA256) throw UnimplementedError("Only SHA-256 is supported for adding text-hashed data, but '%1' was given", printHashType(hashType)); diff --git a/src/libstore/derivations.cc b/src/libstore/derivations.cc index fc76ae7ad..1f5f78964 100644 --- a/src/libstore/derivations.cc +++ b/src/libstore/derivations.cc @@ -216,6 +216,8 @@ static DerivationOutput parseDerivationOutput(const Store & store, { if (hashAlgo != "") { ContentAddressMethod method = ContentAddressMethod::parsePrefix(hashAlgo); + if (method == TextIngestionMethod {}) + experimentalFeatureSettings.require(Xp::DynamicDerivations); const auto hashType = parseHashType(hashAlgo); if (hashS == "impure") { experimentalFeatureSettings.require(Xp::ImpureDerivations); diff --git a/src/libstore/derivations.hh b/src/libstore/derivations.hh index 65901ec6d..b36e4ea91 100644 --- a/src/libstore/derivations.hh +++ b/src/libstore/derivations.hh @@ -339,12 +339,14 @@ struct Derivation : BasicDerivation Store & store, const std::map, StorePath> & inputDrvOutputs) const; - /* Check that the derivation is valid and does not present any - illegal states. - - This is mainly a matter of checking the outputs, where our C++ - representation supports all sorts of combinations we do not yet - allow. */ + /** + * Check that the derivation is valid and does not present any + * illegal states. + * + * This is mainly a matter of checking the outputs, where our C++ + * representation supports all sorts of combinations we do not yet + * allow. + */ void checkInvariants(Store & store, const StorePath & drvPath) const; Derivation() = default; diff --git a/src/libstore/remote-store.cc b/src/libstore/remote-store.cc index ff5348830..b3f5251f2 100644 --- a/src/libstore/remote-store.cc +++ b/src/libstore/remote-store.cc @@ -629,7 +629,7 @@ ref RemoteStore::addCAToStore( if (repair) throw Error("repairing is not supported when building through the Nix daemon protocol < 1.25"); std::visit(overloaded { - [&](const TextHashMethod & thm) -> void { + [&](const TextIngestionMethod & thm) -> void { if (hashType != htSHA256) throw UnimplementedError("Only SHA-256 is supported for adding text-hashed data, but '%1' was given", printHashType(hashType)); @@ -782,7 +782,7 @@ StorePath RemoteStore::addTextToStore( RepairFlag repair) { StringSource source(s); - return addCAToStore(source, name, TextHashMethod {}, htSHA256, references, repair)->path; + return addCAToStore(source, name, TextIngestionMethod {}, htSHA256, references, repair)->path; } void RemoteStore::registerDrvOutput(const Realisation & info) diff --git a/src/libutil/experimental-features.cc b/src/libutil/experimental-features.cc index bd1899662..ad0ec0427 100644 --- a/src/libutil/experimental-features.cc +++ b/src/libutil/experimental-features.cc @@ -12,7 +12,7 @@ struct ExperimentalFeatureDetails std::string_view description; }; -constexpr std::array xpFeatureDetails = {{ +constexpr std::array xpFeatureDetails = {{ { .tag = Xp::CaDerivations, .name = "ca-derivations", @@ -199,6 +199,16 @@ constexpr std::array xpFeatureDetails = {{ networking. )", }, + { + .tag = Xp::DynamicDerivations, + .name = "dynamic-derivations", + .description = R"( + Allow the use of a few things related to dynamic derivations: + + - "text hashing" derivation outputs, so we can build .drv + files. + )", + }, }}; static_assert( diff --git a/src/libutil/experimental-features.hh b/src/libutil/experimental-features.hh index 3c00bc4e5..409100592 100644 --- a/src/libutil/experimental-features.hh +++ b/src/libutil/experimental-features.hh @@ -29,6 +29,7 @@ enum struct ExperimentalFeature Cgroups, DiscardReferences, DaemonTrustOverride, + DynamicDerivations, }; /** diff --git a/tests/ca/text-hashed-output.sh b/tests/ca/text-hashed-output.sh index bbe5de763..dcb7e1e96 100644 --- a/tests/ca/text-hashed-output.sh +++ b/tests/ca/text-hashed-output.sh @@ -2,8 +2,10 @@ source common.sh -# Globally enable the ca derivations experimental flag -sed -i 's/experimental-features = .*/& ca-derivations ca-references/' "$NIX_CONF_DIR/nix.conf" +# Globally enable dynamic-derivations in addition to CA derivations +enableFeatures "dynamic-derivations" + +restartDaemon # In the corresponding nix file, we have two derivations: the first, named root, # is a normal recursive derivation, while the second, named dependent, has the @@ -15,13 +17,13 @@ sed -i 's/experimental-features = .*/& ca-derivations ca-references/' "$NIX_CONF # - build the dependent derivation # - check that the path of the output coincides with that of the original derivation -drv=$(nix-instantiate --experimental-features ca-derivations ./text-hashed-output.nix -A root) +drv=$(nix-instantiate ./text-hashed-output.nix -A root) nix show-derivation "$drv" -drvDep=$(nix-instantiate --experimental-features ca-derivations ./text-hashed-output.nix -A dependent) +drvDep=$(nix-instantiate ./text-hashed-output.nix -A dependent) nix show-derivation "$drvDep" -out1=$(nix-build --experimental-features ca-derivations ./text-hashed-output.nix -A dependent --no-out-link) +out1=$(nix-build ./text-hashed-output.nix -A dependent --no-out-link) nix path-info $drv --derivation --json | jq nix path-info $out1 --derivation --json | jq