-
Notifications
You must be signed in to change notification settings - Fork 222
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use libcurl for HTTP transfers #218
base: master
Are you sure you want to change the base?
Conversation
Advantage of libcurl over wxHTTP is that we get HTTPS support out of the box. Disadvantage is it being another dependency. Therefore using libcurl is optional now, but strongly encouraged. Since HTTP download was the last piece of network functions not replaced by Boost.Asio, using libcurl together with boost makes it possible to forget wxWidgets networking completely.
Minimum required libcurl version is now 7.32.0, required by CURLOPT_XFERINFOFUNCTION. Since it's already seven years old, we're not planning to support older versions.
This is mainly a demonstration of how we can select new functionality if available. First, we have to check the compile-time headers version whether they have the new functionality or not, to be able to compile it in. Then, we have to check the library version at runtime because that might very well be different from what we used for compilation. If both checks succeed then and only then can we use the new interface. In all other cases we need to revert to use the old interface.
We use the progress callback to periodically check whether the current download is requested to be cancelled (i.e. the user pressed the 'Cancel' button, or the application is exiting). That requires that we always have a progress callback installed, therefore the minimum required libcurl version is lowered to 7.19.1.
@Vollstrecker could you please take a look on 71e56dd ? I'm still new to debian packaging... Also, although I set the required minimum version to >= 7.19.1, the binary packages contain a dependency on >= 7.16.2. Is there a way to override that (maybe without knowing the exact name of the dependent binary package)? |
Even oldoldstable has 7.38, so I would omit the version at all. But it doesn't harm to have it in. |
Thank you |
The
|
|
Thanks, and I see, it uses libcurl on Linux, anyway. However, it won't be available before 3.2.0 and we have problems even with 3.1.x.... not to say we still support 2.8.12. Even if we'll once change to it completely we need a rewrite of our HTTP download logic. |
I can conform it works on Windows, but some improvement to the autotools build system are needed:
BTW, MinGW GCC will report the error "cannot convert ‘LPCTSTR’ {aka ‘const char*’} to ‘LPCWSTR’ {aka ‘const wchar_t*’}" when aMule is built with boost, pupnp or libcurl, see #275. The workaround is to include the wxwidgets headers before the headers of boost, pupnp and libcurl. |
Although commonly downloaded files (server.met, nodes.dat, etc.) are now available via plain HTTP, issues #197 and #193 prove it wasn't always the case. libcurl supports HTTPS for us out of the box.
It is now optional to use libcurl, as it's a new dependency, but this might change in the future.
Since HTTP download was the only piece of neworking code not covered by Boost.Asio, combined with libcurl now allows for completely forgetting about wxWidgets networking.
The patch is working but isn't yet complete, I provide it here mainly for testing purposes. Please report any failures here. The testing branch has been updated with the contents of this pull request.
TODO: