Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

libexpr: add position to builtins.trace #3170

Closed
wants to merge 1 commit into from

Conversation

zimbatm
Copy link
Member

@zimbatm zimbatm commented Oct 28, 2019

Let the user know where the trace was produced by adding the filename
and lineno from where that function was called.

/cc happy birthday @domenkozar :p

Let the user know where the trace was produced by adding the filename
and lineno from where that function was called.
@edolstra
Copy link
Member

Don't think this is a good idea because it changes the meaning of the trace message, possibly in a confusing way. E.g.

trace: warning: types.string is deprecated because it quietly concatenates strings

becomes

trace: warning: types.string is deprecated because it quietly concatenates strings at /some/file/name.nix:1234

but that's not where types.string is called. Or what about

trace: warning: `stdenv.isArm` is deprecated after 18.03. Please use `stdenv.isAarch32` instead at /some/file/name.nix:1234

which tells you to use something in the wrong file.

@domenkozar
Copy link
Member

What about trace:/some/file/name.nix:1234: warning: types.string is deprecated because it quietly concatenates strings

Ideally, there would be an optional way to print the traceback for traces, but at least this makes tracing more useful.

@zimbatm
Copy link
Member Author

zimbatm commented Oct 29, 2019

Fundamentally the issue with builtins.trace not reporting the location is a valid usability issue. Currently the way to find the call location can be done but is cumbersome as it involves using grep on the nix code.

There are two issues with the current implementation:

  • It changes the output. Some tools (like lorri) have started depending on the output.
  • The reported position is not always useful as it reports calls to builtins.trace, which might be wrapped in a function, like in lib.traceVal

What if there was a new trace builtin to which the caller position could be passed?

@domenkozar
Copy link
Member

@edolstra what if --show-trace would show full stack of trace?

We are importing like 10 different github repositories with Nix, and going around each and grep for usage is quite cumbersome.

@zimbatm
Copy link
Member Author

zimbatm commented Nov 25, 2019

I don't know if nix can produce a readable stacktrace in the imperative sense. When the thunk gets evaluated it already lost most of its context.

@edolstra
Copy link
Member

@domenkozar What do you mean with "full stack of trace"?

@domenkozar
Copy link
Member

Seems like I'm asking for #1610

@zimbatm
Copy link
Member Author

zimbatm commented Nov 16, 2020

Closing since a lot of improvements have been added to Nix's error reporting since then.

@zimbatm zimbatm closed this Nov 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants