aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJohn Ericson <John.Ericson@Obsidian.Systems>2023-01-10 17:18:59 -0500
committerJohn Ericson <John.Ericson@Obsidian.Systems>2023-05-15 09:03:38 -0400
commitd2162e7acdad4818788d229ed2d4600412b2be52 (patch)
treea9c989bec1f54c60a4e6dab1111f55a2d3b0da7f
parent5a23b80b0aa711877a05b18e2f8f221f8bd4844a (diff)
Make more string values work as installables
As discussed in #7417, it would be good to make more string values work as installables. That is to say, if an installable refers to a value, and the value is a string, it used to not work at all, since #7484, it works somewhat, and this PR make it work some more. The new cases that are added for `BuiltPath` contexts: - Fixed input- or content-addressed derivation: ``` nix-repl> hello.out.outPath "/nix/store/jppfl2bp1zhx8sgs2mgifmsx6dv16mv2-hello-2.12" nix-repl> :p builtins.getContext hello.out.outPath { "/nix/store/c7jrxqjhdda93lhbkanqfs07x2bzazbm-hello-2.12.drv" = { outputs = [ "out" ]; }; } The string matches the specified single output of that derivation, so it should also be valid. - Floating content-addressed derivation: ``` nix-repl> (hello.overrideAttrs (_: { __contentAddressed = true; })).out.outPath "/1a08j26xqc0zm8agps8anxpjji410yvsx4pcgyn4bfan1ddkx2g0" nix-repl> :p builtins.getContext (hello.overrideAttrs (_: { __contentAddressed = true; })).out.outPath { "/nix/store/qc645pyf9wl37c6qvqzaqkwsm1gp48al-hello-2.12.drv" = { outputs = [ "out" ]; }; } ``` The string is not a path but a placeholder, however it also matches the context, and because it is a CA derivation we have no better option. This should also be valid. We may also want to think about richer attrset based values (also discussed in that issue and #6507), but this change "completes" our string-based building blocks, from which the others can be desugared into or at least described/document/taught in terms of. Progress towards #7417 Co-authored-by: Robert Hensing <roberth@users.noreply.github.com>
-rw-r--r--src/libcmd/installable-attr-path.cc10
-rw-r--r--src/libcmd/installable-flake.cc30
-rw-r--r--src/libcmd/installable-value.cc22
-rw-r--r--src/libcmd/installable-value.hh18
-rw-r--r--tests/build.sh24
-rw-r--r--tests/flakes/build-paths.sh32
6 files changed, 110 insertions, 26 deletions
diff --git a/src/libcmd/installable-attr-path.cc b/src/libcmd/installable-attr-path.cc
index cf513126d..b35ca2910 100644
--- a/src/libcmd/installable-attr-path.cc
+++ b/src/libcmd/installable-attr-path.cc
@@ -46,7 +46,15 @@ std::pair<Value *, PosIdx> InstallableAttrPath::toValue(EvalState & state)
DerivedPathsWithInfo InstallableAttrPath::toDerivedPaths()
{
- auto v = toValue(*state).first;
+ auto [v, pos] = toValue(*state);
+
+ if (std::optional derivedPathWithInfo = trySinglePathToDerivedPaths(
+ *v,
+ pos,
+ fmt("while evaluating the attribute '%s'", attrPath)))
+ {
+ return { *derivedPathWithInfo };
+ }
Bindings & autoArgs = *cmd.getAutoArgs(*state);
diff --git a/src/libcmd/installable-flake.cc b/src/libcmd/installable-flake.cc
index 37e59cfdf..eb944240b 100644
--- a/src/libcmd/installable-flake.cc
+++ b/src/libcmd/installable-flake.cc
@@ -95,31 +95,13 @@ DerivedPathsWithInfo InstallableFlake::toDerivedPaths()
// FIXME: use eval cache?
auto v = attr->forceValue();
- if (v.type() == nPath) {
- auto storePath = v.path().fetchToStore(state->store);
- return {{
- .path = DerivedPath::Opaque {
- .path = std::move(storePath),
- },
- .info = make_ref<ExtraPathInfo>(),
- }};
- }
-
- else if (v.type() == nString) {
- NixStringContext context;
- auto s = state->forceString(v, context, noPos, fmt("while evaluating the flake output attribute '%s'", attrPath));
- auto storePath = state->store->maybeParseStorePath(s);
- if (storePath && context.count(NixStringContextElem::Opaque { .path = *storePath })) {
- return {{
- .path = DerivedPath::Opaque {
- .path = std::move(*storePath),
- },
- .info = make_ref<ExtraPathInfo>(),
- }};
- } else
- throw Error("flake output attribute '%s' evaluates to the string '%s' which is not a store path", attrPath, s);
+ if (std::optional derivedPathWithInfo = trySinglePathToDerivedPaths(
+ v,
+ noPos,
+ fmt("while evaluating the flake output attribute '%s'", attrPath)))
+ {
+ return { *derivedPathWithInfo };
}
-
else
throw Error("flake output attribute '%s' is not a derivation or path", attrPath);
}
diff --git a/src/libcmd/installable-value.cc b/src/libcmd/installable-value.cc
index 3a7ede4e2..1eff293cc 100644
--- a/src/libcmd/installable-value.cc
+++ b/src/libcmd/installable-value.cc
@@ -41,4 +41,26 @@ ref<InstallableValue> InstallableValue::require(ref<Installable> installable)
return ref { castedInstallable };
}
+std::optional<DerivedPathWithInfo> InstallableValue::trySinglePathToDerivedPaths(Value & v, const PosIdx pos, std::string_view errorCtx)
+{
+ if (v.type() == nPath) {
+ auto storePath = v.path().fetchToStore(state->store);
+ return {{
+ .path = DerivedPath::Opaque {
+ .path = std::move(storePath),
+ },
+ .info = make_ref<ExtraPathInfo>(),
+ }};
+ }
+
+ else if (v.type() == nString) {
+ return {{
+ .path = state->coerceToDerivedPath(pos, v, errorCtx),
+ .info = make_ref<ExtraPathInfo>(),
+ }};
+ }
+
+ else return std::nullopt;
+}
+
}
diff --git a/src/libcmd/installable-value.hh b/src/libcmd/installable-value.hh
index 5ab7eee16..3138ce8ec 100644
--- a/src/libcmd/installable-value.hh
+++ b/src/libcmd/installable-value.hh
@@ -98,6 +98,24 @@ struct InstallableValue : Installable
static InstallableValue & require(Installable & installable);
static ref<InstallableValue> require(ref<Installable> installable);
+
+protected:
+
+ /**
+ * Handles either a plain path, or a string with a single string
+ * context elem in the right format. The latter case is handled by
+ * `EvalState::coerceToDerivedPath()`; see it for details.
+ *
+ * @param v Value that is hopefully a string or path per the above.
+ *
+ * @param pos Position of value to aid with diagnostics.
+ *
+ * @param errorCtx Arbitrary message for use in potential error message when something is wrong with `v`.
+ *
+ * @result A derived path (with empty info, for now) if the value
+ * matched the above criteria.
+ */
+ std::optional<DerivedPathWithInfo> trySinglePathToDerivedPaths(Value & v, const PosIdx pos, std::string_view errorCtx);
};
}
diff --git a/tests/build.sh b/tests/build.sh
index b579fc374..697aff0f9 100644
--- a/tests/build.sh
+++ b/tests/build.sh
@@ -57,6 +57,30 @@ nix build -f multiple-outputs.nix --json 'e^*' --no-link | jq --exit-status '
(.outputs | keys == ["a_a", "b", "c"]))
'
+# test buidling from non-drv attr path
+
+nix build -f multiple-outputs.nix --json 'e.a_a.outPath' --no-link | jq --exit-status '
+ (.[0] |
+ (.drvPath | match(".*multiple-outputs-e.drv")) and
+ (.outputs | keys == ["a_a"]))
+'
+
+# Illegal type of string context
+expectStderr 1 nix build -f multiple-outputs.nix 'e.a_a.drvPath' \
+ | grepQuiet "has a context which refers to a complete source and binary closure."
+
+# No string context
+expectStderr 1 nix build --expr '""' --no-link \
+ | grepQuiet "has 0 entries in its context. It should only have exactly one entry"
+
+# Too much string context
+expectStderr 1 nix build --impure --expr 'with (import ./multiple-outputs.nix).e.a_a; "${drvPath}${outPath}"' --no-link \
+ | grepQuiet "has 2 entries in its context. It should only have exactly one entry"
+
+nix build --impure --json --expr 'builtins.unsafeDiscardOutputDependency (import ./multiple-outputs.nix).e.a_a.drvPath' --no-link | jq --exit-status '
+ (.[0] | .path | match(".*multiple-outputs-e.drv"))
+'
+
# Test building from raw store path to drv not expression.
drv=$(nix eval -f multiple-outputs.nix --raw a.drvPath)
diff --git a/tests/flakes/build-paths.sh b/tests/flakes/build-paths.sh
index b399a066e..ff012e1b3 100644
--- a/tests/flakes/build-paths.sh
+++ b/tests/flakes/build-paths.sh
@@ -41,10 +41,27 @@ cat > $flake1Dir/flake.nix <<EOF
a8 = builtins.storePath $dep;
a9 = "$dep";
+
+ drvCall = with import ./config.nix; mkDerivation {
+ name = "simple";
+ builder = ./simple.builder.sh;
+ PATH = "";
+ goodPath = path;
+ };
+
+ a10 = builtins.unsafeDiscardOutputDependency self.drvCall.drvPath;
+
+ a11 = self.drvCall.drvPath;
+
+ a12 = self.drvCall.outPath;
+
+ a13 = "\${self.drvCall.drvPath}\${self.drvCall.outPath}";
};
}
EOF
+cp ../simple.nix ../simple.builder.sh ../config.nix $flake1Dir/
+
echo bar > $flake1Dir/foo
nix build --json --out-link $TEST_ROOT/result $flake1Dir#a1
@@ -63,4 +80,17 @@ nix build --json --out-link $TEST_ROOT/result $flake1Dir#a6
nix build --impure --json --out-link $TEST_ROOT/result $flake1Dir#a8
diff common.sh $TEST_ROOT/result
-(! nix build --impure --json --out-link $TEST_ROOT/result $flake1Dir#a9)
+expectStderr 1 nix build --impure --json --out-link $TEST_ROOT/result $flake1Dir#a9 \
+ | grepQuiet "has 0 entries in its context. It should only have exactly one entry"
+
+nix build --json --out-link $TEST_ROOT/result $flake1Dir#a10
+[[ $(readlink -e $TEST_ROOT/result) = *simple.drv ]]
+
+expectStderr 1 nix build --json --out-link $TEST_ROOT/result $flake1Dir#a11 \
+ | grepQuiet "has a context which refers to a complete source and binary closure"
+
+nix build --json --out-link $TEST_ROOT/result $flake1Dir#a12
+[[ -e $TEST_ROOT/result/hello ]]
+
+expectStderr 1 nix build --impure --json --out-link $TEST_ROOT/result $flake1Dir#a13 \
+ | grepQuiet "has 2 entries in its context. It should only have exactly one entry"