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

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

Open
vbehar opened this issue Mar 19, 2023 · 18 comments
Labels

Comments

@vbehar
Copy link

vbehar commented Mar 19, 2023

Describe the bug

since https://github.com/mikefarah/yq/releases/tag/v4.32.1 and more specifically #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
Copy link
Author

vbehar commented Mar 19, 2023

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

@vbehar
Copy link
Author

vbehar commented Mar 19, 2023

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 vbehar changed the title unwrapScalar no longer enabled by default since v4.32.1 new behaviour since v4.32.1: input json produces output json by default instead of yaml Mar 19, 2023
@MrMarkW
Copy link

MrMarkW commented Mar 19, 2023

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

@mikefarah
Copy link
Owner

@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.

@ryenus
Copy link
Contributor

ryenus commented Mar 19, 2023

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.

@mikefarah
Copy link
Owner

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

@cmaahs
Copy link

cmaahs commented Mar 20, 2023

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...

@mikefarah
Copy link
Owner

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

@cmaahs
Copy link

cmaahs commented Mar 20, 2023

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

@cmaahs
Copy link

cmaahs commented Mar 20, 2023

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

@UnleashSpirit
Copy link

UnleashSpirit commented Mar 21, 2023

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

@tobiasehlert
Copy link

tobiasehlert commented Mar 22, 2023

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

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

@mikefarah
Copy link
Owner

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

@hubchub
Copy link

hubchub commented Apr 3, 2023

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 )

@mikefarah
Copy link
Owner

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 :(

@ryenus
Copy link
Contributor

ryenus commented Apr 18, 2023

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

@vcschapp
Copy link

vcschapp commented Jun 1, 2023

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!!

@StanleyP
Copy link

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

10 participants