Closed nirosys closed 1 year ago
The appveyor build is failing with:
[00:02:08] [ RUN ] IonTextDownconvert.IntsFloatsAndDecimals
[00:02:08] C:\projects\ion-c-3akm7\test\test_ion_text.cpp(1408): error: Expected: IERR_OK
[00:02:08] Which is: 0
[00:02:08] To be equal to: convert_to_json("1.0e0", json_text, sizeof(json_text))
[00:02:08] Which is: 7
[00:02:08] [ FAILED ] IonTextDownconvert.IntsFloatsAndDecimals (2 ms)
The error returned (value: 7) is IERR_UNRECOGNIZED_FLOAT
.
MacOS build with gcc-11 failed due to an assertion in the linker.. which is.. neat.
[ 62%] Linking CXX shared library libion_events.dylib
0 0x106059ffa __assert_rtn + 139
1 0x105e8d28d mach_o::relocatable::Parser<x86_64>::parse(mach_o::relocatable::ParserOptions const&) + 4989
2 0x105e7df8f mach_o::relocatable::Parser<x86_64>::parse(unsigned char const*, unsigned long long, char const*, long, ld::File::Ordinal, mach_o::relocatable::ParserOptions const&) + 207
3 0x105ef49d4 ld::tool::InputFiles::makeFile(Options::FileInfo const&, bool) + 2036
4 0x105ef7fa0 ___ZN2ld4tool10InputFilesC2ER7Options_block_invoke + 48
5 0x7ff81a1e234a _dispatch_client_callout2 + 8
6 0x7ff81a1f4c45 _dispatch_apply_invoke_and_wait + 213
7 0x7ff81a1f4161 _dispatch_apply_with_attr_f + 1178
8 0x7ff81a1f4327 dispatch_apply + 45
9 0x105ef7e2d ld::tool::InputFiles::InputFiles(Options&) + 669
10 0x105e68d48 main + 840
A linker snapshot was created at:
/tmp/libion_events.dylib-2022-12-01-112417.ld-snapshot
ld: Assertion failed: (_file->_atomsArrayCount == computedAtomCount && "more atoms allocated than expected"), function parse, file macho_relocatable_file.cpp, line 2061.
collect2: error: ld returned 1 exit status
MacOS issue was due to a bug in Xcode 14.0.1. I'm running 14.1 and I was unable to reproduce the issue, so I've added a build step for macos builds in the GH Actions workflow to set the default xcode to 14.1 (it is installed in the image, just not used by default). That seems to have fixed the MacOS issue.. now on to the windows issue..
Reproduced the issue in the windows build, tracked it down to a misplaced #endif
. Everything should be green now.
Thank You @tgregg!
I'm going to merge this tonight and follow up with a PR for the changes mentioned above if no one has any arguments against that.
Issue #, if available: n/a
Description of changes: This PR adds functionality to the ion text writer to allow for a basic lossy conversion to JSON encoding. The down-conversion is done in the same manner described in the cookbook, with a couple minor differences that will be corrected soon.
The most noticeable difference is the ranges in which unicode is escaped. Currently, any non-ascii characters are emitted as unicode escape sequences. I'll follow up with a fix for that soon. I have identified the issue, and just need to implement the fix and test it.
An existing issue that needs some consensus is how multiple top-level values are handled. With this commit multiple top-level items are serialized with no delimeter. This is supported by some JSON parsers, such the one used by the tool jq, but may be an issue with others. A simple hacky fix can be added to place all top-level values into a list, but I didn't feel comfortable with the implementation unless it is truly needed. Another option might be to use ~JSON Lines~ (website seems to be down).
The changes add a new option to
ION_WRITER_OPTIONS
calledjson_downconvert
which, when true will format any output written to the writer using the JSON friendly formatting described in the cookbook link (minus the above notes).An example of converting ion data to json, can be found in
test/test_ion_text.cpp
in theconvert_to_json
function.This PR also extends the CLI to allow the output formats
json
andjson-pretty
, which perform the down conversion with pretty printing off, and on, respectively. Examples of usingion process
to perform downconversion has also been added to the help.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.