Update JSON for Modern C++ to version 3

[LocutusOfBorg]

Patch configure file to find the new json.hpp file location

[LocutusOfBorg]

https://tracker.debian.org/pkg/nlohmann-json3
this is the new version of nlohmann-json package, we split the packaging because some packages are incompatible with the new version.
Poedit seems to build fine with it

[vslavik]

Sure, let's upgrade the version if we can — but this PR is nowhere near complete.

As long as the bundled copy is older, it doesn't make sense to support only the new version (I'm OK with supporting only the new one because there's a bundled copy). At the same tike, I'd like to avoid complicated support for both. Includes needs to be updated in code too.

The reason why the builtin copy wasn't updated in a while is nlohmann/json#610 and so this PR cannot be included until:

• builtin copy is upgraded to 3.x
• everything still builds on all platforms with this version — you appear to have quickly made the change using web interface, so I'm not sure if you tried compiling at least on Debian?
• the boost situation is resolved somehow:
  • may no longer apply to 3.x
  • stop using boost::future in Poedit code
  • wait it out until old Boost.Thread is carried by no distro of interest (currently, Ubuntu 16.04 remains of some interest)

[LocutusOfBorg]

sorry, the AC macro changed the defined value, so I wrongly used the embedded version instead of the system one -.-' stupid mistake! I corrected the code to now look for all of the variants, so people can choose their best version

As long as the bundled copy is older, it doesn't make sense to support only the new version (I'm OK >with supporting only the new one because there's a bundled copy). At the same tike, I'd like to avoid >complicated support for both. Includes needs to be updated in code too.

as you wish, let me know what you prefer :)

The reason why the builtin copy wasn't updated in a while is nlohmann/json#610 and so this PR cannot >be included until:
builtin copy is upgraded to 3.x
everything still builds on all platforms with this version — you appear to have quickly made the change >using web interface, so I'm not sure if you tried compiling at least on Debian?

yes, it works correctly in Debian (I was wrongly using the bundled copy because of the defines changed, but I checked again and it is fine now)

the boost situation is resolved somehow:
may no longer apply to 3.x

yes, it works with the latest boost in Ubuntu (we did default to 1.67 some days ago)
https://launchpadlibrarian.net/381063905/buildlog_ubuntu-cosmic-s390x.poedit_2.1.1-3~build1_BUILDING.txt.gz

stop using boost::future in Poedit code
wait it out until old Boost.Thread is carried by no distro of interest (currently, Ubuntu 16.04 remains of >some interest)

I think this isn't an issue wrt boost

[LocutusOfBorg]

So, right now, the patch I submitted here works in both Debian, and Ubuntu, and uses the new json 3 version that seems to have less gcc troubles, and less boost troubles.

If you want me to update the submodule I can do, but testing is still relegated to Debian/Ubuntu versions.

[LocutusOfBorg]

I did update also the submodule to 3.1.2 tag (I'm not sure we should blindly follow devel branch...)

[vslavik]

it works with the latest boost

Good for you, but the concern was specifically about old distros.

I think this isn't an issue wrt boost

If it cannot be compiled on (a bit) older distros, it is an issue. If it cannot be compiled on 16.04, it is an issue for me...

I cannot apply this as is. I don't like supporting both versions (as previously stated), and won't merge this without verifying it works with Boost ≤ 1.64 first (or make a conscious decision to stop supporting that). If you can't test the PR for that, that's fine, I will, but I can't promise how quickly that'll be.

(It also should be a single clean commit, not a series of partials, but that's nitpicking.)

[GitCop]

Thanks for contributing! Unfortunately there were the following style issues with your Pull Request:

• Commit: 3a28c58
• Your subject line is longer than 50 characters

Please follow git commit message guidelines (you can use git push -f to update this PR).

---

This message was auto-generated by https://gitcop.com

[vslavik]

When you post compilation logs, please post only the relevant bits. I got lost, repeatedly, in this enormous wall of mostly irrelevant output. I see some errors there. I'm not sure whether you're saying it is OK or not OK. There's some mention of "this patch", did you forget to link to it or are you referring to the PR? If the latter, what is the first output about?

[LocutusOfBorg]

testing I performed. (everything starting from version 2.1.1 released)
a)

• install everything in xenial,
• backport boost and pugixml and jsoncpp to ppa and use them.
• relax wx dependency to 3.0.2

do the compilation, everything built correctly
b)

• remove jsoncpp from the system, apply this "patch" (pull request) and try a nw build
  with an "#error" statement to be sure the right json.hpp was used (and it failed)
• remove that "#error", build was successful.

so, with the above two points, I can say that on Ubuntu 16.04, with appropriate backports, everything still build correctly, with bundled new json, or with a backported system one.

I can't test other OS, but to me it looks good (wrt Debian/Ubuntu).
Removing the old version as fallback might make somebody sad, due to an additional backport, but nlhmann/json is really trivial to backport, and builds even with g++-5 with no issues

[vslavik]

backport boost

That's not the point. The relevant piece of information is whether this PR can be built with Boost ≤ 1.64. Of course you'll be able to build if you use the latest Boost version, but such invasive backports are not a reasonable thing to expect from people.

Again, the interesting thing is whether after applying this PR the whole thing can be compiled on e.g. Ubuntu 16.04 as is (except installing non-stock dependencies).

Travis build of this PR passes, so I guess it is OK. I'm deeply confused by its log, though, because it shows some issue:

  CXX      crowdin_client.o
In file included from /usr/include/boost/shared_ptr.hpp:17:0,
                 from /usr/include/boost/thread/csbl/memory/shared_ptr.hpp:30,
                 from /usr/include/boost/thread/detail/nullary_function.hpp:16,
                 from /usr/include/boost/thread/executors/work.hpp:11,
                 from /usr/include/boost/thread/executors/executor.hpp:16,
                 from /usr/include/boost/thread/executor.hpp:12,
                 from ../../src/concurrency.h:46,
                 from ../../src/crowdin_client.h:33,
                 from ../../src/crowdin_client.cpp:27:
/usr/include/boost/thread/future.hpp: In instantiation of ‘boost::future<R>::move_dest_type boost::future<R>::get() [with R = nlohmann::basic_json<>; boost::future<R>::move_dest_type = nlohmann::basic_json<>]’:
../../src/concurrency.h:209:66:   required from ‘static T dispatch::detail::continuation_calling_helper<T>::unpack_arg(boost::future<T>&&) [with T = nlohmann::basic_json<>]’
../../src/concurrency.h:365:93:   required from ‘dispatch::future<T>::then(F&&)::<lambda(boost::future<T>)> [with F = CrowdinClient::GetUserInfo()::<lambda(json)>; T = nlohmann::basic_json<>]’
../../src/concurrency.h:364:56:   required from ‘struct dispatch::future<T>::then(F&&) [with F = CrowdinClient::GetUserInfo()::<lambda(json)>; T = nlohmann::basic_json<>; typename dispatch::detail::future_unwrapper<typename std::result_of<F(T)>::type>::type = CrowdinClient::UserInfo]::<lambda(class boost::future<nlohmann::basic_json<> >)>’
../../src/concurrency.h:366:31:   required from ‘dispatch::future<typename dispatch::detail::future_unwrapper<typename std::result_of<F(T)>::type>::type> dispatch::future<T>::then(F&&) [with F = CrowdinClient::GetUserInfo()::<lambda(json)>; T = nlohmann::basic_json<>; typename dispatch::detail::future_unwrapper<typename std::result_of<F(T)>::type>::type = CrowdinClient::UserInfo]’
../../src/crowdin_client.cpp:177:10:   required from here
/usr/include/boost/smart_ptr/shared_ptr.hpp:796:31: note: candidate 1: bool boost::operator==(const boost::shared_ptr<X>&, boost::detail::sp_nullptr_t) [with T = boost::detail::shared_state<nlohmann::basic_json<> >; boost::detail::sp_nullptr_t = std::nullptr_t]
 template<class T> inline bool operator==( shared_ptr<T> const & p, boost::detail::sp_nullptr_t ) BOOST_NOEXCEPT
                               ^~~~~~~~
In file included from ../../src/json.h:34:0,
                 from ../../src/http_client.h:32,
                 from ../../src/crowdin_client.cpp:29:
../../src/../deps/json/single_include/nlohmann/json.hpp:15149:17: note: candidate 2: bool nlohmann::operator==(nlohmann::basic_json<ObjectType, ArrayType, StringType, BooleanType, NumberIntegerType, NumberUnsignedType, NumberFloatType, AllocatorType, JSONSerializer>::const_reference, ScalarType) [with ScalarType = int; typename std::enable_if<std::is_scalar<ScalarType>::value, int>::type <anonymous> = 0; ObjectType = std::map; ArrayType = std::vector; StringType = std::basic_string<char>; BooleanType = bool; NumberIntegerType = long int; NumberUnsignedType = long unsigned int; NumberFloatType = double; AllocatorType = std::allocator; JSONSerializer = nlohmann::adl_serializer; nlohmann::basic_json<ObjectType, ArrayType, StringType, BooleanType, NumberIntegerType, NumberUnsignedType, NumberFloatType, AllocatorType, JSONSerializer>::const_reference = const nlohmann::basic_json<>&; nlohmann::basic_json<ObjectType, ArrayType, StringType, BooleanType, NumberIntegerType, NumberUnsignedType, NumberFloatType, AllocatorType, JSONSerializer>::value_type = nlohmann::basic_json<>]
     friend bool operator==(const_reference lhs, const ScalarType rhs) noexcept
                 ^~~~~~~~

But: there's location/inclusion information, there are candidates listed, but there's no actual error in the output. WTH?

[LocutusOfBorg]

That's not the point. The relevant piece of information is whether this PR can be built with Boost ≤ 1.64. >Of course you'll be able to build if you use the latest Boost version, but such invasive backports are not >a reasonable thing to expect from people.

I used boost 1.61, xenial has no this boost available by default.
xenial has a boost default pointing to 1.58, and I backported 1.61 to test this PR, while the latest version is 1.67.

So, I think in any case, if you want to use xenial codebase, you have to backport a boost from somewhere.
https://launchpad.net/ubuntu/+source/boost-defaults/1.58.0.1ubuntu1

[LocutusOfBorg]

wrt the build warning, I don't really know, I see in Debian logs too (with system json and new boost), but nobody ever complained about something not working :)

[vslavik]

wrt the build warning,

It's not a warning — it's just notes normally accompanying warnings or errors, in a place where the combination with boost 1.61 normally produces an error, and still does in this PR with clang:

In file included from /Users/vaclav/Development/poedit2/src/concurrency.h:47:
/Users/vaclav/Development/poedit2/deps/boost/boost/thread/future.hpp:1678:31: error: use of overloaded operator '==' is ambiguous (with operand types 'boost::detail::basic_future<nlohmann::basic_json<std::map, std::vector, std::__1::basic_string<char>, bool, long long, unsigned long long, double, std::allocator, adl_serializer> >::future_ptr' (aka 'shared_ptr<detail::shared_state<basic_json<std::map, std::vector, basic_string<char>, bool, long long, unsigned long long, double, std::allocator, adl_serializer> > >') and 'int')
            if (this->future_ == 0)
                ~~~~~~~~~~~~~ ^  ~
In file included from /Users/vaclav/Development/poedit2/src/crowdin_client.cpp:1:
In file included from /Users/vaclav/Development/poedit2/src/Poedit-Prefix.pch:7:
/Users/vaclav/Development/poedit2/src/concurrency.h:209:62: note: in instantiation of member function 'boost::future<nlohmann::basic_json<std::map, std::vector, std::__1::basic_string<char>, bool, long long, unsigned long long, double, std::allocator, adl_serializer> >::get' requested here
    static T unpack_arg(boost::future<T>&& arg) { return arg.get(); }
                                                             ^
/Users/vaclav/Development/poedit2/src/concurrency.h:365:83: note: in instantiation of member function 'dispatch::detail::continuation_calling_helper<nlohmann::basic_json<std::map, std::vector, std::__1::basic_string<char>, bool, long long, unsigned long long, double, std::allocator, adl_serializer> >::unpack_arg' requested here
                                 return detail::call_and_unwrap_if_future(f, cch::unpack_arg(std::move(x)));
                                                                                  ^
