yq icon indicating copy to clipboard operation
yq copied to clipboard

new behaviour since v4.32.1: input json produces output json by default instead of yaml

Open vbehar opened this issue 1 year ago • 18 comments

Describe the bug

since https://github.com/mikefarah/yq/releases/tag/v4.32.1 and more specifically https://github.com/mikefarah/yq/pull/1582 a JSON input file is now auto-detected as JSON - instead of the previous YAML default format.

which implies that the output is automatically set to JSON format - instead of being YAML previously (= in the previous versions)

the impact is that for a single output value, the result is now wrapped in quotes.

Version of yq: 4.32.1 Operating system: mac and linux Installed via: binary release and homebrew

Input JSON data.json:

{"version": "1.2.3"}

Command The command you ran:

yq '.version' data.json

Actual behavior

"1.2.3"

Expected behavior

1.2.3

(which was the actual behaviour in v4.31.2 and older versions)

Additional context

  • forcing a YAML output "fixes" the issue: yq -oy '.version' test.json

vbehar avatar Mar 19 '23 13:03 vbehar

I'm wondering if it's related to https://github.com/mikefarah/yq/pull/1582 because forcing a YAML input format with yq -p yaml '.version' test.json produces the result without quotes as expected

vbehar avatar Mar 19 '23 13:03 vbehar

testing in verbose mode, I understood the issue: the file is auto-detected as JSON, which somehow also set the output format to JSON - instead of the expected YAML. which explains the quotes. so not related to unwrapScalar, but rather to a wrong auto-detected output format. unless it is now expected that a JSON input produces JSON output...

$ yq -v '.version' test.json
14:09:21 processArgs [DEBU] processed args: [.version test.json]
14:09:21 maybeFile [DEBU] checking '.version' is a file
14:09:21 maybeFile [DEBU] error: stat .version: no such file or directory
14:09:21 maybeFile [DEBU] result: false
14:09:21 processArgs [DEBU] assuming expression is '.version'
14:09:21 FormatFromFilename [DEBU] checking file extension 'test.json' for auto format detection
14:09:21 FormatFromFilename [DEBU] detected format 'json'
14:09:21 FormatFromFilename [DEBU] checking file extension 'test.json' for auto format detection
14:09:21 FormatFromFilename [DEBU] detected format 'json'
14:09:21 initCommand [DEBU] Using outputformat json
14:09:21 ParseExpression [DEBU] Parsing expression: [.version]
14:09:21 func1 [DEBU] PathToken version
14:09:21 handleToken [DEBU] processing version (55)
14:09:21 handleToken [DEBU]   adding token to the fixed list
14:09:21 ConvertToPostfix [DEBU] postfix processing currentToken version (55)
14:09:21 ConvertToPostfix [DEBU] put version (55) onto the opstack
14:09:21 ConvertToPostfix [DEBU] postfix processing currentToken )
14:09:21 popOpToResult [DEBU] popped version (55) from opstack to results
14:09:21 ConvertToPostfix [DEBU] opstackLen: 0
14:09:21 ConvertToPostfix [DEBU] PostFix Result:
14:09:21 ConvertToPostfix [DEBU] > version
14:09:21 createExpressionTree [DEBU] pathTree version
14:09:21 Decode [DEBU] going to decode
14:09:21 GetMatchingNodes [DEBU] Processing Op: version
14:09:21 GetMatchingNodes [DEBU] D0, P[], (doc)::version: 1.2.3
14:09:21 GetMatchingNodes [DEBU] >>
14:09:21 traversePathOperator [DEBU] -- traversePathOperator
14:09:21 traverse [DEBU] Traversing D0, P[], (doc)::version: 1.2.3
14:09:21 traverse [DEBU] digging into doc node
14:09:21 traverse [DEBU] Traversing D0, P[], (!!map)::version: 1.2.3
14:09:21 traverse [DEBU] its a map with 1 entries
14:09:21 doTraverseMap [DEBU] checking version (!!str)
14:09:21 matchKey [DEBU] pattern: version
14:09:21 doTraverseMap [DEBU] MATCHED
14:09:21 doTraverseMap [DEBU] including value
14:09:21 PrintResults [DEBU] PrintResults for 1 matches
14:09:21 GetMatchingNodes [DEBU] Processing Op: EXPLODE
14:09:21 GetMatchingNodes [DEBU] D0, P[version], (!!str)::1.2.3
14:09:21 GetMatchingNodes [DEBU] >>
14:09:21 explodeOperator [DEBU] -- ExplodeOperation
14:09:21 GetMatchingNodes [DEBU] getMatchingNodes - nothing to do
14:09:21 PrintResults [DEBU] -- print sep logic: p.firstTimePrinting: false, previousDocIndex: 0, mappedDoc.Document: 0
14:09:21 PrintResults [DEBU] D0, P[version], (!!str)::1.2.3
"1.2.3"
14:09:21 PrintResults [DEBU] done printing results
14:09:21 Decode [DEBU] going to decode

vbehar avatar Mar 19 '23 13:03 vbehar

I have an additional issue with it not handling the file's extension.

Previous this worked and outputted the content in yaml: yq -v -P '.outputs ' .terraform.tfstate

Now I get this error: Error: unknown format 'tfstate' please use [yaml|json|props|csv|tsv|xml]

13:26:22 FormatFromFilename [DEBU] checking file extension '.terraform.tfstate' for auto format detection
13:26:22 FormatFromFilename [DEBU] detected format 'tfstate'
13:26:22 FormatFromFilename [DEBU] checking file extension '.terraform.tfstate' for auto format detection
13:26:22 FormatFromFilename [DEBU] detected format 'tfstate'

I renamed my input file to .terraform.json

