diff options
author | John Ericson <John.Ericson@Obsidian.Systems> | 2023-01-10 17:18:59 -0500 |
---|---|---|
committer | John Ericson <John.Ericson@Obsidian.Systems> | 2023-05-15 09:03:38 -0400 |
commit | d2162e7acdad4818788d229ed2d4600412b2be52 (patch) | |
tree | a9c989bec1f54c60a4e6dab1111f55a2d3b0da7f | |
parent | 5a23b80b0aa711877a05b18e2f8f221f8bd4844a (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.cc | 10 | ||||
-rw-r--r-- | src/libcmd/installable-flake.cc | 30 | ||||
-rw-r--r-- | src/libcmd/installable-value.cc | 22 | ||||
-rw-r--r-- | src/libcmd/installable-value.hh | 18 | ||||
-rw-r--r-- | tests/build.sh | 24 | ||||
-rw-r--r-- | tests/flakes/build-paths.sh | 32 |
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" |