Use BinDeps.download_cmd for Windows. #81
Conversation
|
this does lose the retrying which is a bit unfortunate, could that be kept? |
|
How does that look? |
| @@ -43,22 +43,13 @@ function __init__() | |||
| end | |||
|
|
|||
| @unix_only download(source::AbstractString) = (x=HTTPC.get(source); (bytestring(x.body),x.http_code)) | |||
| @windows_only function download(source::AbstractString; retry = 5) | |||
| dest = Array(UInt16,261) | |||
| @windows_only function download(source::AbstractString, retry = 5) | |||
There was a problem hiding this comment.
retry was a keyword argument before, not positional
|
I'm still a novice with Git, but decided to go ahead and try a rebase to make it cleaner. How is that? |
| warn("Unknown download failure, error code: $res") | ||
| run(BinDeps.download_cmd(source,filename)) | ||
| if isfile(filename) | ||
| return readall(filename),200 |
There was a problem hiding this comment.
what we should try to do for cleanup is something like
str = readall(filename)
rm(filename)
return str, 200
|
👍 Thanks. Added the cleanup (and rebased again). How's that? |
|
Will need a fair bit of testing, but looks like it might do the job. Fair warning, #77 is in progress to fix the deprecation warnings, and may result in this needing a rebase. |
| C_NULL,utf16(source),dest,sizeof(dest)>>1,0,C_NULL) | ||
| if res == 0 | ||
| resize!(dest, findfirst(dest, 0)) | ||
| filename = LegacyStrings.utf8(UTF16String(dest)) |
There was a problem hiding this comment.
good news is it looks like this will allow us to remove LegacyStrings from REQUIRE
|
@tkelman Should this have been merged in July (before conflicts were introduced)? |
|
It hadn't been tested very thoroughly yet, so it wasn't entirely ready to go. |
|
So what's the way forward? Merge it and ask for people to test it in various configurations before tagging a release? There have been many reports of issues, so we could probably get some feedback from users. |
|
Yeah, something like that. We should probably set up AppVeyor here in some way, and be sure to test on both Server versions of Windows (which have different default security policies that have made a difference here) and when running Julia inside mintty (which has caused freezes when using BinDeps.download_cmd in a few cases, IIRC). |
|
Hope this get merged soon. Installation issue is really painful. |
This PR assumes a folder called
cacheexists. Will this cause problems?It also leaves .gz files in the
cachefolder. Suggestions welcome 😅Closes #80