-
Notifications
You must be signed in to change notification settings - Fork 13
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 BinDeps2 to install binaries #48
Conversation
Let's see if this works
|
||
@write_deps_file libRmath | ||
else | ||
error("Your platform $(Sys.MACHINE) is not supported by this package!") |
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.
leave a fallback
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.
What kind of fallback did you have in mind?
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.
Presumably some way to build from source for folks on systems which don't have BinaryBuilder support yet, such as FreeBSD.
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.
I think the answer to that is to add FreeBSD support to BinaryBuilder.
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.
See JuliaPackaging/BinaryBuilder.jl#32. If that's the solution then we shouldn't pull the trigger on this until that's implemented. But even still, there are also systems such as Alpine, which support Julia but will similarly be left out in the cold without a fallback.
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.
I can add a fallback for now, but BinDeps will not be maintained anymore once we switch a good fraction of the ecosystem over to the new stuff.
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.
Then that's a pretty major hole in the design and plan for BinaryProvider.
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.
As I said, I'm happy to have the fallback until we add sufficient support to BinaryBuilder to add the two remaining platforms that julia supports (musl/Linux, FreeBSD), but BinaryBuilder does not. We've been through this. If you want support for your platform, go through BinaryBuilder.
BinaryProvider needs a small update and a tag, but other than that, this seems to be working. Yay! |
|
||
# Listing of files generated by BinaryBuilder: | ||
download_info = Dict( | ||
BinaryProvider.Linux(:aarch64, :glibc) => ("$bin_prefix/rmath.aarch64-linux-gnu.tar.gz", "0e56d5c5472ef832f76949e863958676c272eeaa2405b3239930e1e0b8d6313d"), |
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.
My remark it completely unrelated to this PR, but I wonder why one needs to list manually files for all architectures. Wouldn't it be possible for BinaryBuilder to automatically generate a file with this list (and checksums), and download that file to get this information? Then only one URL/checksum pair would have to be listed here, and platform details wouldn't pollute the code.
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.
The file does get generated and uploaded (look at the release artifacts here: https://github.com/JuliaStats/Rmath-julia/releases). We could certainly just download and execute that, though I do like this because it's very explicit. I think it's fine to go with this style for now and maybe revisit it once we have some more experience.
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.
OK, great. I wasn't too concerned about this package, but more about less experienced package authors who might be scared by all the platform-specific code.
Superseded by #55 |
Let's see if this works