-
Notifications
You must be signed in to change notification settings - Fork 29
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
Mostly fix Julia 1.0 compatibility #42
Conversation
Ok. I should have checked the list of open PRs before jumping into fixing something. Please feel free to drop this PR if #41 is accepted. |
.travis.yml
Outdated
- 0.5 | ||
- 0.6 | ||
- 0.7 | ||
- 1.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
REQUIRE file and appveyor.yml also need to be updated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
Codecov Report
@@ Coverage Diff @@
## master #42 +/- ##
========================================
+ Coverage 9.09% 100% +90.9%
========================================
Files 2 1 -1
Lines 11 1 -10
========================================
Hits 1 1
+ Misses 10 0 -10
Continue to review full report at Codecov.
|
appveyor.yml
Outdated
- JULIA_URL: "https://julialang-s3.julialang.org/bin/winnt/x86/0.7/julia-0.7-latest-win32.exe" | ||
- JULIA_URL: "https://julialang-s3.julialang.org/bin/winnt/x64/0.7/julia-0.7-latest-win64.exe" | ||
- JULIA_URL: "https://julialang-s3.julialang.org/bin/winnt/x86/1.0/julia-1.0-latest-win32.exe" | ||
- JULIA_URL: "https://julialang-s3.julialang.org/bin/winnt/x64/1.0/julia-1.0-latest-win64.exe" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You'll probably run into problems with this. You can use the template in https://github.com/JuliaCI/Appveyor.jl
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the hint, new commit pushed
appveyor.yml
Outdated
|
||
build_script: | ||
# Need to convert from shallow to complete for Pkg.clone to work | ||
- IF EXIST .git\shallow (git fetch --unshallow) | ||
- C:\projects\julia\bin\julia -e "versioninfo(); | ||
- C:\julia\bin\julia -e "versioninfo(); import Pkg; | ||
Pkg.clone(pwd(), \"LibCURL\"); Pkg.build(\"LibCURL\")" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
versioninfo()
isn't going to work anymore. InteractiveUtils.versioninfo()
will work.
src/LibCURL.jl
Outdated
curl_multi_setopt(handle, opt, ptrval::Integer) = ccall((:curl_multi_setopt, libcurl), CURLMcode, (Ptr{CURLM}, CURLMoption, Clong), handle, opt, ptrval) | ||
curl_multi_setopt{T}(handle, opt, ptrval::Ptr{T}) = ccall((:curl_multi_setopt, libcurl), CURLMcode, (Ptr{CURLM}, CURLMoption, Ptr{T}), handle, opt, ptrval) | ||
curl_multi_setopt(handle, opt, ptrval::T) where {T} = ccall((:curl_multi_setopt, libcurl), CURLMcode, (Ptr{CURLM}, CURLMoption, T), handle, opt, ptrval) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be Ptr{T}
src/lC_common_h.jl
Outdated
@ctypedef curl_sockopt_callback Ptr{Void} | ||
type curl_sockaddr | ||
@ctypedef curl_sockopt_callback Ptr{Cvoid} | ||
struct curl_sockaddr |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be a mutable struct
src/lC_curl_h.jl
Outdated
@c Void curl_formfree (Ptr{Void},) libcurl | ||
@c CURLFORMcode curl_formadd (Ptr{Ptr{Cvoid}}, Ptr{Ptr{Nothing}}) libcurl | ||
@c Cint curl_formget (Ptr{Cvoid}, Ptr{Nothing}, curl_formget_callback) libcurl | ||
@c Cvoid curl_formfree (Ptr{Nothing},) libcurl |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These Nothing
types should be Cvoid
Thanks @iht for getting the ball rolling. I've added some additional changes and I noticed that we still need to create a BinaryBuilder repo for things to work correctly on 1.0. I'll be taking care of that now. |
Thanks @omus, happy to help with anything I could contribute. I think all the changes you have requested are now in this pull request. Looking forward to having 1.0 compatibility :). |
I wonder if this Travis job shouldn't re-triggered https://travis-ci.org/JuliaWeb/LibCURL.jl/jobs/414573418 |
Waiting on: JuliaWeb/LibCURLBuilder#2 |
Progress! Should have this PR updated and working shortly |
The BinaryBuilder update has run into a roadblock at the moment. I'll see if I can wrap this up with Homebrew and WinRPM tomorrow to get a working version of LibCURL.jl out. Sorry for the delay. |
It looks like WinRPM has Julia 1.0 issues. Once those are addressed this PR should be working. I'll get this merged so that at least the master branch of this package will be ready to go. |
I have done some changes to bump this package to Julia 1.0.
I have translated all the
type
declarations intomutable struct
, so instances of those types can be modified. Maybe some of those can be declared juststruct
. I am not very familiar with the code, so I am not sure of how each one of those types are used.Lots of packages depend on this one, and are currently uninstallable on Julia 1.0; merging this PR will make it possible to install those packages.