google / go-jsonnet

Apache License 2.0
1.63k stars 234 forks source link

A commandline option for output format #195

Open sbarzowski opened 6 years ago

sbarzowski commented 6 years ago

We could provide a convenience option --wrap and it could be used as jsonnet --wrap std.manifestYAML file.jsonnet and it would pass whatever file.jsonnet evaluates to into std.manifestYAML (and assume raw string output afterwards, I guess).

That would allow output in any format, including custom ones.

This way, for example manifestINI, which we already have, could be used in the same way and even if someone decides they want to generate, say, nginx config, with their custom function, they could do it just as easily.

Is there a better name for this option than --wrap?

sparkprime commented 6 years ago

I used to call it custom manifesters. The idea was that the built-in JSON manifester would no-longer be used, and all Jsonnet code would emit a string that is printed verbatim. To make that practical, every execution would be implicitly wrapped by a manifester, by default std.manifestJson but you can specify arbitrary code on the commandline. The -S mode just means use the identity function.

The only problem is that error stacktraces are bad when using this feature because any errors form user code get wrapped in a load of traces (especially for recursive formats like JSON) and the actual context also gets lost. Actually it looks like this context is lost in go-jsonnet too:

dcunnin@dcunnin7:~$ jsonnet -e 'local thing = {x: error "override x"}; {y: thing {x: 1}, z: thing }' -t 100
RUNTIME ERROR: override x
        <cmdline>:1:19-37       object <thing>
        <cmdline>:1:61-66       object <anonymous>
        During manifestation
dcunnin@dcunnin7:~$ jsonnet -e 'std.manifestJson(local thing = {x: error "override x"}; {y: thing {x: 1}, z: thing })' -t 100
RUNTIME ERROR: override x
        <cmdline>:1:36-54       object <v>
        std.jsonnet:826:50-54   thunk <v>
        std.jsonnet:801:16      thunk <a>
        std.jsonnet:801:16-25   function <anonymous>
        std.jsonnet:801:16-25   function <aux>
        std.jsonnet:826:46-85   thunk <array_element>
        std.jsonnet:219:21-27   thunk <a>
        std.jsonnet:219:21-35   function <anonymous>
        std.jsonnet:219:21-35   function <aux>
        std.jsonnet:224:17-57   function <aux>
        std.jsonnet:230:13-34   function <anonymous>
        std.jsonnet:829:17-36   function <aux>
        std.jsonnet:826:46-85   thunk <array_element>
        std.jsonnet:219:21-27   thunk <a>
        std.jsonnet:219:21-35   function <anonymous>
        std.jsonnet:219:21-35   function <aux>
        std.jsonnet:226:17-63   function <aux>
        std.jsonnet:230:13-34   function <anonymous>
        std.jsonnet:829:17-36   function <aux>
        std.jsonnet:830:9-27    function <anonymous>
        std.jsonnet:797:27-60   function <anonymous>
        <cmdline>:1:1-86
dcunnin@dcunnin7:~$ go-jsonnet -e 'local thing = {x: error "override x"}; {y: thing {x: 1}, z: thing }' -t 100
RUNTIME ERROR: override x
        <cmdline>:1:19-37       object <thing>
        During manifestation

dcunnin@dcunnin7:~$ go-jsonnet -e 'std.manifestJson(local thing = {x: error "override x"}; {y: thing {x: 1}, z: thing })' -t 100
RUNTIME ERROR: override x
        <cmdline>:1:36-54       object <thing>
        <std>:826:50-54 thunk from <thunk from <thunk from <thunk from <thunk <lines> from <function <aux>>>>>>
        <std>:801:16-17 function <aux>
        <std>:1036:29-30        thunk from <thunk <ta> from <function <anonymous>>>
        <builtin>       builtin function <type>
        <std>:1038:33-35        thunk from <function <anonymous>>
        <builtin>       builtin function <primitiveEquals>
        <std>:1038:12-40        function <anonymous>

        <std>:826:46-85 thunk from <thunk from <thunk from <thunk <lines> from <function <aux>>>>>
        <std>:(825:44)-(826:85) thunk from <thunk from <thunk from <thunk <lines> from <function <aux>>>>>
        <builtin>       builtin function <join>
        <std>:826:46-85 thunk from <thunk from <thunk from <thunk <lines> from <function <aux>>>>>
        <std>:(825:44)-(826:85) thunk from <thunk from <thunk from <thunk <lines> from <function <aux>>>>>
        <builtin>       builtin function <join>
        <std>:830:9-27  function <anonymous>
        <std>:797:27-60 function <anonymous>
        <cmdline>:1:1-86        $
        During evaluation

A solution to make std.manifestJson work is to allow catching errors to inject the extra context (i.e. the path through the object) into the message / trace somehow. E.g. the error could be:

RUNTIME ERROR: While manifesting $.z, got "override x"
sbarzowski commented 6 years ago

Showing the path like $.z would be great UX, independently from what we will do with stack traces.

