Merge lp://staging/~gary-wzl77/net-cpp/dual_landing_support into lp://staging/net-cpp

"changelog.in" is not valid, bileto generates the changelog for you, get rid of that.

(better explanation inline)

One more thing inline.

Marcus, my reviews above explain the failures in bileto, it must be fixed before you can build.

Ok, this looks good, one minor nitpick inline.

Okay, here's a list of todo items before we can land this:

1. split the speed limit/duration member of Request::Configuration out and allow the user to configure it through an extra method on the Client class.

2. Ensure that the defaults for this setting match what previous net-cpp did. The current limit=1, duration=30s values seem different to the default:

https://curl.haxx.se/libcurl/c/CURLOPT_LOW_SPEED_LIMIT.html (limit=0)
https://curl.haxx.se/libcurl/c/CURLOPT_LOW_SPEED_TIME.html (duration=0)

3. Set SOVERSION to 1 for vivid, and 2 for anything else.

3. Resurrect the libnet-cpp1.symbols file from current trunk, and the libnet-cpp2.symbols file from the current net-cpp package in yakkety. Both files would need to be updated to include the new methods on Client.

Alternatively, ship libnet-cpp1.shlibs and libnet-cpp2.shlibs files with appropriate contents, but it is probably going to be easier to prove things are compatible to the landing team by sticking with the existing symbols files.

It looks like this isn't binary compatible yet. I did the following:

1. pulled this branch and built it using RelWithDebInfo build type.

2. Grabbed the latest package version and did likewise:

    dget https://launchpad.net/ubuntu/+archive/primary/+files/net-cpp_2.0.0-0ubuntu2.dsc

I modified the top-level CMakeLists.txt to disable -Werror rather than trying to fix the warnings that have already been addressed in this branch.

3. replaced the libnet-cpp.so.2 in my net-cpp-2.0.0 build tree with a symlink to the library in dual_landing_support, and then ran the tests.

The result was http_streaming_client_test segfaulting.

I rebuilt both libraries with "-fvisibility=hidden" removed from the compile flags to see if I could get better information from abigail, which gave me the following diff: http://paste.ubuntu.com/23063717/

So this is showing all the virtuals from StreamingClient have been offset by two in the vtable in the new version of the library. I'm kind of surprised this didn't show up when we checked it back on Friday, so this might be a limitation in Abigail.

I think the only way we can work around this is to make the two new methods on Client into normal non-virtual methods. Their implementation would effectively do manual dynamic dispatch. Something like:

std::shared_ptr<http::Request> http::Client::post(const http::Request::Configuration& configuration, std::istream& payload, std::size_t size)
{
    auto *curl_client = dynamic_cast<http::impl::curl::Client*>(this);
    if (curl_client)
    {
        return curl_client->post(configuration, payload, size);
    }
    throw some_exception(...);
}

It'd probably be best to put these thunks in a separate file to the other methods from client.cpp, and add a FIXME noting the problem.

I also tested these branches on my vivid/armhf Chromebook, and got a similar segfault in http_streaming_client_test after copying the library between build trees.

And one last small issue: you've set the project version number in the CMakeLists.txt back to 1.3.0, while the Debian version number is 2.1.0.

Since net-cpp 2.0.0 has been released into xenial and yakkety, this new version needs to have a version number greater than that, so I'd suggest setting it to 2.1.0 everywhere.

Thanks for your detailed reply. James.
Basically, from the results of abidiff,
making the two new methods(del/post) on Client into normal non-virtual methods is not enough due to http_client_test failed(segfaulting).

As new added three methods(pause/resume/abort_request_if) on Request class still have bad effect on the offset(+1)
in the vtable for Request::async_execute and Request::execute.
http://paste.ubuntu.com/23063717/

    the vtable offset of method virtual void core::net::http::impl::curl::Request::async_execute(const core::net::http::Request::Handler&, const

core::net::http::StreamingRequest::DataHandler&) changed from 9 to 12

    the vtable offset of method virtual core::net::http::Response core::net::http::impl::curl::Request::execute(const core::net::http::Request::ProgressHandler&, const

core::net::http::StreamingRequest::DataHandler&) changed from 8 to 11

That's why when i check the uploaded abidiff results the above two virtuals from Request have been offset by *three* instead of *two* in the vtable in the new version of the library.

moving pause/resume/abort_request_if from Request to Streaming_Request will fix above issue.

The recent changes are looking a lot better. Here's an updated abidiff report after disabling -fvisibility=hidden: http://paste.ubuntu.com/23066000/

Just a few small issues with the changes in the most recent revision:

1. for the new symbols you've added to the .symbols files, set the version number to "0replaceme" rather than "2.1.0-0ubuntu1". CI Train will automatically replace that tag with the final version number for the released package.

2. I notice you've made a variable and two functions in src/core/net/http/impl/curl/request.h static. Was there any particular reason for this? My understanding is that this will result in the variable being instantiated once for each file the header is included in, which seems unnecessary.

The symbols are hidden anyway, so it doesn't improve things from an ABI perspective.

Looks good. This seems to fix all the ABI issues, and the packaging changes seem solid.

LGTM.