Changes to make Nix output more informative errors

[Mathnerd314]

• Output several more locations during --show-trace
• Option -C / --color for use with less -r
• nix-instantiate respects '-k' by continuing evaluation after errors (only with derivations)

[Mathnerd314]

@edolstra Is there any possibility of this being merged?

[copumpkin]

Don't leave us hanging! You want to make nix output more what? :smile:

[domenkozar]

@edolstra didn't read the code itself, but this is worth persuing

[copumpkin]

:+1:

[domenkozar]

Another error that needs reasoning explanation:

error: access to path ‘/home/domen/nixpkgs/nixos/release-combined.nix’ is forbidden in restricted mode

It should at least say that it will evaluate nix only in search path and in Nix store.

[domenkozar]

This would be so nice to have. @Mathnerd314 could you resolve merge errors and I'll test?

[Mathnerd314]

I might update this eventually, but not anytime soon. There are some systematic design flaws which I want to resolve first.

[alesya-h]

Any update on this?

[Mathnerd314]

@alesguzik I've stopped most Nix development until June 24, 2016; you're welcome to do something with it before then though.

[domenkozar]

@Mathnerd314 would love to see this going, best to open one PR per fix. That way there is a higher chance they will get merged.

[copumpkin]

@Mathnerd314 could you elaborate on

There are some systematic design flaws which I want to resolve first.

so someone else can pick this up effectively?

[Mathnerd314]

@copumpkin That's just #296 and related issues; the only issue with this PR I know of is that I'm too lazy to update it to fix the merge conflicts. From Haskell it's clear that lazy evaluation works, and so far there's been nothing urgent enough in Nix development to force me to update. But of course anyone else is welcome to pick up the code and run with it.

[dtzWill]

This would be great! Has anyone looked at tackling the conflicts yet? Hopefully not too bad, but even if they are... breaking this into incremental improvements (as suggested above, and I agree should be done) means progress can be made without fixing all the things.

Is this likely to have costs to eval time/size? I've always assumed --show-trace existed (and was off by default) so that these costs were opt-in for debugging, but never actually checked how/if that's done. Also I have "show-trace = 1" or whatever in my nix.conf haha. Anyway, let's revive this!

[deliciouslytyped]

Is there any life left in this?

[stale[bot]]

I marked this as stale due to inactivity. → More info

[Ericson2314]

I am sad this was neglected. I started looking to see if there was anything to salvage after our recent log overhauling.

[Ericson2314]

https://github.com/NixOS/nix/compare/master...obsidiansystems:noisy-nix this is the first commit of this PR (without the color parts, as we have much more of that now) rebase and building but untested. That's enough for now.

[nixos-discourse]

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

https://discourse.nixos.org/t/2022-12-09-nix-team-meeting-minutes-15/23951/1

[Ericson2314]

@layus Would you be able to help me understand what parts of this haven't been obviated by changes in the last few years --- I ask you in particular because your (merged) worked landing more error context seems related.

[layus]

It is really difficult to say, given the codebase changed quite a lot.

• Output several more locations during --show-trace

This one seems similar to my changes, although more involved as it tracks more locations in the values.

I can have a more in-depth check if you want.

• Option -C / --color for use with less -r nix-instantiate

I do not think we have this.

• respects '-k' by continuing evaluation after errors (only with derivations)

Seems interesting, no idea if we have that.

Le 19 février 2023 19:27:44 GMT+01:00, John Ericson @.***> a écrit :

@layus Would you be able to help me understand what parts of this haven't been obviated by changes in the last few years --- I ask you in particular because your landing more error context seems related.

-- Reply to this email directly or view it on GitHub: https://github.com/NixOS/nix/pull/612#issuecomment-1436059509 You are receiving this because you were mentioned.

Message ID: @.***>

[Ericson2314]

Thanks for taking a look, @layus!

[Mathnerd314]

As far as the improved locations there were various changes:

• Pos had end line/column added. I guess this is a little hard to implement with the PosIdx structure, probably you would need a new Span type. The main issue is you have to change all the usages at once.
• Positions were passed around in a lot more places. I think this is better in the current code, in particular the main change of storing positions in values seems to have made in #4440, at least for lambdas, applications, and attribute sets. I stored the position for all types of values but maybe that adds too much overhead. I never measured it explicitly but I didn't notice a big slowdown when I used the patch.
• I systematically went through every place an error/trace was reported and made sure a position was passed. Probably this can be worked on incrementally / separately.
• I added some more places that reported errors/traces, for example ExprVar::eval, forceValue, forceValueDeep on list positions, getDerivation, and callPrimOp. Again this can probably be its own PR.

[Ericson2314]

Thanks for that summary, @Mathnerd314.

[fricklerhandwerk]

Thanks @Mathnerd314, @layus, and @Ericson2314! We're on a good path away from getting overwhelmed with PRs and hopefully will slowly catch up with those stalled ones.