sbarzowski commented 6 years ago

Not sure why the outer object doesn't show up in go-jsonnet stack trace, maybe I'll investigate later. But that's a separate issue.

sbarzowski commented 6 years ago

I had a solution for custom manifester stack traces, that doesn't require special error handling inside them. Special error handling there would be a pain even when we have try/catch stuff.

The solution is to deeply evaluate it, and maybe strip hidden fields, before passing to manifester. This way all the special error handling can happen in one place - the recursive evaluator. And it can be done now, without any new language features.

sparkprime commented 6 years ago

So some sort of builtin that turns a Jsonnet value into a JSON value (as the semantics defines manifestation) but also caches each error that occurs in the process with stacktrace beginning at the point in the json tree the expression was found?

sbarzowski commented 6 years ago

Something along these lines. Not sure what you mean by caching an error. But yeah, it pretty much boils down to manifesting it to JSON first to handle all the errors and passing safe data to the actual manifester. It could be implemented as interpreter.manifestJSON, but with the result represented as jsonnet values (instead of string/slice/map).

sparkprime commented 6 years ago

I guess you don't need to cache the errors because you can abort at the first one with the path and stacktrace at that point.

dancompton commented 5 years ago

Why have CLI parameters at all? Shouldn't manifesters be implemented in jsonnet or via a std.native function as they are now? Why can't the sugared jsonnet be the intermediate format for the manifesters?

function manifest(rawSnippet, manifestFunc=interpreter.manifestJsonnetString)

Is that a possibility or has the jsonnet been desurgared at this point? If so, it would be easy enough to implement a manifestFunc that replaces jsonnet fmt.

sbarzowski commented 5 years ago

Right now the Jsonnet command and library have explicit support for YAML output. The point here is to generalize this, so that other formats could be used.

Right now it's possible to write a custom manifester for any format and just wrap your program in it. Even right now it's as easy as: jsonnet -S -e 'std.manifestINI(import "whatever_file_I_want.jsonnet")'. Of course you could use any other function instead of std.manifestINI.

The core functionality is there. In this issue we're really discussing what would be the most elegant/consistent/convenient user interface. We need to think about various use cases. For example I can imagine a reasonable setup in which only only "real" content is defined in Jsonnet source file and the format determined by the target file extension. A --wrap option would be a bit more convenient in such case.

There was another usability point about stack traces raised by @sparkprime, and the conclusion is basically that such --wrap option could fully evaluate the original file first, so that in case of errors, they won't be obscured by the manifester.

@dan-compton I don't understand what you mean by "Why can't the sugared jsonnet be the intermediate format for the manifesters?" Desugaring is only an implementation detail, an AST transformation we do to make evaluation easy.

sbarzowski commented 5 years ago

(This is sort of off-topic for this particular issue)

@dan-compton:

It's fundamentally impossible to ordinarily treat a part of currently running program as data. Jsonnet has referential transparency, which basically means that you can substitute all 4 in your program with 2+2 and the output must not change. In general you can replace any expression with its result or vice versa and the result must not change (the performance may change, but not the final output). This is a really good property to have - for example refactoring is way easier.

However we could in principle have something like parseJsonnet function which takes a string (possible imported with importstr) and returns a representation of Jsonnet AST, which could then be transformed and manifested. I'm still thinking about how such API could look like.

dancompton commented 5 years ago

@sbarzowski I do not see how this is the case if a metacircular interpreter is implemented. For example, lisp provides the backtick which prevents evaluation of a snippet of code. http://cl-cookbook.sourceforge.net/macros.html#LtohTOCentry-2

I now see your response on the other thread

sbarzowski commented 5 years ago

@dan-compton

By "impossible to ordinarily treat a part of currently running program as data" I meant impossible without special quoting etc (and while preserving other assumptions). Once you introduce special quoting the problem goes away. It really is the same as having a special kind of literal for Jsonnet AST. It's also pretty much the same as keeping it as string and using parseJsonnet. It satisfies the requirement that "expression and a representation of an expression are different things" which I mentioned in the other issue.

dancompton commented 5 years ago

@sbarzowski

It's also pretty much the same as keeping it as string and using parseJsonnet.

That's precisely how my simple POC was implemented. It was based on fodder collected from c-style comments, but rather than build a FodderElement, it built a string and emitted a "StringBlock" which is held as verbatim.

Witness the power of this behavior vs doing the same via the go AST:

local toRefactor = /~ {string: "Old"} ~/;  or local toRefactor = importstr 'input.jsonnet';
local codeVal = std.parseJsonnet(toRefactor);
local refactoredCodeVal = 
    if codeVal.string == "Old" then 
             codeval {string: "New"}
    else 
          codeVal;
{
    'input.jsonnet':  refactoredCodeVal,
}
sbarzowski commented 4 years ago

I think now that both the deeply evaluating variant and simple wrapping would be useful. The former for the custom manifester case and the latter for retrieving part of the output (e.g. extracting a single field, so that it can be used in a shell script easily).