feat: allow writing to output file

[christf]

This allows to write the output to a file and closes #2418 . When calling jq inside a docker context this will be helpful as it means jq will not have to be called from within a shell with output redirection.

Caveats do apply:

• I have only started looking at the jq codebase today, so the code may be doing something insane

[christf]

Test on linux with utf-8:

✘-INT ~/Programmierung/jq [ofile|✚ 1…1]
19:42 $ echo '{ "foo": "💩" } '
{ "foo": "💩" }
✔ ~/Programmierung/jq [ofile|✚ 1…1]
19:42 $ echo '{ "foo": "💩" } ' | ./jq .
{
"foo": "💩"
}
✔ ~/Programmierung/jq [ofile|✚ 1…1]
19:42 $ echo '{ "foo": "💩" } ' | ./jq . -o foo
✔ ~/Programmierung/jq [ofile|✚ 1…1]
19:43 $ cat foo
{
"foo": "💩"
}
✔ ~/Programmierung/jq [ofile|✚ 1…1]
19:43 $

[wader]

There has been talks long time ago about adding adding -i/--inplace and jaq already supports it. Could that be an option? but one thing about how jq handles multiple inputs files, as they were concatenated, probably have be resolved in some way in that case.

$ echo -n 12 > a.json
$ echo -n 34 > b.json

$ jq . a.json b.json
1234

$ jaq . a.json b.json
12
34
$ jaq -i '.+1' a.json b.json
$ jaq . a.json b.json
13
35

[christf]

There has been talks long time ago about adding adding -i/--inplace and jaq already supports it. Could that be an option? but one thing about how jq handles multiple inputs files, as they were concatenated, probably have be resolved in some way in that case.

$ echo -n 12 > a.json
$ echo -n 34 > b.json

$ jq . a.json b.json
1234

$ jaq . a.json b.json
12
34
$ jaq -i '.+1' a.json b.json
$ jaq . a.json b.json
13
35

that is an interesting thought. In the use case I am hoping to solve, the output file needs to be in a certain place that is designed to hold only results. This is, what the tekton task specification might look like

- image: ghcr.io/jqlang/jq:1.9.0
  name: digest-to-results
  command:
    - jq -j '.[0].value' -o /tekton/results/IMAGE-DIGEST image-digested

[christf]

How can this best be progressed?

[wader]

Hey sorry for the delay. I think the implementation look resonable. I'm mostly concern and reluctant about adding more arguments and features and in this case maybe that we might want inplace edit in the future, bloat to have both?

It would also be nice to get some input from some other maintainers, ping @itchyny @emanuele6 @pkoppstein

[christf]

bloat to have both?

You, of course can make that decision. For the case I outlined above, in-place edit would not solve the problem as tekton separates inputs and outputs, with inputs being accessible read-only.

[thaliaarchi]

I don’t agree that this is bloat, even if --inplace is also implemented. --inplace implies that the entire input be read before any output be written, which is made more difficult by streaming inputs and outputs, and would require a buffer. Although these flags both concern output files, I think they’re rather unrelated. There’s also several flags concerning input files.

[wader]

To clarify i menat to have both might be bloaty both in terms of implementation and adding more cli arguments, but don't think i have that strong opinion about more than concern about making the code a lot less maintainable. Maybe a good pitch to do any of this could be a nice refactor? not that i know that much the input/output code

[christf]

To clarify i menat to have both might be bloaty both in terms of implementation and adding more cli arguments, but don't think i have that strong opinion about more than concern about making the code a lot less maintainable. Maybe a good pitch to do any of this could be a nice refactor? not that i know that much the input/output code

Where do you see the refactoring potential in the area of the output code?
What should be the goal of the refactoring?
Why is a changeset of <50LOC a trigger for a suggestion of refactoring of two different areas of code when one of these is not even touched by the changeset?

[thaliaarchi]

@wader I think calling it bloat is definitely reaching too far. It seems useful to me and the code only needs minor fixups.

@christf I'm reviewing it now. I think jq avoids global state so multiple VMs can be instantiated, so the global FILE *ofile would need to be made local. The biggest change would be replacing removing jv_dump and replacing all uses with jv_dumpf. I'm preparing a patch you can apply on your PR with this and some nits.

[thaliaarchi]

@christf I ended up making much more significant changes than I expected, so I figured a new PR would be warranted. I have kept your commit, as it is useful as a base, but I squashed a couple tiny changes into it (e.g., consistent ordering and formatting) to reduce the diff. See #3410.

[thaliaarchi]

To clarify i menat to have both might be bloaty both in terms of implementation and adding more cli arguments, but don't think i have that strong opinion about more than concern about making the code a lot less maintainable. [...] not that i know that much the input/output code

I'm probably the last person to have touched the argument parsing. I don't think this adds an abnormal amount of code for a new flag.

As for in-place editing, I think the work here actually assists in that effort and it probably wouldn't take much effort on top of what's here. Before processing any of the input, it would read the first input file to the end and store it in a buffer. then seek back to the start, change it to write-only, set it to be the ofile, then proceed as normal. (Or alternatively, use a temp file for the output, until finished, then overwrite.) That would reuse everything from --output-file.

Also, in my version of --output-file, I clearly delineate between what things write to the primary output, so the JSON output stream would go to --output-file/--inplace and a disassembly or debug trace would still go to stdout. You'd want that for --inplace too.

[christf]

Thank you very much for picking up the pieces. Your work goes much beyond what I initially raised as a PR. Also it leaves the codebase in better shape by preparing for a feature. That bit about the debug trace is an aspect that wasn't even discussed in here. Thank you!

[wader]

I'm probably the last person to have touched the argument parsing. I don't think this adds an abnormal amount of code for a new flag.

Sorry i didn't mean to come of as harsh. Maybe just my bad memories of trying to add/fix some IO-related things like #1225 that ended up quite complex for how to implement and how it would work in streaming mode, with -e etc and combinations of it.

But great work and it would be good to get some more maintainers opinon on this before spending much time on it.