rasterlib: Rast_print_json_colors() added (taken from r.colors.out)

[NishantBansal2003]

Added Rast_print_json_colors()(Renamed from print_json_colors) to the raster library for use in both r.colors.out and v.colors.out to avoid duplication, as discussed in #4660 (comment).

[NishantBansal2003]

Not sure why the build failed here: https://github.com/OSGeo/grass/actions/runs/11716779361/job/32635385282?pr=4665#step:10:887, but it's working fine on my local. Working on a fix...

Edit: Is there any way to reproduce this issue locally so I can identify and test the root cause?

[neteler]

The error is

json_color_out.c:21:10: fatal error: grass/parson.h: No such file or directory

which looks like a development package yet missing from the CI workflow.

[petrasovaa]

The problem is in the Makefile, maybe somebody else can help here?

[wenzeslaus]

Perhaps parson headers are only compiled (build/copied) after the raster library. I didn't look at the makefiles, though.

[NishantBansal2003]

Not sure why the r.colors.out and r3.colors.out pytests are failing in OSGeo4W, especially since they run fine on my local setup and in other CI builds.
cc: @nilason
Could these tests be flaky?

[echoix]

How do we know if the data result is saved in a file, and that file is not the same between tests? How do we make sure that these files are correctly teared down after each test? It is using pytest-xdist, where different modules (test files) can run in parallel.

[echoix]

What about trying to manually run a command in the windows CI, before the tests, to see if the parson output on windows is really what are tests are giving (to see if it is a problem in tests, or a problem with the module output itself on windows)?

[echoix]

Ie: edit the test_thorough.bat, comment out pytest in osgeo4w.yml, as it runs before.

You can try it on a separate PR in against your fork if you don't want to wait as much for CI time, and debug as much as you'd like.

[NishantBansal2003]

I tried to debug this with a separate PR on my fork, and it looks like there is a problem with the module output on Windows. I attempted to run the r.colors.out command with some print statements, but I'm completely clueless about how to fix this issue. Any help would be appreciated: link to the output.
cc: @nilason

[NishantBansal2003]

I tried to debug this, and it looks like only json_object_set_string is not working. When I print the color output string, the string is fine. Also, when I check if the json_object_set_string does not return an error, it runs fine. So, I am not sure what the issue is. Is there anything you can refer me to so I can learn and understand what might be causing the issue and how to fix it?
cc: @nilason @echoix

[echoix]

I've looked up issues in the parson repo for some ideas. From kgabis/parson#154, it makes me think of encoding problems, like the fact that Windows likes UTF16 LE (https://stackoverflow.com/questions/13499920/what-unicode-encoding-utf-8-utf-16-other-does-windows-use-for-its-unicode-da, https://learn.microsoft.com/en-us/windows/win32/intl/unicode). There they talk about the file itself, I'm thinking about the console code page.

Otherwise, there was this that was interesting: kgabis/parson#151 talks about using arrayRecord vs root_object, and thus the types JsonValue vs JsonObject. They link to https://stackoverflow.com/questions/49957648/how-to-construct-json-array-with-parson. There's a similarity with missing values in the json.

Then why only windows doesn't work? Maybe try with a bleeding edge GCC version to see if it does the same on Linux, as the toolchain in msys2/osgeo4w is quite up to date.

[NishantBansal2003]

I tried building GRASS (from this PR) using GCC 14.2.0 on an Ubuntu VM, and I did not encounter any issues that I experienced on Windows. The program runs successfully. However, I have opened a discussion to see if anyone else has encountered a similar issue and can provide a fix. You can check it out here: https://github.com/kgabis/parson/discussions/216 .

[NishantBansal2003]

Key Points to Figure Out the Root Cause of the Issue:

• The issue does not occur when running the code in the module.
• The issue arises when running the code in the library.
• The code works in the library except when using OSGeo4W workflow.
• Only json_object_set_string is causing the issue; json_object_set_number works without problems.
• The output string passed to json_object_set_string appears fine, and no errors are returned, but the string is not being processed as expected.

[NishantBansal2003]

Fixes the issue after discussion on: GitHub discussion link.
Could you please let me know if this is an appropriate way to make changes in the library?

[nilason]

Fixes the issue after discussion on: GitHub discussion link. Could you please let me know if this is an appropriate way to make changes in the library?

That is a workaround which may not be guaranteed to work in the future. It took quite a deal of debugging to finally track this one down. The symbol json_object_set_string, which is problematic is also part of JSON-C API. JSON-C is embedded in GDAL and as the GRASS raster library links against GDAL there is a symbol conflict!

A more stable fix, might be to create a thin GRASS wrapper API (with functions prefixed like G_json_object_set_string()). Such a move definitely deserves a separate PR. Thoughts, @wenzeslaus, @cwhite911 ?

[nilason]

Fixes the issue after discussion on: GitHub discussion link. Could you please let me know if this is an appropriate way to make changes in the library?

That is a workaround which may not be guaranteed to work in the future. It took quite a deal of debugging to finally track this one down. The symbol json_object_set_string, which is problematic is also part of JSON-C API. JSON-C is embedded in GDAL and as the GRASS raster library links against GDAL there is a symbol conflict!

A more stable fix, might be to create a thin GRASS wrapper API (with functions prefixed like G_json_object_set_string()). Such a move definitely deserves a separate PR. Thoughts, @wenzeslaus, @cwhite911 ?

New related issue reported with #4697.

[nilason]

(I should add noticing the tools I used for this on CI: adding mingw-w64-x86_64-ntldd package and executing ntldd --list-imports dist.x86_64-w64-mingw32/lib/libgrass_raster.8.5.dll, ntldd --list-exports dist.x86_64-w64-mingw32/lib/libgrass_raster.8.5.dll. See ntldd help.)

[petrasovaa]

This needs to be updated once #4801 is merged and (hopefully) that should resolve the issue.

[petrasovaa]

This looks great, thank you! Could you or somebody else check the includes of parson.h, they may not be needed, no?

[petrasovaa]

This looks good to me! Thank you!