/Users/vaclav/Development/poedit2/src/crowdin_client.cpp:170:10: note: in instantiation of function template specialization 'dispatch::future<nlohmann::basic_json<std::map, std::vector, std::__1::basic_string<char>, bool, long long, unsigned long long, double, std::allocator, adl_serializer> >::then<(lambda at /Users/vaclav/Development/poedit2/src/crowdin_client.cpp:170:15)>' requested here
        .then([](json r)
         ^
In file included from /Users/vaclav/Development/poedit2/src/crowdin_client.cpp:1:
In file included from /Users/vaclav/Development/poedit2/src/Poedit-Prefix.pch:7:
In file included from /Users/vaclav/Development/poedit2/src/concurrency.h:46:
In file included from /Users/vaclav/Development/poedit2/deps/boost/boost/thread/executor.hpp:12:
In file included from /Users/vaclav/Development/poedit2/deps/boost/boost/thread/executors/executor.hpp:16:
In file included from /Users/vaclav/Development/poedit2/deps/boost/boost/thread/executors/work.hpp:11:
In file included from /Users/vaclav/Development/poedit2/deps/boost/boost/thread/detail/nullary_function.hpp:16:
In file included from /Users/vaclav/Development/poedit2/deps/boost/boost/thread/csbl/memory/shared_ptr.hpp:30:
In file included from /Users/vaclav/Development/poedit2/deps/boost/boost/shared_ptr.hpp:17:
/Users/vaclav/Development/poedit2/deps/boost/boost/smart_ptr/shared_ptr.hpp:796:31: note: candidate function [with T = boost::detail::shared_state<nlohmann::basic_json<std::map, std::vector, std::__1::basic_string<char>, bool, long long, unsigned long long, double, std::allocator, adl_serializer> >]
template<class T> inline bool operator==( shared_ptr<T> const & p, boost::detail::sp_nullptr_t ) BOOST_NOEXCEPT
                              ^
In file included from /Users/vaclav/Development/poedit2/src/crowdin_client.cpp:29:
In file included from /Users/vaclav/Development/poedit2/src/http_client.h:32:
In file included from /Users/vaclav/Development/poedit2/src/json.h:34:
/Users/vaclav/Development/poedit2/src/../deps/json/single_include/nlohmann/json.hpp:15149:17: note: candidate function [with ScalarType = int, $1 = 0]
    friend bool operator==(const_reference lhs, const ScalarType rhs) noexcept
                ^
/Users/vaclav/Development/poedit2/src/../deps/json/single_include/nlohmann/json.hpp:15078:17: note: candidate function
    friend bool operator==(const_reference lhs, const_reference rhs) noexcept
                ^
1 error generated.

The Travis build currently uses Boost 1.61 too. To apply this PR, I'll need to

• update Boost in deps/boost first
• update README with higher requirements on Boost
• pull newer Boost images for trusty+xenial — ppa:mhier/libboost-latest looks like a usable source (might even be used for Snap packaging #468 then, although it's probably safer not to)
• alternatively: migrate builds to AppVeyor which has a modern Ubuntu version available.

[LocutusOfBorg]

update Boost in deps/boost first

I'm not really sure about this point... I tried to unpack the new boost, since it seems to be not under version control, but there are modifications from you, and I'm not sure the files in the directory should just be deleted and the new version unpacked... can you please give me an hint?

[vslavik]

can you please give me an hint?

Vendor branches. Upstream boost is in the vendor/boost branch, merged into the tree using
git merge -s subtree -X subtree=deps/boost vendor/boost

IOW, it's not really something that can be easily done in a PR…

Part of why I didn't update Boost in a while is that they migrated to git, opening the way to use it as a submodule (which would be preferable, even though it won't help with polluted huge history)… but they have split it into multiple repos and migrating to that is downright scary — it means either

1. some processing script to install headers, i.e. nontrivial change to the way it is built now; or
2. about a gazillion or two of -I flags (in makefiles, Xcode and MSVC projects); or
3. using some smart (generated) wrapping headers

[vslavik]

Is it an issue for Debian to bump the JSON dependency mid-cycle, i.e. in 2.2.1 as opposed to waiting to 2.3?

[LocutusOfBorg]

Ehm, Debian is already using nlohmann-json3, so 3.1.2 right now, and 3.5 in two days (I just updated it to 3.5 and uploaded)

[vslavik]

Ehm, Debian is already using nlohmann-json3, so 3.1.2 right now, and 3.5 in two days (I just updated it to 3.5 and uploaded)

I'm aware. What I'm not aware of is your policies when changing dependencies of packages in more stable branches of the distro (if even poedit-2.2 is in stable or testing; I don't know that either).

[LocutusOfBorg]

poedit 1.8 is in stable
poedit 2.2 is in unstable/testing (and testing is going to freeze in the next days/weeks)