-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
add Downloads stdlib #37340
add Downloads stdlib #37340
Conversation
ce8ebdf
to
c7c4d35
Compare
085583d
to
ba2a4d9
Compare
78619e9
to
1957fed
Compare
be36c1f
to
70652d7
Compare
Aside from passing CI, I think this PR is mergeable. @KristofferC, you had marked it as "needs docs" but I'm not entirely sure why—was that because the Downloads stdlib doesn't have docs? Currently, I'm just viewing this as a replacement for existing functionality and I've updated the |
70652d7
to
24600f4
Compare
The Win64 tester keeps hanging, I have no idea why. It was passing previously, so I'm not sure if this is related to this PR or just because there's something up with the infrastructure. @staticfloat, any idea what's up? |
This is awesome, and closes many long standing issues. If this is now a stdlib, shouldn't there now be a depwarn for |
Yes, I could add one since deprecation warnings default to off these days, right? |
Yep, and the next step is to use it for downloads in Pkg as well, so even more problems solved there. Figuring out how to use libcurl correctly from Julia with libuv was pretty tricky, but now that it works, libcurl is great: it's very portable and has every feature we might ever want for downloading/uploading stuff—all the protocols, progress support, authentication, etc. Since I have a good understanding of the API now, if we need any additional features in the future, I'm pretty confident that libcurl has them and that we can add them easily. |
24600f4
to
b88d280
Compare
d5ae7e0
to
2c20730
Compare
Regarding the docs question, I just meant an entry in the manual so it is listed as an stdlib with the API it has, like the other stdlibs. |
Right, I can add that once I've got it working. |
8678895
to
7ef18fa
Compare
Ok, well, this finally works. Turns out the Win64 hang was a libcurl bug. Upgrading libcurl fixes it, but requires adding nghttp2 as a dependency. That's fine, since we want to support HTTP/2 anyway. For the record fde145d was the commit that worked. |
fde145d
to
add692c
Compare
Anything else needed here besides news and a manual section? |
add692c
to
7aad328
Compare
7aad328
to
a561a39
Compare
- Base.download is deprecated in favor of Downloads.download - the former is now implemented by calling the latter
a561a39
to
a39c252
Compare
Deprecating The best fix for that is to just change it to use |
This is pretty amazing! Off-topic, we really need to move away from having a file for each download hash. |
@StefanKarpinski can you try
|
[deps] | ||
Libdl = "8f399da3-3557-5675-b5ff-fb832c97cbdb" | ||
|
||
[extra] |
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.
s
name = "MozillaCACerts_jll" | ||
uuid = "14a3606d-f60d-562e-9121-12d972cd8159" | ||
|
||
[extra] |
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.
Same here
Where is this thrown from? |
I've already compiled an older commit, so I cannot easily go back to see :( |
This PR adds a Downloads stdlib and replaces the
Base.download
function which relies on a miscellany of external programs (curl
,wget
,PowerShell
) which may or may not happen to be present in order to download files, withDownloads.download
which useslibcurl
to download files in the same way everywhere. In the process, we also gain support for passing headers and more (to be exposed and used in Pkg in future PRs).