Problems related to scopedImport

[infinisil]

Confusing free variables

A bit more problematic is the fact that files called with a scopedImport have free variables, which can be confusing as to where they're actually defined, compared to a function argument at the top where it's clear that the variables are passed in by a function call.

This is in my opinion the main reason we might consider removing it.

Doesn't allow file evaluation to be cached

Here's an example where scopedImport can't cache files the same way import can. This is how evaluation is cached with import:

# function.nix
let
  b = builtins.trace "calculating b" (1 + 1);
in { a }: a + b

# call-function.nix
import ./function.nix { a = 1; }
*
import ./function.nix { a = 1; }

Note how calculating b is only printed once:

$ nix-instantiate --eval call-function.nix
trace: calculating b
9

Trying the same with scopedImport:

# scoped.nix
let
  b = builtins.trace "calculating b" (1 + 1);
in a + b

# import-scoped.nix
builtins.scopedImport { a = 1; } ./scoped.nix
*
builtins.scopedImport { a = 1; } ./scoped.nix

Now calculating b is printed twice:

$ nix-instantiate --eval import-scoped.nix
trace: calculating b
trace: calculating b
9

But this still makes sense if you think of scopedImport as just importing the file as the function body, prepending an attribute argument to it and then calling it, so like this:

# uncached.nix
{ a }: # This line would be prepended implicitly
let
  b = builtins.trace "calculating b" (1 + 1);
in a + b

# call-uncached.nix
import ./uncached.nix { a = 1; }
*
import ./uncached.nix { a = 1; }

which will also calculate b twice:

$ nix-instantiate --eval call-uncached.nix
trace: calculating b
trace: calculating b
9

So I don't think these semantics are problematic, as long as the docs clearly explain them.

Very related is #6228 for caching function application.

Combined with the ability to shadow builtins

However another more fundamental problem is that builtins can be shadowed:

let
  import = file: "Not importing ${toString file}";
in import ./file.nix

Which when combined with scopedImport can make all import's not be able to cache evaluation anymore:

# expr.nix
builtins.trace "Evaluating expr.nix" {
  nested = import ./expr.nix;
}

# scopedImport.nix
let
  scope = {
    import = file: builtins.scopedImport scope file;
  };
in builtins.scopedImport scope ./expr.nix

# normalImport.nix
import ./expr.nix

$ nix-instantiate --eval scopedImport.nix -A nested.nested.nested
trace: Evaluating expr.nix
trace: Evaluating expr.nix
trace: Evaluating expr.nix
trace: Evaluating expr.nix
{ nested = <CODE>; }
$ nix-instantiate --eval normalImport.nix -A nested.nested.nested
trace: Evaluating expr.nix
{ nested = «repeated»; }

This will automatically become impossible once we fix the problem of being able to shadow builtins, so it's not a problem with scopedImport specifically.

[roberth]

scopedImport came with a wonderful commit message. Check it out c273c15

It has existed for almost a decade, so I don't think we're going to remove it, possibly ever. We might disallow it in a new language version, but that's a new feature that we're not pursuing at this time. Is that something you're interested in doing? I can't promise that scopedImport will be removed in a language version, but perhaps if you're interested in this kind of thing, you could help out with

• [RFC 0137] Nix language versioning rfcs#137

For a more immediately actionable improvement, perhaps you could contribute to the primop's documentation?

[infinisil]

Yeah I've seen the commit message and the RFC before 🙂. I don't have any time to dedicate towards this right now though, I only opened this issue to write down the results of my investigation and track these problems.