2023-04-08 Nix team meeting minutes #136

thufschmitt · April 8, 2024, 3:33pm

Better support for printing large values in `nix repl` and error messages · Issue #9942 · NixOS/nix · GitHub

Better support for printing large values in `nix repl` and error messages

opened 07:30PM - 05 Feb 24 UTC

feature idea approved

### Is your feature request related to a problem? Please describe. Currently,… some error messages will print values in a "limited" fashion, after PRs like #9753. Using the value printer refactor from #9606, Nix will print at most 10 attributes, 10 list items, and 1024 bytes of each string in error messages before giving up and printing a message like `«20373 attributes elided»` to indicate the missing values: ``` $ nix repl nixpkgs Loading installable 'flake:nixpkgs#'... Added 5 variables. nix-repl> builtins.map (lib.id) legacyPackages.aarch64-darwin error: … while calling the 'map' builtin at «string»:1:1: 1| builtins.map (lib.id) legacyPackages.aarch64-darwin | ^ … while evaluating the second argument passed to builtins.map error: expected a list but found a set: { _type = "pkgs"; AAAAAASomeThingsFailToEvaluate = «thunk»; AMB-plugins = «thunk»; ArchiSteamFarm = «thunk»; AusweisApp2 = «thunk»; BeatSaberModManager = «thunk»; CHOWTapeModel = «thunk»; ChowCentaur = «thunk»; ChowKick = «thunk»; ChowPhaser = «thunk»; «20373 attributes elided» } ``` `nix repl` doesn't currently use this output-limiting functionality, but instead sets a maximum depth (of 1 level) to print to, with the `:p` command used to print the entire structure: ``` $ nix repl nix-repl> { a.b.c.d.e.f.g.h.i.j.k.l.m.n.o.p.q.r.s.t.u.v.w.x.y.z = 1; } { a = { ... }; } nix-repl> :p { a.b.c.d.e.f.g.h.i.j.k.l.m.n.o.p.q.r.s.t.u.v.w.x.y.z = 1; } { a = { b = { c = { d = { e = { f = { g = { h = { i = { j = { k = { l = { m = { n = { o = { p = { q = { r = { s = { t = { u = { v = { w = { x = { y = { z = 1; }; }; }; }; }; }; }; }; }; }; }; }; }; }; }; }; }; }; }; }; }; }; }; }; }; } ``` But the maximum depth of 1 still produces virtually unbounded output for large _flat_ attribute sets like `nixpkgs`. Evaluating `legacyPackages.aarch64-darwin` in `nix repl nixpkgs` will happily spend 4 minutes printing 400,000 lines of output, and from experience seems rather uninterested in stopping when Ctrl-C is pressed. ### Describe the solution you'd like I would like to set `nix repl` to pretty-print (as in #9931) a modest number of maximum attributes, list items, and strings. The printer will track if any values were omitted, and this information will be used to format a tip for the user. Roughly like this: ``` $ nix repl nixpkgs nix-repl> legacyPackages.aarch64-darwin { _type = "pkgs"; AAAAAASomeThingsFailToEvaluate = «thunk»; AMB-plugins = «thunk»; ArchiSteamFarm = «thunk»; AusweisApp2 = «thunk»; BeatSaberModManager = «thunk»; CHOWTapeModel = «thunk»; ChowCentaur = «thunk»; ChowKick = «thunk»; ChowPhaser = «thunk»; «20373 attributes elided» } # Note: Some attributes were elided. Print the whole value with `:p` or # increase `--option max-attrs-print ...` to see more of the value. ``` (NB: In practice, we'd want to print a decent amount of attributes, maybe 100, before eliding any.) Combined with #9922 and #9944, we could also add a REPL command to change the maximum values to be printed at runtime: ``` nix-repl> :set max-attrs-print 100 nix-repl> legacyPackages.aarch64-darwin # Print the first 100 attributes. ``` This could be used to improve error ergonomics as well, by providing an opt-in for printing more of failing values: ``` error: … while calling the 'map' builtin at «string»:1:1: 1| builtins.map (lib.id) legacyPackages.aarch64-darwin | ^ … while evaluating the second argument passed to builtins.map error: expected a list but found a set: { _type = "pkgs"; AAAAAASomeThingsFailToEvaluate = «thunk»; AMB-plugins = «thunk»; ArchiSteamFarm = «thunk»; AusweisApp2 = «thunk»; BeatSaberModManager = «thunk»; CHOWTapeModel = «thunk»; ChowCentaur = «thunk»; ChowKick = «thunk»; ChowPhaser = «thunk»; «20373 attributes elided» } note: Some attributes were elided. To print more attributes, use `--option max-attrs-print ...`. ``` ### Additional context <details> <summary>Why does this functionality require setting handlers (#9922)?</summary> It may not seem obvious, but this functionality requires setting handlers (#9922). This is because [C++ initializes `static` values in an arbitrary and undefined order](https://en.cppreference.com/w/cpp/language/siof) before executing the `main` function. In `print-options.hh`, we have some code like this: ```c++ static PrintOptions errorPrintOptions = PrintOptions { .maxAttrs = 10, // ... }; ``` If we naïvely added a setting like this: ```cpp Setting<size_t> maxAttrsPrint{this, 10, "max-attrs-print", "The maximum number of attributes to print before eliding."}; ``` And updated the `errorPrintOptions` definition to match: ```c++ static PrintOptions errorPrintOptions = PrintOptions { .maxAttrs = evalSettings.maxAttrsPrint, // ... }; ``` Then the compiler may very well decide to initialize `errorPrintOptions` before `evalSettings`, leading to _zero_ attributes being printed in error messages. (Worse, the compiler could initialize the two `static`s in different threads, leading to a non-deterministic number of attributes being printed.) With a setting handler, we could ask the `max-attrs-print` setting to simply update `errorPrintOptions.maxAttrs` when it's set. The alternative, which I think is probably a good idea regardless of whether settings handlers are added, is to make setting values (and many other `static`s in the Nix codebase) functions, so they run in a predictable, programmer-controlled order, from within the `main` function. </details> ### Priorities Add :+1: to [issues you find important](https://github.com/NixOS/nix/issues?q=is%3Aissue+is%3Aopen+sort%3Areactions-%2B1-desc).

Add `repl-overlays` setting by 9999years · Pull Request #10203 · NixOS/nix · GitHub

@roberth: It would be unfortunate if everyone who wants the Nixpkgs flake learns a solution that doesn’t just work out of the box. Unnecessary lore / learning curve.
@tomberek: A gdbrc-like script would be a nice feature
- @roberth: Agree, but not preferably not as the first solution we merge for the packages.${currentSystem}/convenience use case
- @roberth: Serves a different (valid!) use case, which is to support a specific activity with a repl

@roberth: Example of how an in-flake solution may look

# nixpkgs/flake.nix'
{
  outputs = { self ,... }: {
    legacyPackages = ...;

    # (extraReplScope?)
    # This is merged into the repl scope, not the flake
    replScope = info: {
      pkgs = self.legacyPackages.${info.currentSystem};
    };
  }
}

@edolstra: No objections, although @roberth example is cleaner
@ericson2314: Would rather not have this at all, or have @robert’s flake version. It’s too ad-hoc for me as is, but flakes are the “batteries included” bundle of many ad-hoc conveniences today so it makes more sense there.

What is `resolve-system-dependencies` · Issue #9769 · NixOS/nix · GitHub

Assigned to @ericson2314

Make debugger traces less confusing (and other low hanging fruit) · Issue #9729 · NixOS/nix · GitHub

github.com/NixOS/nix

Make debugger traces less confusing (and other low hanging fruit)

opened 07:35PM - 09 Jan 24 UTC

roberth

feature documentation error-messages repl

**Is your feature request related to a problem? Please describe.** The debugg…er has commands for two types of "traces". - `:bt`, which aligns with the call stack - `:st`, which shows the scope I wouldn't think of the latter as a "trace" but rather just the scope. Nonetheless, `:st <num>` selects from the `:bt` stack. I suppose the reasoning is that it switches the repl _scope_ to a different call stack frame. **Describe the solution you'd like** Add `:scope` (and not `:scope <num>`). It behaves like `:st`. Hide `:st` and make it tell you to use `:scope`. Add `:frame <num>` to select a scope from the backtrace. This matches GDB. (LLDB has `frame select <num>`.) `:bt` is good and aligns with GDB and LLDB. Maybe add a `:scope <num>` command for the purpose of accessing shadowed variables, or something more explicit like `:scope leave <num>`. Clean up and clarify the `:?` help text. Review the output printed by the commands. Some terms don't match well with the user's frame of reference; e.g. "Env level" -> "Scope level", and "static:" seems irrelevant. Drop the printed pointers? Reverse the print order of the call stack (`:bt`), to match the order of `--show-trace`. (ie without changing the frame<->number mapping. 0 is still the latest frame, but will be printed last) **Describe alternatives you've considered** Not so much an alternative, but alternative naming can be considered. Maybe make it more noun-verb like, like the CLI. Feels more LLDB-like, although hopefully more consistent. ``` :bt :frame select 2 :scope :scope select 2 ``` Doesn't feel like a big difference though, and more effort to type that in. **Additional context** **Priorities** Add :+1: to [issues you find important](https://github.com/NixOS/nix/issues?q=is%3Aissue+is%3Aopen+sort%3Areactions-%2B1-desc).

Experimentally return `treeHash` on git and github fetchers by Ericson2314 · Pull Request #10344 · NixOS/nix · GitHub

github.com/NixOS/nix

Comment by thufschmitt to Experimentally return `treeHash` on git and github fetchers

NixOS:master ← obsidiansystems:return-treeHash

Triaged during the Nix maintainers team meeting: Not ready yet. <details> …- @ericson2314: explains some of the issues that he and Robert had discussed - @roberth points at unresolved questions about just how much we want to trust fetchers today. Laziness complicates that - @ericson2314: Agreed. E.g. if we are no longer always putting the entire source accessor in the store as one big store object, it is unclear when we would recheck a `narHash` in the lock file. - @roberth: We could use an in-memory overlay store for both .drvs (optimize the I/O latency) _and_ as a store for lazy sources. This would allow the narHash to be computed without I/O and storage in the real store. - @roberth would be nice to just store commit objects so we can faithfully use original hash for commits (root commit object) and submodules (nested commit object) - two options for the extra data - store with dot files in tree (like the case hack) - store in DB (store object, but not file system object) Awkward to pollute tree, but also awkward to store arbitrary amounts of extra data in DB. - @ericson2314: In any event, can side-step this unexplored design space for now by focusing on getting `outputHashMode` argument attributes on all the built-ins that need it (`fetchTree`, `path`, etc.). That is an easier-to-think-about next step, and will buy us some time to mull on these issues.

moved documentation files into documentation directory by p01arst0rm · Pull Request #10284 · NixOS/nix · GitHub

Seems unnecessary and can be solved differently if needed.
Assigned to @ericson2314.