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

Support "bundled", "bindgen" and opt-in static linking #720

Merged
merged 8 commits into from
Nov 8, 2017

Conversation

Cobrand
Copy link
Member

@Cobrand Cobrand commented Oct 31, 2017

This is a follow up on the unmerged PR693.

The build.rs file in sdl2-sys has been totally overhauled. The "bindgen" feature is now smart enough o use the headers directly from the source if "bundled" is specified. Likewise, the library is compiled dynamically or statically (and linked as well), whether or not the feature "static-link" is enabled.

  • Still needs documentation in readme as of what these do
  • Needs testing across all platforms, with at least the following features:
    • vanilla
    • "bundled"
    • "static-link"
    • "bundled,static-link"
    • "static-link,pkg-config" (if possible)
    • "pkg-config" (if possible)
    • "bindgen" (if possible)
    • "bindgen,pkg-config" (if possible)

Currently been tested on:

  • Linux by @Cobrand
  • macOS by @michaelfairley
  • Windows (msvc) (everything but bindgen has been tested, bindgen crashes, see below)
  • Windows (gnu, requires proper static parameters)
  • iOS (not deemed possible, see later inthis thread)

PS: If someone has a clue of what's happening with travis, I'd be glad.

Allows statically linking to a self-contained build of SDL2.
#[cfg(feature = "static-link")]
fn get_pkg_config() -> pkg_config::Library {
pkg_config::Config::new()
.statik(false)
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems backwards (as does the non-static version a few lines down from here).

Copy link
Member Author

Choose a reason for hiding this comment

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

Oops, good catch! I don't think it affects anything though, because it still worked for me with this on linux.

Copy link
Contributor

Choose a reason for hiding this comment

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

It'll still build, but you pass static-link it'll build a dynamic version, and without static-link it'll build a static version.

@michaelfairley
Copy link
Contributor

I was able to build each of those feature combos on Mac. 👍

@Cobrand
Copy link
Member Author

Cobrand commented Nov 2, 2017

Thanks for the help! Do you think you will be able to test that for iOS as well?

@michaelfairley
Copy link
Contributor

Sooo... iOS is going not work with most of these features:

  • bundled: this uses SDL2's still-experimental CMake build, which only supports mac/win/linux.
  • static-link: SDL2 for iOS only builds (at least by default) as a static library in the first place, so it's always statically linked. iOS builds probably shouldn't be affected by whether or not this feature is enabled.
  • use-pkgconfig: pkg-config and iOS aren't really things that go together.
  • use-bindgen: I expected this to work, but it doesn't. (It blows up somewhere inside of bindgen, and I don't have enough experience with bindgen to adequately debug right now).

@Cobrand
Copy link
Member Author

Cobrand commented Nov 2, 2017

I at least expected it to work with bindgen as well, would you mind giving us the error you got? Maybe it can be useful in the future or someone can have a look at it and find a solution.

@michaelfairley
Copy link
Contributor

   Compiling libc v0.2.27
   Compiling lazy_static v0.2.8
   Compiling bitflags v0.7.0
   Compiling num-traits v0.1.40
   Compiling sdl2-sys v0.31.0 (file:///Users/michaelfairley/os/rust-sdl2/sdl2-sys)
   Compiling rand v0.3.15
   Compiling num-integer v0.1.35
   Compiling num-iter v0.1.34
   Compiling num v0.1.40
error: failed to run custom build command for `sdl2-sys v0.31.0 (file:///Users/michaelfairley/os/rust-sdl2/sdl2-sys)`
process didn't exit successfully: `/Users/michaelfairley/os/rust-sdl2/target/debug/build/sdl2-sys-c54ba0bd0ba29887/build-script-build` (exit code: 101)
--- stderr
thread 'main' panicked at 'TranslationUnit::parse failed', src/libcore/option.rs:819:4
stack backtrace:
   0: std::sys::imp::backtrace::tracing::imp::unwind_backtrace
   1: std::panicking::default_hook::{{closure}}
   2: std::panicking::default_hook
   3: std::panicking::rust_panic_with_hook
   4: std::panicking::begin_panic
   5: std::panicking::begin_panic_fmt
   6: rust_begin_unwind
   7: core::panicking::panic_fmt
   8: core::option::expect_failed
   9: <core::option::Option<T>>::expect
  10: bindgen::ir::context::BindgenContext::new
  11: bindgen::Bindings::generate
  12: bindgen::Builder::generate
  13: build_script_build::generate_bindings
  14: build_script_build::main
  15: __rust_maybe_catch_panic
  16: std::rt::lang_start
  17: main

@Cobrand
Copy link
Member Author

Cobrand commented Nov 4, 2017

Got this error when trying to use bindgen on msvc:

error[E0277]: the trait bound `[u8; 40]: std::fmt::Debug` is not satisfied
     --> D:\Code\rust-sdl2\target\debug\build\sdl2-sys-74769dd41c412ff6\out/bindings.rs:12272:1545
      |
12272 | } # [ repr ( C ) ] # [ derive ( Debug , Copy , Clone ) ] pub struct tagMONITORINFO { pub cbSize : DWORD , pub rcMonitor : RECT , pub rcWork : RECT , pub dwFlags : DWORD , } # [ test ] fn bindgen_test_layout_tagMONITORINF
O ( ) { assert_eq ! ( :: std :: mem :: size_of :: < tagMONITORINFO > ( ) , 40usize , concat ! ( "Size of: " , stringify ! ( tagMONITORINFO ) ) ) ; assert_eq ! ( :: std :: mem :: align_of :: < tagMONITORINFO > ( ) , 4usize , conc
at ! ( "Alignment of " , stringify ! ( tagMONITORINFO ) ) ) ; assert_eq ! ( unsafe { & ( * ( 0 as * const tagMONITORINFO ) ) . cbSize as * const _ as usize } , 0usize , concat ! ( "Alignment of field: " , stringify ! ( tagMONITO
RINFO ) , "::" , stringify ! ( cbSize ) ) ) ; assert_eq ! ( unsafe { & ( * ( 0 as * const tagMONITORINFO ) ) . rcMonitor as * const _ as usize } , 4usize , concat ! ( "Alignment of field: " , stringify ! ( tagMONITORINFO ) , "::
" , stringify ! ( rcMonitor ) ) ) ; assert_eq ! ( unsafe { & ( * ( 0 as * const tagMONITORINFO ) ) . rcWork as * const _ as usize } , 20usize , concat ! ( "Alignment of field: " , stringify ! ( tagMONITORINFO ) , "::" , stringif
y ! ( rcWork ) ) ) ; assert_eq ! ( unsafe { & ( * ( 0 as * const tagMONITORINFO ) ) . dwFlags as * const _ as usize } , 36usize , concat ! ( "Alignment of field: " , stringify ! ( tagMONITORINFO ) , "::" , stringify ! ( dwFlags
) ) ) ; } pub type MONITORINFO = tagMONITORINFO ; pub type LPMONITORINFO = * mut tagMONITORINFO ; # [ repr ( C ) ] # [ derive ( Debug , Copy , Clone ) ] pub struct tagMONITORINFOEXA { pub __bindgen_padding_0 : [ u8 ; 40usize ] ,
 pub szDevice : [ CHAR ; 32usize ] , pub __bindgen_align : [ u32 ; 0usize ] , } # [ test ] fn bindgen_test_layout_tagMONITORINFOEXA ( ) { assert_eq ! ( :: std :: mem :: size_of :: < tagMONITORINFOEXA > ( ) , 72usize , concat ! (
 "Size of: " , stringify ! ( tagMONITORINFOEXA ) ) ) ; assert_eq ! ( :: std :: mem :: align_of :: < tagMONITORINFOEXA > ( ) , 4usize , concat ! ( "Alignment of " , stringify ! ( tagMONITORINFOEXA ) ) ) ; assert_eq ! ( unsafe { &
 ( * ( 0 as * const tagMONITORINFOEXA ) ) . szDevice as * const _ as usize } , 40usize , concat ! ( "Alignment of field: " , stringify ! ( tagMONITORINFOEXA ) , "::" , stringify ! ( szDevice ) ) ) ; } pub type MONITORINFOEXA = t
agMONITORINFOEXA ; pub type LPMONITORINFOEXA = * mut tagMONITORINFOEXA ; # [ repr ( C ) ] # [ derive ( Debug , Copy , Clone ) ] pub struct tagMONITORINFOEXW { pub __bindgen_padding_0 : [ u16 ; 20usize ] , pub szDevice : [ WCHAR
; 32usize ] , pub __bindgen_align : [ u32 ; 0usize ] , } # [ test ] fn bindgen_test_layout_tagMONITORINFOEXW ( ) { assert_eq ! ( :: std :: mem :: size_of :: < tagMONITORINFOEXW > ( ) , 104usize , concat ! ( "Size of: " , stringi
fy ! ( tagMONITORINFOEXW ) ) ) ; assert_eq ! ( :: std :: mem :: align_of :: < tagMONITORINFOEXW > ( ) , 4usize , concat ! ( "Alignment of " , stringify ! ( tagMONITORINFOEXW ) ) ) ; assert_eq ! ( unsafe { & ( * ( 0 as * const ta
gMONITORINFOEXW ) ) . szDevice as * const _ as usize } , 40usize , concat ! ( "Alignment of field: " , stringify ! ( tagMONITORINFOEXW ) , "::" , stringify ! ( szDevice ) ) ) ; } pub type MONITORINFOEXW = tagMONITORINFOEXW ; pub
 type LPMONITORINFOEXW = * mut tagMONITORINFOEXW ; pub type MONITORINFOEX = MONITORINFOEXA ; pub type LPMONITORINFOEX = LPMONITORINFOEXA ; extern "C" {
      |





                                                                                                                                                                                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `
[u8; 40]` cannot be formatted using `:?`; if it is defined in your crate, add `#[derive(Debug)]` or manually implement it
      |
      = help: the trait `std::fmt::Debug` is not implemented for `[u8; 40]`
      = note: required because of the requirements on the impl of `std::fmt::Debug` for `&[u8; 40]`
      = note: required for the cast to the object type `std::fmt::Debug`

Looks like this is a bug with bindgen so I'm oing to drop support of this platform for bindgen only (everything else but pkg-config works). I'm still looking for someone to support using bindgen with windows-gnu. @dariost would you mind helping with that?

@icefoxen
Copy link
Contributor

icefoxen commented Nov 4, 2017

Testing on Windows 10 x64 using rustc 1.21 and MSVC.

  • vanilla: Runs
  • "bundled": Runs
  • "static-link": Runs
  • "bundled,static-link": Runs

pkg-config and bindgen features don't appear to exist on this platform.

All I did was cargo run --feature=foo --example renderer-yuv, so I wasn't trying to give it a super rigorous workout. rustc also already knows where to find the SDL2 dll's on my windows box, and I'm not sure whether or not that might give a false positive and make this work where it otherwise wouldn't.

@Cobrand
Copy link
Member Author

Cobrand commented Nov 4, 2017

I tested with msvc as well and bindgen can work if clang is installed, but even still it doesn't work because of the error that I had just before your post. i'm dropping bindgen support for msvc, the pregenerated bindings will have to do! (which they probably will).

@Cobrand
Copy link
Member Author

Cobrand commented Nov 8, 2017

I wanted support for windows-gnu but no one's up for the task, but I guess we can still merge for those who need these features on everything but windows-gnu.

Good work everyone!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants