configure test and requirements for using rapidjson

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

configure test and requirements for using rapidjson

John W. Eaton
Administrator
Hi,

Thanks for working on this new feature.

After

   changeset:   28623:5da49e37a6c9
   user:        Abdallah Elshamy <[hidden email]>
   date:        Thu Aug 13 23:57:07 2020 +0900
   summary:     New functions jsondecode and jsonencode (bug #53100).

I noticed all tests for jsonencode and jsondecode were skipped on my
system.  I have

   ii  rapidjson-dev  1.1.0+dfsg2-6 all          fast JSON
parser/generator for C++ with SAX/DOM style API

installed.  When running configure, I see

   configure: WARNING: RapidJSON 1.1.0 or older found, but latest
development version needed.  Octave will not be able to read or write
JSON files.

The configure test that generates that message checks for
rapidjson/cursorstreamwrapper.h, but the error message is about the
RapidJSON version, and cursorstreamwrapper.h does not appear to be used
directly in the Octave sources.  What feature(s) do we really need here?
  If possible, think we should check for the required features and, if
they are not found, issue a warning about that rather than something
about a version since the need for a development version will likely
change soon.

Thanks,

jwe

Reply | Threaded
Open this post in threaded view
|

Re: configure test and requirements for using rapidjson

siko1056
On 8/18/20 2:06 AM, John W. Eaton wrote:

> Hi,
>
> Thanks for working on this new feature.
>
> After
>
>   changeset:   28623:5da49e37a6c9
>   user:        Abdallah Elshamy <[hidden email]>
>   date:        Thu Aug 13 23:57:07 2020 +0900
>   summary:     New functions jsondecode and jsonencode (bug #53100).
>
> I noticed all tests for jsonencode and jsondecode were skipped on my
> system.  I have
>
>   ii  rapidjson-dev  1.1.0+dfsg2-6 all          fast JSON
> parser/generator for C++ with SAX/DOM style API
>
> installed.  When running configure, I see
>
>   configure: WARNING: RapidJSON 1.1.0 or older found, but latest
> development version needed.  Octave will not be able to read or write
> JSON files.
>
> The configure test that generates that message checks for
> rapidjson/cursorstreamwrapper.h, but the error message is about the
> RapidJSON version, and cursorstreamwrapper.h does not appear to be used
> directly in the Octave sources.  What feature(s) do we really need here?
>  If possible, think we should check for the required features and, if
> they are not found, issue a warning about that rather than something
> about a version since the need for a development version will likely
> change soon.
>
> Thanks,
>
> jwe
>

Short:
Abdallah is still working on it [1].  RapidJSON is a header only library
and is only needed during the Octave build.  There will be two ways to
deal with this:

1. Abdallah makes some newer RapidJSON features selectively available
[1].  ~95% of his code will work with RapidJSON 1.1.0 (which is as
package available in every Linux distribution newer than 2016).  I push
his changes soon after review today.

2. Download the latest RapidJSON library [2] and make it available
during the Octave build `CPPFLAGS=-I/path/to/rapidjson/include` or
default system paths.


Long:
The last public release of RapidJSON [3] was 2016-08-25.  Since then
some useful bug fixes and improvements (a pretty-writer, etc.) came into
the library.  All begging by RapidJSON users didn't motivate the
maintainers for a new release.  But it is still the fastest JSON
implementation around [4].

I pushed Abdallah's changeset a little ahead of time for our motivation.
 His major GSoC tasks are done ahead of time.  I am very satisfied with
his work.  Now comes extensions and more polishing.

Often our GSoC candidate's code wasn't applied even half year later and
I wanted to avoid this GSoC final cset push paralysis.  The problem with
this "final changeset" is, that there is always something to improve.
The frustration with the student and mentor grows when having to create
this "final changeset" cset and review over and over again.  And even
after the push there were errors to be fixed.

Thus my idea this year was to apply the working code early to the
development repository.

Kai

[1]
https://github.com/Abdallah-Elshamy/octave/commit/8e4c55fdfbd9220828b8ae74c561224bba3288ff
[2] https://github.com/Tencent/rapidjson/archive/master.zip
[3] https://github.com/Tencent/rapidjson/releases/tag/v1.1.0
[4] https://github.com/miloyip/nativejson-benchmark

Reply | Threaded
Open this post in threaded view
|

Re: configure test and requirements for using rapidjson

John W. Eaton
Administrator
On 8/17/20 9:44 PM, Kai Torben Ohlhus wrote:

> Short:
> Abdallah is still working on it [1].  RapidJSON is a header only library
> and is only needed during the Octave build.  There will be two ways to
> deal with this:

OK.  Sorry if my message seemed overly critical, I did not mean it to be.

> Long:
> The last public release of RapidJSON [3] was 2016-08-25.  Since then
> some useful bug fixes and improvements (a pretty-writer, etc.) came into
> the library.  All begging by RapidJSON users didn't motivate the
> maintainers for a new release.  But it is still the fastest JSON
> implementation around [4].

Thanks for the info.  I didn't know about the release status.

> Often our GSoC candidate's code wasn't applied even half year later and
> I wanted to avoid this GSoC final cset push paralysis.  The problem with
> this "final changeset" is, that there is always something to improve.
> The frustration with the student and mentor grows when having to create
> this "final changeset" cset and review over and over again.  And even
> after the push there were errors to be fixed.
>
> Thus my idea this year was to apply the working code early to the
> development repository.

I agree.  Thanks for your work on this project.

jwe