Make more string values work as installables #7601

Ericson2314 · 2023-01-14T23:40:12Z

Motivation

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.

Context

See #7417, where these ideas were discussed.

The history is broken out to be legible. One can review commit-by-commit to see all the support infra put in place before it is finally used in the last commit.

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.

~~Depends on #7710~~

Checklist for maintainers

Maintainers: tick if completed or explain if not relevant

agreed on idea
agreed on implementation strategy
tests, as appropriate
- functional tests - tests/**.sh
- unit tests - src/*/tests
- integration tests
documentation in the manual
code and comments are self-explanatory
commit message explains why the change was made
new feature or bug fix: updated release notes

roberth · 2023-01-23T12:09:19Z

Seems like this would solve #7654 partially? Maybe allow the whole string to be printed in the repl and CLI?

src/libcmd/installables.cc

dpulls · 2023-04-21T06:15:01Z

🎉 All dependencies have been resolved !

roberth

Needs a bit of cleanup.

src/libcmd/installable-attr-path.cc

src/libexpr/eval.cc

roberth

Almost there 🚀

src/libcmd/installable-value.hh

tests/build.sh

This well help us with some unit testing

This gives us some round trips to test. `EvalState::coerceToDerivedPathUnchecked` is a factored out helper just for unit testing.

As discussed in NixOS#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 NixOS#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 NixOS#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 NixOS#7417 Co-authored-by: Robert Hensing <roberth@users.noreply.github.com>

nixos-discourse · 2023-05-15T13:46:36Z

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/2023-05-12-nix-team-meeting-minutes-54/28197/1

Since PR NixOS#7601, we've had enough flexibility in our types of installables that it is no longer needed. Progress towards NixOS#7261

Ericson2314 requested review from edolstra and thufschmitt as code owners January 14, 2023 23:40

Ericson2314 force-pushed the string-installables branch from 738c727 to 7e946b5 Compare January 16, 2023 13:28

thufschmitt assigned tomberek Jan 27, 2023

tomberek reviewed Jan 27, 2023

View reviewed changes