Executed yq -P '.outputs ' .terraform.json, but now the content is JSON.

The work around was to force the output format: yq -o yaml -P '.outputs' .terraform.json or yq -o yaml '.outputs' .terraform.json

MrMarkW avatar Mar 19 '23 18:03 MrMarkW

@vbehar yeah the intention is if you give it a JSON file it produces JSON output (and same for other formats) unless explicitly specified. I didn't think it would break existing behaviours, mainly because I had XML/CSV/etc in my head, and for those you previously had to specify the input format so this wouldn't affect them 🤔

Still I think it makes sense JSON => JSON by default.

@MrMarkW I think that's a bug in the new behaviour, and should be a separate issue so I can resolve it separately to this - if it doesn't recognise the extension I should just make it default to yaml. I'll fix that ASAP.

mikefarah avatar Mar 19 '23 22:03 mikefarah

Though JSON => JSON makes lots of sense, personally I like it as well, on the other hand it's also a breaking change since yq had been always using yaml as the default output format. And that's something the users have get used to, so Hyrum's Law applies here.

@mikefarah maybe the default output format change should be enabled only in next major version like yq v5? Just my 2 cents.

ryenus avatar Mar 19 '23 23:03 ryenus

Yeah it's a fair point - happy to reverse it if more people think it should....

mikefarah avatar Mar 19 '23 23:03 mikefarah

As a YAML tool, I feed it filenames of all sort, that are YAML. It feels awkward to have to specify -p yaml and -o y because my filename has a domain name as the suffix of the file. In this case ending with .rocks, so yq is trying to determine the type based on .rocks .

% echo ${KUBECONFIG} /Users/christopher.maahs/.kube/test.maahsome.rocks

% yq e '.current-context' "${KUBECONFIG}" Error: unknown format 'rocks' please use [yaml|json|props|csv|tsv|xml]

I can certainly still cat the file and pipe it in, and all YAML is assumed...

cat ${KUBECONFIG} | yq e '.current-context' test.maahsome.rocks

On the plus side, you have clearly made a fantastic tool that we use every day! And thanks for that!

In the case of an extension that is not on the list above, just default to YAML and process away, it is then the user responsibility to feed it YAML...

cmaahs avatar Mar 20 '23 00:03 cmaahs

I've just made an update to default to yaml for unknown file extensions (v4.32.2) sorry about that! @cmaahs

mikefarah avatar Mar 20 '23 00:03 mikefarah

I just read #1582, and it looks like that was the intent :) These things happen...

cmaahs avatar Mar 20 '23 00:03 cmaahs

I did get a chuckle out of and failure in ... 4.32.1, like a grand countdown... That version number aligned nicely...

cmaahs avatar Mar 20 '23 12:03 cmaahs

Hi Got same issue and spending lot of time to identify why I have a "file nout found" while I can ls -l this same file :D I was because of those quotes.

I agreed both that parsing json should logically output json but also that yq is a yaml tool so output yaml by default (w'ont help you to choose ^^) But as its "breaks" previous code it may be considered as breaking change if you came back to parsing json -> output json

UnleashSpirit avatar Mar 21 '23 15:03 UnleashSpirit

I do also experience the same issue with output being json formatted since v4.32.1.

explicitly adding -o=yaml fixed it for me :)

tobiasehlert avatar Mar 22 '23 12:03 tobiasehlert

I've updated 4.33.1 to log a warning for this issue:

JSON file output is now JSON by default (instead of yaml). Use '-oy' or '--output-format=yaml' for yaml output

mikefarah avatar Mar 26 '23 00:03 mikefarah

this doesn't really seem like a sufficient fix.

4.32.1 -> 4.33.1 is just a minor release and should remain backwards compatible but it has breaking changes

screenshot-2023-04-03_09-57-49

we use YQ across a lot of projects within github actions for version control and every one of them broke underneath us ( yes we should be using a fixed version and it is our fault too )

hubchub avatar Apr 03 '23 14:04 hubchub

Yeah I'm really sorry about that - at the time I released it I didn't realise it was a breaking change :/ - and wouldn't have released it if I did.

I'm reluctant to go back now - as it's been out in the wild for a couple of weeks and that could cause the same compatibility issues you are mentioning, but in reverse :S

I think going forward, it makes sense to product JSON output when it gets JSON input - but I am sorry about unintentionally causing you to do a pile of work from the breaking change :(

mikefarah avatar Apr 05 '23 04:04 mikefarah

Bitten by this last week in my own release script, just found out 😂

ryenus avatar Apr 18 '23 03:04 ryenus

Can I mention another subtle reason why this new behaviour isn't ideal?

I have a script that runs in a GitHub action to test about of inputs and outputs, some JSON, some YAML. The new behaviour didn't break the script (because it uses -r for raw output, it seems there was no difference before and after). However, when I run locally with my Ubuntu distro's yq version 3.1.0 I get clean output, but when it runs in GH the output is polluted with a tonne of warnings like this:

00:37:53 initCommand [WARN] JSON file output is now JSON by default (instead of yaml). Use '-oy' or '--output-format=yaml' for yaml output

Then when I try to suppress the warnings in the script by putting -oy because why not, it fails locally because version 3.1.0 doesn't recognize those arguments.

So there doesn't seem to be a way to have it work with clean output across both versions.

Thanks for your consideration!!

vcschapp avatar Jun 01 '23 14:06 vcschapp

This was really unfortunate change of behaviour, broke a lot of scripting in unpredictable way for my project. I suggest to avoid any automagic (detecting output format depending on input format), it is much better to have stable and predictable behavior. Since yq is de facto "jq for yaml" I would really prefer to have YAML output by default.

StanleyP avatar Oct 24 '23 11:10 StanleyP