[gdal-dev] json-c 0.10 update status and json-c copy removal proposed

[Mateusz Loskot]

On 15 July 2012 11:01, Even Rouault <even.rouault at mines-paris.org> wrote:
>>
>> I'm trying to update GDAL's copy of json-c library.
>> The task is not trivial, due to the fact GDAL has applied
>> some sort of improvements to json-c, and also due to
>> significant changes to how json-c works, UTF-8 support, etc.
>
> Just curious about the changes in UTF-8 support : does json-c do any transcoding
> now ?

It does not do any transcoding. It's just that UTF-8 is first class
citizen there now.

> It turns out it does not, I would expect that previous versions dealt with UTF-8 without doing
> anything particular.

Perhaps it dealt.

>> Some of GDAL's fixes have been submitted and accepted to the upstream,
>> but there are a few which hasn't, like custom function
>> json_object_new_double_with_precision(), or use of CPL string functions.
>>
>> Personally, I don't really see benefits of using CPL functions in json-c.
>> The json_object_new_double_with_precision could be moved out to GDAL
>> source files.
>
> Hum, I see I'm the one to blame for json_object_new_double_with_precision().
> (This was done at a time where I wrongly assumed that there was no libjsonc
> upstream activity). Unfortunately it won't be possible to implement that only in
> a GDAL specific file, because it requires an additional field in a private
> structure of libjson-c.

Yes, you are right. I've forgot about this extra member.

> The implementation as it is currently is a bit bad
> looking, so I can understand it will be difficult to upstream. Perhaps a simpler
> version that does not try to do clever truncation of trailing 0's could be
> accepted. Something like that (uncompiled & untested, being only with an email
> environmenent right no tw):
> [...]
> or perhaps a version where instead of passing the precision, we would pass a
> pointer to a callback and a user_data pointer, so that the formatting function
> could be completely implemented on GDAL side ?
>[...]
> Actually, after having written the above, I'm wondering if a version where we
> would directly pass the custom string representation in
> [...]

I have no idea what could be submitted and what couldn't.
So, if you care, submit a pull request
https://github.com/json-c/json-c
or just forward these functions to
https://groups.google.com/forum/#!forum/json-c

I'm not really interested in getting involved with the json-c development myself
any deeper than I have got already.
If I hadn't made mistake including json-c sources in GDAL, I wouldn't have to
write this e-mail and discuss how to get rid of it :)

> If it is not accepted, that's not critical. It was an enhancement (
> http://trac.osgeo.org/gdal/ticket/4108 ), but we could
> live without it and just revert to json_object_new_double().

Good.

>> So, if we could compromise and give up on GDAL-specific json-c version,
>> I'd like to propose to completely remove json-c sources from GDAL
>> and rely on externally provided json-c (libjsonc)
>> Especially, that building json-c is versy simple...if its native build
>> configuration is used.
>
> It would be good if you could coordinate with Tamas and OSGeo4W to make sure
> that they have a libjson binary, so that the most popular builds for Windows
> don't loose geojson support.

I have patched json-c for Windows and Visual C++, and patches have
been accepted,
so I guess there is no problem with building for Windows/OSGeo4W.

>> I don't see any reason why GDAL couldn't rely on external json-c.
>> So, I'd like to propose to:
>> - remove GDAL's copy of json-c from ogr/ogrsf_frmts/geojson/jsonc
>> - configure Unix and Windows builds to use external binaries
>
> I assume you will deal with the libjsonc dependency as an optional one ?

If GDAL agrees to use external libjsonc, then I will make it optional
in build config.
If that's what you mean.

> I'm not sure if you're aware of it, but in addition to the OGR GeoJSON et CouchDB
> drivers, there's also the new GDAL ARG driver that depends on libjsonc.

I know about the CouchDB, but didn't know about the ARG.

>The corresponding autotests of the 3 drivers should likely be modified to skip if
> the drivers aren't compiled in.

Yes.

> For the ease of using on Linux, it would be good if libjson-c 0.9 could still be
> used, being the version currently packaged.

I have no idea to what extent the API has changed in 0.10, but I can
give it a try.

>> I'm volunteering to apply those changes.
>> Please, shall I make RFC, call for votes or this e-mail is sufficient to
>> discuss and make decision?
>
> I believe this email is sufficient.

To summary, I plan the following steps during next week or so:

0. Work will happen in trunk only
1. I will remove the json-c sources
2. I will configure build to use external libjsonc (preferably 0.9+)
3. I will update the drivers, if necessary.
4. I will update the tests, if necessary.
5. Once confirmed sound on Linux, I will push changes as single commit.
6. I continue testing on Windows (hopefully someone will be willing to
help with testing for Windows).

Do you agree?

I think I'll make a ticket for reference too.

Best regards
-- 
Mateusz Loskot, http://mateusz.loskot.net