Skip to content
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 binaries from JuliaBinaryWrappers #5

Closed
wants to merge 2 commits into from

Conversation

giordano
Copy link

@giordano giordano commented Oct 7, 2019

@NHDaly
Copy link
Owner

NHDaly commented Oct 9, 2019

Thanks Mosè!

Question: will this only work in Julia 1.3 though? I didn't understand that part of the readme

@giordano
Copy link
Author

giordano commented Oct 9, 2019

It's only the new artifacts framework that requires julia 1.3, as this is baked in the Pkg shipped in that version: https://julialang.github.io/Pkg.jl/dev/artifacts/. The plain tarballs "manually" installed can work with any version of julia

@NHDaly
Copy link
Owner

NHDaly commented Oct 10, 2019

Thanks! I'm learning about all this stuff for the first time. :)

The build is failing because this PR renames the library variable in julia from libSDL2 to libsdl2. Is there any reason you used the lowercase name? Is that how it's named in the BinaryBuilder used in Yggdrasil?

It seems like the capitals make sense, because that's how it's installed by homebrew and apt-get i think?

@giordano
Copy link
Author

giordano commented Oct 10, 2019

The build is failing because this PR renames the library variable in julia from libSDL2 to libsdl2. Is there any reason you used the lowercase name? Is that how it's named in the BinaryBuilder used in Yggdrasil?

Strictly speaking, line 9 of build.jl could remain the same and there would be no change, as it's this line that dictates what's written to deps.jl. However, we're trying (not always successfully) to keep consistency about naming, and having all libraries lowercase. If and when this package will switch to the new artifacts framework, the name of the variable must match whatever it is in Yggdrasil. You can suggest renaming the variable in Yggdrasil if you think it's makes sense (and in this case it could do, as the name of the library itself has unusually a lowercase bit).

Just let me know what you prefer 🙂

@NHDaly
Copy link
Owner

NHDaly commented Oct 10, 2019

I see. I think either one could make sense, since via homebrew it installs as libSDL2.dylib:
~/.julia/packages/Homebrew/s09IX/deps/usr/lib/libSDL2.dylib

But their website is libsdl.org and, as you say, you're trying to keep a convention of lowercase lib names. @jonathanBieler, do you have any preference?

@jonathanBieler
Copy link

I don't have any preference no, I would go with the Yggdrasil convention then.

@giordano
Copy link
Author

Ok, I'm going to change libSDL2 -> libsdl2 then

@NHDaly
Copy link
Owner

NHDaly commented Oct 10, 2019

Thanks Mosè! Let me know if you need any help on our end making that change. Thanks again for intervening here! :)

Also, before we can drop BinDeps, we're going to need to do BinaryBuilder for libsdl2_ttf and libsdl2_mixer (and ideally also SDL_image though we didn't have that in the previous builder). Should we add those via PRs to Yggdrasil, or make our own custom BinaryProvider repos?

@@ -13,6 +13,11 @@ module SimpleDirectMediaLayer
error("SimpleDirectMediaLayer not properly installed. Please run Pkg.build(\"SimpleDirectMediaLayer\") then restart Julia.")
end

# Deprecate old library variables
Base.@deprecate_binding libSDL2 SimpleDirectMediaLayer.libsdl2 false
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

<3 woah, this is awesome. Thanks, you're the best!

@giordano
Copy link
Author

Thanks Mosè! Let me know if you need any help on our end making that change. Thanks again for intervening here! :)

I think I've already done 🙂

Also, before we can drop BinDeps, we're going to need to do BinaryBuilder for libsdl2_ttf and libsdl2_mixer (and ideally also SDL_image though we didn't have that in the previous builder).

Sure, it's better to have everything built together.

Should we add those via PRs to Yggdrasil, or make our own custom BinaryProvider repos?

I already started attacking libsdl2_ttf, but it's failing for Windows: JuliaPackaging/Yggdrasil#123. libsdl2_mixer requires additional dependencies. libvorbis is in JuliaPackaging/Yggdrasil#88

@NHDaly
Copy link
Owner

NHDaly commented Oct 10, 2019

:) haha awesome, thanks!!! :)

I already started attacking libsdl2_ttf, but it's failing for Windows: JuliaPackaging/Yggdrasil#123. libsdl2_mixer requires additional dependencies. libvorbis is in JuliaPackaging/Yggdrasil#88

Regarding the windows failure, I remember we had issues with building libsdl2_ttf in the past as well. Hopefully this might help?:
JuliaMultimedia#2 (comment)

I can't remember if this is all the documentation we have on this. Maybe @jonathanBieler will know more. :)

@aviks
Copy link

aviks commented Jan 26, 2020

This can now be closed.

@giordano giordano closed this Jan 26, 2020
@giordano giordano deleted the sdl2-yggdrasil branch January 26, 2020 17:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants