2024-08-07 Nix team meeting minutes #167

Development Nix
1

In this meeting we’ve done a small amount of triaging and discussed progress towards fetchTree stabilisation, both because we need to plan what to do after the Meson transition is complete, and because it comes up in issues and contributor PRs: a lot hinges on it, specifically:

  • flakes stabilisation
  • PRs to improve fetchTree and/or flake inputs semantics

Last Monday’s meeting was skipped.

Attendees: @edolstra, @roberth, @tomberek, @Ericson2314, guests: @L-as, @fzakaria

docs: add language/string-literals.md by rhendric · Pull Request #11233 · NixOS/nix · GitHub

Previously reviewed by @fricklerhandwerk. Conflict resolved; merging.

Don't copy nix store path to nix store by yshui · Pull Request #11229 · NixOS/nix · GitHub

@yshui So reading this more yeah you are raising a good point with this PR — I am uncomfortable with the change but I am also unfortable with the status quo :).

I think what would be good is to finish Lazy `fetchTree` `outPath` path values by roberth · Pull Request #10252 · NixOS/nix · GitHub first, after which a bunch of this code should change anyways (because we are decoupling “fetching” from “adding to the store”).

Lazy `fetchTree` `outPath` path values by roberth · Pull Request #10252 · NixOS/nix · GitHub

Things to test and try out:

  • check differences with flakes in various scenarios

    • expectation: isPath self.outPath now returns true
    • expectation: "${self.outPath}" remains the same

  • measure performance impact

    • expectation: e.g. NixOS/nix flake copies about half as much to the store
    • expectation: Nixpkgs is still copied wholesale when used with NixOS, or perhaps meta.location, but not in nix build nixpkgs#hello

  • consider a inputs.THING.lazy-paths=true; or nixConfig.lazy-paths = true; to control behavior

    • @roberth: I’d prefer lazy-path = true; at top level, because it is not a global config. It only affects the current flake.nix and the outPath in its outputs. Can settle for inputs.self.lazyPath for backcompat, though it’s not a fetchTree parameter (and shouldn’t be)

  • consider flakeurl?lazy=true

    • @roberth: This would be redundant and hard to get rid of. We should prefer composition over extra parameters where possible
      • fetchTree { ... } for returning a path
      • "${fetchTree { ... }" for the (currently strict) string case
      • let r = fetchTree { ... }; in r // { outPath = "${r}"; } for behavior identical to Nix <= 2.24

  • consider what the defaults should be

    • returning a path value in the flake outPath could be opt-in or opt-out, depending on our own testing with the flake regressions suite

  • consideration: @Ericson2314: Nixpkgs and NixOS modules don’t benefit from lazy-paths and would break ( this can change historical eval values ). There might be a situation where three types of paths need to be maintained.

    • @Ericson2314: eager or not-eager path-to-string conversion
    • does 10252 change historical eval of Nixpkgs? No. Because fetchTree is not used in Nixpkgs. But someone fetching Nixpkgs with fetchTree would experience different store paths? Probably not.
    • @edolstra: this PR does not solve the original issue of Nixpkgs performance improvement. We want to avoid 3 path types.
      1. lazy-paths: lazy-tree branch -
      2. fetch-tree: this pr -
      3. legacy-paths: legacy -
    • @roberth: lazy-trees needs lazy path semantics anyway
    • @roberth: need to distinguish absolute system paths vs lazy-paths
    • @Ericson2314: we may end up needing 3 variants anyway, thus this PR does not cause a back-compat problem
      • prove/disprove this? improve confidence?
      • @roberth: doesn’t lazy-trees already have the 3 variants?
        • path values with a lazy source accessor
        • virtual paths referring indirectly to a lazy source accessor
        • legacy “OS” absolute paths
    • @edolstra: flake inputs would be different from fetchTree?
      • we need to check flake regressions
      • path value instead of string
      • @roberth: confident that opt-in/out can recover the current behavior during transition
    • breakage: nix eval of consumer flakes might change when observing
      • eg: lib.hasPrefix path may change
      • if there is breakage, what exactly is it? “It can be a useful break.”
      • Flakes generally should not be used as paths
        • Current exception is Nixpkgs, but it already supports being imported from a path value (a non-flakes use case)

  • @Ericson2314 an observable change of changing the returned outPath to a path value is:

    "${foo + "/bar"}"

    today:

    "/nix/store/asdfasdfasdfasdfasdf-foo/bar"

    with this PR:

    "/nix/store/asdfasdfasdfasdfasdf-bar"
    • In the context of flakes, this only has an effect when
      • self is used as a string or path
      • inputs.x is used as a string or path

  • @Ericson2314: libutil layer violation: currently the PR adds a method that determines the toString semantics of a path value that uses the specific type of source accessor. This is a language-level concern.

    • @roberth: Could match on the SourceAccessor being a PosixSourceAccessor instead (or whichever C++ class name we pick for the system absolute paths)

    • Doesn’t that lead to problems for path: inputs?

    • @roberth: Those should not return bare PosixSourceAccessors anyway, because fetchTree should return only the referred tree, and not something where you can access things outside the actual tree as in dirOf (dirOf (fetchTree x)) + "/foo"
      This could perhaps be expressed equationally (at least it’s how I make sense of it)

      ToString(FetchTree("path:/nix/store/asdfasdf-source"))
  == ToString(Value(tPath, accessor: asdfasdf /* just the tree by this hash */, subpath:"/"))
  == storeDir + "/" + hash + "-source"
  == "/nix/store/asdfasdf-source"
      • FetchTree makes the original location unobservable, just as it does for e.g. git
      • ToString(Value(tPath, ...)): this PR defines lazy path values to be store paths (laziness is not observable), so accessing it produces a store path

  • (After meeting) @roberth: We don’t currently store the name in tValue or SourceAccessor, and I doubt that we should, so we might want to close Don't copy nix store path to nix store by yshui · Pull Request #11229 · NixOS/nix · GitHub, which would allow the name to be different. Note that lazy paths value already optimize away a copy, so unless you’re using the whole path, lazy paths achieves the same goal.

    • builtins.path can produce a store path string with the custom name, but it wouldn’t be part of the path value
      • for flake inputs with flake = false; this could be done by call-flake.nix

TODO

8 Likes
2

We also briefly reviewed fix: bash mangles flake ref completion by 0x5a4 · Pull Request #11240 · NixOS/nix · GitHub (see Tom’s comment)

3

I was very absent this meeting; thanks for recording me anyways :stuck_out_tongue: