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

Replaced downloading SDL2 from libsdl.org with git submodule SDL release 2.0.16 #1150

Merged
merged 4 commits into from
Oct 10, 2021
Merged

Conversation

OlegOAndreev
Copy link
Contributor

@OlegOAndreev OlegOAndreev commented Sep 17, 2021

Fixes #1148. I took the liberty of updating SDL2 to release-2.0.16 along the way. I've also done a minor cleanup of sdl2-sys/build.rs.

Tested on Windows 10 + MSVC 19, macOS 11.6 and Ubuntu Focal.

UPDATE: I have also fixed a couple of static link issues on Android: added required rustc-link-lib= directives and copy all libraries even for static-link builds (SDL2 builds their libhidapi.so shim for Android even in static mode).

…odule SDL

with tag release-2.0.16. Removed the old patches. Simplified the build script.
Added support for custom SDL2 toolchain and build profile. Removed bundled
include files.
@Cobrand
Copy link
Member

Cobrand commented Oct 5, 2021

I think it looks fine, but reviewing your code I did indeed find the .patch fields being removed. Do you know if they have been fixed upstream? We might want to add one way or another to add custom patches. 🤔

@OlegOAndreev
Copy link
Contributor Author

For one, it looks like none of the patches were actually applied, as they were created for the different SDL versions (the chosen version was 2.0.14, while the patches were for 2.0.12):
// Only apply patches whose file name is prefixed with the currently // targeted version of SDL2.

Yeah, I'm ready to put the patching infrastructure back, just do not know the best way to do it. Maybe we should copy source files to a separate directory inside OUT_DIR during build and patch there? The other way would be maintaining an SDL2 fork in Rust-SDL2 account, however this may be too much hassle? What would you prefer?

@OlegOAndreev
Copy link
Contributor Author

To clarify a bit on patches: the only patch remaining in build.rs was SDL2-2.0.12-vs2019-intrinsics.patch and it is included in SDL-2.0.16: https://github.com/libsdl-org/SDL/blob/release-2.0.16/src/stdlib/SDL_stdlib.c

@Cobrand
Copy link
Member

Cobrand commented Oct 10, 2021

Then perhaps the patches are not needed anymore. What we could do maybe if patches are required in the future, is to fork SDL, apply our patches, and use that as a submodule? Do you think this would be a good idea?

@OlegOAndreev
Copy link
Contributor Author

Yes, I believe potential SDL fork is one of the strong points for submodule approach.

@OlegOAndreev
Copy link
Contributor Author

I'm extremely sorry, I consistenly forget to run cargo fmt after changing files =(

@Cobrand
Copy link
Member

Cobrand commented Oct 10, 2021

No worries, thanks for the PR!

@Cobrand Cobrand merged commit bbb84d3 into Rust-SDL2:master Oct 10, 2021
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.

Use git submodule for bundled SDL2
2 participants