search

maralorn / nix-output-monitor

Pipe your nix-build output through the nix-output-monitor a.k.a nom to get additional information while building.
GNU Affero General Public License v3.0
835 stars 24 forks source link

display trace/warning messages #132

Closed daniel-sampliner closed 5 months ago

daniel-sampliner commented 5 months ago

Fixes issue #128.

daniel-sampliner commented 5 months ago

I'm a definitely a complete novice to Haskell, so I won't deny this PR carries the stench of a copy-and-paste hack job. No offense taken if this is rejected in favor of a cleaner solution.

maralorn commented 5 months ago

Don’t worry. Thank you for looking into this. And I am glad that you learned a bit of Haskell along the way. :D

I will look into it as soon as I got the time.

maralorn commented 5 months ago

The Haskell code looks quite fine to me. Only question is what is operationally the "correct" solution. Does it make sense to special case messages prepended with "trace"? Are there other similar messages which we should also display?

daniel-sampliner commented 5 months ago

Does it make sense to special case messages prepended with "trace"? Are there other similar messages which we should also display?

Yeah I was concerned about the same. I was hoping that these kind of messages would have a unique level in the json logs (or any other concrete signature), but unfortunately that doesn't appear to be the case.

That being said, is it possible to print messages to the console outside of builtins.trace (or throwing an error, but I think we can safely say errors are already covered by nom)? There isn't anything like printf, so I imagine any kind of abstractions in nixos modules and the various lib.traceX are all eventually going to call builtins.trace?

maralorn commented 5 months ago

Thank you. This is an improvement as is.

viperML commented 5 months ago

Could we get a new release tag that includes this PR? Or maybe something else blocking on master?

maralorn commented 5 months ago

There is a test failing on the main branch and I haven’t had the patience to look into it. Also I want to fine tune and improve the trace display a bit before I release it. So this might take a bit, sorry.

viperML commented 5 months ago

No problem, just wanted to know about that 🙂

maralorn commented 1 month ago

Sorry it took so long but this is included in 2.1.3