tvix/eval: incorrect infinite recursion while evaluating derivation-attr-paths.nix

#281
Opened by sterni at 2023-06-20T10·49+00

To reproduce:

> git checkout [r/6337](https://code.tvl.fyi/commit/?id=refs/r/6337)
> wget https://raw.githubusercontent.com/NixOS/cabal2nix/master/distribution-nixpkgs/derivation-attr-paths.nix
> cargo run --bin tvix -- --expr '(import ./derivation-attr-paths.nix) { } # uses <nixpkgs>
note: while evaluating this Nix code
     --> [code]:1:1
      |
1     | import ~/src/hs/cabal2nix/distribution-nixpkgs/derivation-attr-paths.nix {}
      | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
note: while evaluating this as native code (concatLists)
     --> /home/lukas/src/hs/cabal2nix/distribution-nixpkgs/derivation-attr-paths.nix:37:45
      |
37    |             ] else if recurseInto path x then lib.concatLists (
      |  _____________________________________________^
38    | |             lib.mapAttrsToList (n: go (path ++ [ n ])) x
39    | |           ) else [];
      | |___________^
note: while evaluating this Nix code
     --> /home/lukas/src/hs/cabal2nix/distribution-nixpkgs/derivation-attr-paths.nix:34:11
      |
34    | /           if !success then []
35    | |           else if lib.isDerivation value then [
36    | |             path
37    | |           ] else if recurseInto path x then lib.concatLists (
37    | |           ] else if recurseInto path x then lib.concatLists (
39    | |           ) else [];
      | |___________________^
note: while evaluating this as native code (force)
     --> /home/lukas/src/hs/cabal2nix/distribution-nixpkgs/derivation-attr-paths.nix:34:14
      |
34    |           if !success then []
      |              ^^^^^^^^
note: while evaluating this Nix code
     --> /home/lukas/src/hs/cabal2nix/distribution-nixpkgs/derivation-attr-paths.nix:29:20
      |
29    |           inherit (builtins.tryEval x)
      |                    ^^^^^^^^^^^^^^^^^^
error[E014]: infinite recursion encountered
     --> /home/lukas/src/hs/cabal2nix/distribution-nixpkgs/derivation-attr-paths.nix:29:20
      |
29    |           inherit (builtins.tryEval x)
      |                    ^^^^^^^^^^^^^^^^^^
      |                    |
      |                    was first requested to be evaluated here
      |                    but then requested again here during its own evaluation
      | 
     ::: /nix/var/nix/profiles/per-user/root/channels/vuizvui/nixpkgs/pkgs/top-level/all-packages.nix
      |
25280 |     mod_wsgi2 = throw "mod_wsgi2 has been removed since Python 2 is EOL. Use mod_wsgi3 instead";
      |                 -------------------------------------------------------------------------------
      |                 |     |
      |                 |     this lazily-evaluated code
      |                 which was instantiated here

  1. Minimal reproducer:

    let
      # Unless this throws, evaluation succeeds.
      x = throw "lol";
    
      # If `x` is inlined, evaluation succeeds.
      # If `builtins.tryEval` is floated into a `let`, evaluation succeeds.
      inherit (builtins.tryEval x) value success;
    in
    # Unless we select from the `builtins.tryEval` thunk twice, evaluation succeeds.
    [ success value ]
    

    sterni at 2023-07-13T17·13+00

  2. The issue is that we can't call builtins.tryEval on the same thunk twice. Tvix essentially compiles let inherit (expr) a b; in … as let a = (expr).a; b = (expr).b; in …, so there are two thunks calling builtins.tryEval on x in this example. This is actually correct and can be validated against C++ Nix by evaluating a similar expression containing a builtins.trace.

    A clearer way to illustrate the issue is:

    let x = throw "lol"; in builtins.map (f: f x) [ builtins.tryEval builtins.tryEval ]
    

    It seems that we need to add another thunk state that remembers failed evaluation for subsequent tryEval calls.

    sterni at 2023-07-13T17·26+00

  3. This seems fixed, I can't reproduce that specific infinite recursion anymore.

    flokli at 2023-10-05T20·55+00

  4. flokli closed this issue at 2023-10-05T20·56+00
  5. For the record, this was fixed in r/6650.

    sterni at 2023-11-15T21·29+00