From bea69509a28a81e4e8c3140a94404c86d4793ad0 Mon Sep 17 00:00:00 2001
From: Qyriad <qyriad@qyriad.me>
Date: Fri, 31 May 2024 17:01:12 -0600
Subject: [PATCH] allow flakes to trivially eval to attrsets

flake.nix has historically been required to be a literal attribute set
expression. This commit relaxes that requirement that it must trivially
evaluate to an attribute set. Thunks are still not allowed, so imports,
most function applications, etc are all still disallowed, but things
like trivial let expressions are.

I love deleting code ^_^

Change-Id: I3b66e2bacb58d1deadfb4a768cc3a356638aeec3
---
 src/libexpr/eval.cc                      | 14 +++-----
 src/libexpr/eval.hh                      | 10 +++---
 src/libexpr/flake/flake.cc               |  8 +++--
 tests/functional/flakes/trivial-flake.sh | 45 ++++++++++++++++++++++++
 tests/functional/meson.build             |  1 +
 5 files changed, 60 insertions(+), 18 deletions(-)
 create mode 100644 tests/functional/flakes/trivial-flake.sh

diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc
index c72b69af2..7520e95a9 100644
--- a/src/libexpr/eval.cc
+++ b/src/libexpr/eval.cc
@@ -1098,7 +1098,7 @@ Value * ExprPath::maybeThunk(EvalState & state, Env & env)
 }
 
 
-void EvalState::evalFile(const SourcePath & path_, Value & v, bool mustBeTrivial)
+void EvalState::evalFile(const SourcePath & path_, Value & v)
 {
     auto path = checkSourcePath(path_);
 
@@ -1124,7 +1124,7 @@ void EvalState::evalFile(const SourcePath & path_, Value & v, bool mustBeTrivial
     if (!e)
         e = parseExprFromFile(checkSourcePath(resolvedPath));
 
-    cacheFile(path, resolvedPath, e, v, mustBeTrivial);
+    cacheFile(path, resolvedPath, e, v);
 }
 
 
@@ -1139,8 +1139,8 @@ void EvalState::cacheFile(
     const SourcePath & path,
     const SourcePath & resolvedPath,
     Expr * e,
-    Value & v,
-    bool mustBeTrivial)
+    Value & v
+)
 {
     fileParseCache[resolvedPath] = e;
 
@@ -1153,12 +1153,6 @@ void EvalState::cacheFile(
                 e->getPos() ? std::make_shared<Pos>(positions[e->getPos()]) : nullptr,
                 "while evaluating the file '%1%':", resolvedPath.to_string())
             : nullptr;
-
-        // Enforce that 'flake.nix' is a direct attrset, not a
-        // computation.
-        if (mustBeTrivial &&
-            !(dynamic_cast<ExprAttrs *>(e)))
-            error<EvalError>("file '%s' must be an attribute set", path).debugThrow();
         eval(e, v);
     } catch (Error & e) {
         addErrorTrace(e, "while evaluating the file '%1%':", resolvedPath.to_string());
diff --git a/src/libexpr/eval.hh b/src/libexpr/eval.hh
index 411364d9f..04f8cc2c9 100644
--- a/src/libexpr/eval.hh
+++ b/src/libexpr/eval.hh
@@ -352,11 +352,9 @@ public:
     Expr * parseStdin();
 
     /**
-     * Evaluate an expression read from the given file to normal
-     * form. Optionally enforce that the top-level expression is
-     * trivial (i.e. doesn't require arbitrary computation).
+     * Evaluate an expression read from the given file to normal form.
      */
-    void evalFile(const SourcePath & path, Value & v, bool mustBeTrivial = false);
+    void evalFile(const SourcePath & path, Value & v);
 
     /**
      * Like `evalFile`, but with an already parsed expression.
@@ -365,8 +363,8 @@ public:
         const SourcePath & path,
         const SourcePath & resolvedPath,
         Expr * e,
-        Value & v,
-        bool mustBeTrivial = false);
+        Value & v
+    );
 
     void resetFileCache();
 
diff --git a/src/libexpr/flake/flake.cc b/src/libexpr/flake/flake.cc
index ac50351ad..13cc29d4c 100644
--- a/src/libexpr/flake/flake.cc
+++ b/src/libexpr/flake/flake.cc
@@ -233,9 +233,13 @@ static Flake getFlake(
         .sourceInfo = std::make_shared<fetchers::Tree>(std::move(sourceInfo))
     };
 
-    // NOTE evalFile forces vInfo to be an attrset because mustBeTrivial is true.
     Value vInfo;
-    state.evalFile(CanonPath(flakeFile), vInfo, true); // FIXME: symlink attack
+    state.evalFile(CanonPath(flakeFile), vInfo); // FIXME: symlink attack
+    if (vInfo.type() != nAttrs) {
+        state.error<EvalError>(
+            "file '%s' must be an attribute set, but is %s", flakeFile, showType(vInfo)
+        ).debugThrow();
+    }
 
     if (auto description = vInfo.attrs->get(state.sDescription)) {
         expectType(state, nString, *description->value, description->pos);
diff --git a/tests/functional/flakes/trivial-flake.sh b/tests/functional/flakes/trivial-flake.sh
new file mode 100644
index 000000000..d30bcd6a0
--- /dev/null
+++ b/tests/functional/flakes/trivial-flake.sh
@@ -0,0 +1,45 @@
+# Allow trivial evaluation in flake metadata.
+source ./common.sh
+
+clearStore
+
+flakeLetDir="$TEST_ROOT/flake-let"
+flakeInterpDir="$TEST_ROOT/flake-interp"
+
+for flakeDir in "$flakeLetDir" "$flakeInterpDir"; do
+  mkdir -vp "$flakeDir"
+done
+
+# let is trivial; this should succeed
+cat > "$flakeLetDir/flake.nix" <<EOF
+let
+  description = "meow";
+in {
+  inherit description;
+  inputs = { };
+
+  outputs = { self }: {
+  };
+}
+EOF
+
+# string interpolation thunks; this should fail
+cat > "$flakeInterpDir/flake.nix" <<EOF
+let
+  lix-forgejo = "git+https://git.lix.systems";
+in {
+  inputs = {
+    lix.url = "\${lix-forgejo}/lix-project/lix";
+  };
+
+  outputs = { self, lix }: {
+  };
+}
+EOF
+
+(cd "$flakeLetDir" && nix flake show)
+
+(
+  cd "$flakeInterpDir"
+  expectStderr 1 nix flake show | grepQuiet "error: expected a string but got a thunk"
+)
diff --git a/tests/functional/meson.build b/tests/functional/meson.build
index a13dee001..567cc5c29 100644
--- a/tests/functional/meson.build
+++ b/tests/functional/meson.build
@@ -174,6 +174,7 @@ functional_tests_scripts = [
   'fetchClosure.sh',
   'completions.sh',
   'flakes/show.sh',
+  'flakes/trivial-flake.sh',
   'impure-derivations.sh',
   'path-from-hash-part.sh',
   'toString-path.sh',