Merge pull request #8231 from obsidiansystems/add-derivation-xp-gates

Gate experimental features in `DerivationOutput::fromJSON`

src/libstore/derivations.cc

@@ -989,7 +989,8 @@ nlohmann::json DerivationOutput::toJSON(
 
 DerivationOutput DerivationOutput::fromJSON(
     const Store & store, std::string_view drvName, std::string_view outputName,
-    const nlohmann::json & _json)
+    const nlohmann::json & _json,
+    const ExperimentalFeatureSettings & xpSettings)
 {
     std::set<std::string_view> keys;
     auto json = (std::map<std::string, nlohmann::json>) _json;
@@ -1028,6 +1029,7 @@ DerivationOutput DerivationOutput::fromJSON(
     }
 
     else if (keys == (std::set<std::string_view> { "hashAlgo" })) {
+        xpSettings.require(Xp::CaDerivations);
         auto [method, hashType] = methodAlgo();
         return DerivationOutput::CAFloating {
             .method = method,
@@ -1040,6 +1042,7 @@ DerivationOutput DerivationOutput::fromJSON(
     }
 
     else if (keys == (std::set<std::string_view> { "hashAlgo", "impure" })) {
+        xpSettings.require(Xp::ImpureDerivations);
         auto [method, hashType] = methodAlgo();
         return DerivationOutput::Impure {
             .method = method,

src/libstore/derivations.hh

@@ -136,11 +136,15 @@ struct DerivationOutput : _DerivationOutputRaw
         const Store & store,
         std::string_view drvName,
         std::string_view outputName) const;
+    /**
+     * @param xpSettings Stop-gap to avoid globals during unit tests.
+     */
     static DerivationOutput fromJSON(
         const Store & store,
         std::string_view drvName,
         std::string_view outputName,
-        const nlohmann::json & json);
+        const nlohmann::json & json,
+        const ExperimentalFeatureSettings & xpSettings = experimentalFeatureSettings);
 };
 
 typedef std::map<std::string, DerivationOutput> DerivationOutputs;

src/libstore/tests/derivation.cc

@@ -1,6 +1,7 @@
 #include <nlohmann/json.hpp>
 #include <gtest/gtest.h>
 
+#include "experimental-features.hh"
 #include "derivations.hh"
 
 #include "tests/libstore.hh"
@@ -9,10 +10,32 @@ namespace nix {
 
 class DerivationTest : public LibStoreTest
 {
+public:
+    /**
+     * 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;
 };
 
-#define TEST_JSON(NAME, STR, VAL, DRV_NAME, OUTPUT_NAME) \
+class CaDerivationTest : public DerivationTest
-    TEST_F(DerivationTest, DerivationOutput_ ## NAME ## _to_json) {    \
+{
+    void SetUp() override
+    {
+        mockXpSettings.set("experimental-features", "ca-derivations");
+    }
+};
+
+class ImpureDerivationTest : public DerivationTest
+{
+    void SetUp() override
+    {
+        mockXpSettings.set("experimental-features", "impure-derivations");
+    }
+};
+
+#define TEST_JSON(FIXTURE, NAME, STR, VAL, DRV_NAME, OUTPUT_NAME) \
+    TEST_F(FIXTURE, DerivationOutput_ ## NAME ## _to_json) {    \
         using nlohmann::literals::operator "" _json;           \
         ASSERT_EQ(                                             \
             STR ## _json,                                      \
@@ -22,7 +45,7 @@ class DerivationTest : public LibStoreTest
                 OUTPUT_NAME));                                 \
     }                                                          \
                                                                \
-    TEST_F(DerivationTest, DerivationOutput_ ## NAME ## _from_json) {  \
+    TEST_F(FIXTURE, DerivationOutput_ ## NAME ## _from_json) {  \
         using nlohmann::literals::operator "" _json;           \
         ASSERT_EQ(                                             \
             DerivationOutput { VAL },                          \
@@ -30,10 +53,11 @@ class DerivationTest : public LibStoreTest
                 *store,                                        \
                 DRV_NAME,                                      \
                 OUTPUT_NAME,                                   \
-                STR ## _json));                                \
+                STR ## _json,                                  \
+                mockXpSettings));                              \
     }
 
-TEST_JSON(inputAddressed,
+TEST_JSON(DerivationTest, inputAddressed,
     R"({
         "path": "/nix/store/c015dhfh5l0lp6wxyvdn7bmwhbbr6hr9-drv-name-output-name"
     })",
@@ -42,7 +66,7 @@ TEST_JSON(inputAddressed,
     }),
     "drv-name", "output-name")
 
-TEST_JSON(caFixed,
+TEST_JSON(DerivationTest, caFixed,
     R"({
         "hashAlgo": "r:sha256",
         "hash": "894517c9163c896ec31a2adbd33c0681fd5f45b2c0ef08a64c92a03fb97f390f",
@@ -56,7 +80,7 @@ TEST_JSON(caFixed,
     }),
     "drv-name", "output-name")
 
-TEST_JSON(caFloating,
+TEST_JSON(CaDerivationTest, caFloating,
     R"({
         "hashAlgo": "r:sha256"
     })",
@@ -66,12 +90,12 @@ TEST_JSON(caFloating,
     }),
     "drv-name", "output-name")
 
-TEST_JSON(deferred,
+TEST_JSON(DerivationTest, deferred,
     R"({ })",
     DerivationOutput::Deferred { },
     "drv-name", "output-name")
 
-TEST_JSON(impure,
+TEST_JSON(ImpureDerivationTest, impure,
     R"({
         "hashAlgo": "r:sha256",
         "impure": true

tests/ca/derivation-json.sh

@@ -16,6 +16,9 @@ drvPath3=$(nix derivation add --dry-run < $TEST_HOME/foo.json)
 # With --dry-run nothing is actually written
 [[ ! -e "$drvPath3" ]]
 
+# But the JSON is rejected without the experimental feature
+expectStderr 1 nix derivation add < $TEST_HOME/foo.json --experimental-features nix-command | grepQuiet "experimental Nix feature 'ca-derivations' is disabled"
+
 # Without --dry-run it is actually written
 drvPath4=$(nix derivation add < $TEST_HOME/foo.json)
 [[ "$drvPath4" = "$drvPath3" ]]

tests/impure-derivations.sh

@@ -10,6 +10,15 @@ clearStore
 # Basic test of impure derivations: building one a second time should not use the previous result.
 printf 0 > $TEST_ROOT/counter
 
+# `nix derivation add` with impure derivations work
+drvPath=$(nix-instantiate ./impure-derivations.nix -A impure)
+nix derivation show $drvPath | jq .[] > $TEST_HOME/impure-drv.json
+drvPath2=$(nix derivation add < $TEST_HOME/impure-drv.json)
+[[ "$drvPath" = "$drvPath2" ]]
+
+# But only with the experimental feature!
+expectStderr 1 nix derivation add < $TEST_HOME/impure-drv.json --experimental-features nix-command | grepQuiet "experimental Nix feature 'impure-derivations' is disabled"
+
 nix build --dry-run --json --file ./impure-derivations.nix impure.all
 json=$(nix build -L --no-link --json --file ./impure-derivations.nix impure.all)
 path1=$(echo $json | jq -r .[].outputs.out)