Build fails on linux with 'error: ‘SIO_DEVANY’ undeclared'

[bgourlie]

This is causing my travis-ci build to fail, and occurs when building with features bundled and static-link (full build output here). SDL fixed this issue here, but the version that rust-sdl2 depends on doesn't include the fix.

I have a branch that upgrades SDL to 2.0.8 here, but my build now fails with a different error, which I'm not sure is a regression relative to the original issue or a followup issue that needs addressing:

Running `rustc --crate-name sdl2_sys /home/travis/.cargo/git/checkouts/rust-sdl2-f1d5a308c7a9c4c5/e998692/sdl2-sys/src/lib.rs --crate-type lib --emit=dep-info,link -C debuginfo=2 --cfg 'feature="bundled"' --cfg 'feature="cmake"' --cfg 'feature="default"' --cfg 'feature="flate2"' --cfg 'feature="reqwest"' --cfg 'feature="static-link"' --cfg 'feature="tar"' -C metadata=cd65aa7aed7740a2 -C extra-filename=-cd65aa7aed7740a2 --out-dir /home/travis/build/bgourlie/rs-nes/target/debug/deps -L dependency=/home/travis/build/bgourlie/rs-nes/target/debug/deps --cap-lints allow -L /home/travis/build/bgourlie/rs-nes/target/debug/build/sdl2-sys-8b6681544607ef39/out/lib -l static=SDL2main -l static=SDL2 -l sndio`
error: could not find native static library `SDL2main`, perhaps an -L flag is missing?

You can find the full build output here

[Cobrand]

I have looked into it, and for your second problem it looks like SDL 2.0.8 outputs "libSDL2maind.a" and "libSDL2d.a" instead of "libSDL2main.a" and "libSDL2.a" like it used to.

I have a branch with your changes and #765 as well, will post it soon, would you mind having a look at it then?

[Cobrand]

@bgourlie Can you check this branch and tell me if it works? https://github.com/Rust-SDL2/rust-sdl2/tree/unstable-build-rs-work

If possible, try to test it on as many platforms as you can, I only have a linux handy personally.

[bgourlie]

I'll do some testing on Windows and OS X and let you know what I find. I had a linux partition on my desktop as well but it appears to be borked. Thanks for looking into this!

[bgourlie]

Tested on Window 10 and OS X. Examples run, and there is only a single test regression on both platforms:

---- audio::test::test_audio_cvt stdout ----
thread 'audio::test::test_audio_cvt' panicked at 'assertion failed: `(left == right)`
  left: `2040`,
 right: `510`', src/sdl2\audio.rs:830:9

[Cobrand]

I've actually got the same error with cargo test on linux. Two possibilities:

• Somehow the way to generate bindings went wrong somewhere, meaning that the SDL2 C API calls we have not are not the same as the SDL2 C API calls we used to do. This is a bug on our side.
• The behavior changed for this test in 2.0.8, in that case we need to update the test.

[bgourlie]

One more thing: I merged your changes into my branch and added another commit. It turns out we still need to link libSDL2main.a, etc in release mode, and the d variant otherwise.

[Cobrand]

So the "d" stands for debug! I see. Maybe it would be best to compile directly sdl2 in release mode everytime with the "bundled" feature. It doesn't really make sense to compile in debug mode SDL2 when we are in debug mode ourselves, does it?

[bgourlie]

To be honest, I'm not sure! I'm relatively new to the world of compiled languages, and am unfamiliar with release/debug build conventions. What you suggest seems totally reasonable. I'll update my branch to always build SDL in release mode.

[reynisdrangar]

Would y'all mind popping open a PR for this? I'm pinned to the fork @bgourlie put up to keep travis happy, and I'd like to note the PR number in a comment next to the pin so I can come back and clean it up later.

Copypasta friendly if anybody else is using bundled builds:

# Override sdl2 to get 2.0.8 (fixes libsndio missing define in travis) rust-sdl2#777
[patch.crates-io]
sdl2 = { git = 'https://github.com/bgourlie/rust-sdl2', branch="sdl-upgrade" }

[reynisdrangar]

Related thought — @Cobrand how would you feel about your travis builds using the bundled feature, either in addition to or instead of manually pulling and building sdl2.0.5 from upstream?

If you're amenable, I'll make a PR for that.

[Cobrand]

Related thought — @Cobrand how would you feel about your travis builds using the bundled feature, either in addition to or instead of manually pulling and building sdl2.0.5 from upstream?

I think that was actually planned at first -- now I can't really say if there is a good reason we didn't do that, or just we didn't have the time do change travis to do that. Please, feel free to write a PR! They are always welcome.

For this ticket, we're so close from merging this, we just have to figure out why test_audio_cvt fails now with the new build. I'll try to give it another look soon.

[reynisdrangar]

#781 adds a travis matrix dimension on environment variables and promotes feature flags to that, so that ought to take care of CI for this use case in the future.

I left static-link off for now since it's broken in trunk (hence this issue) but it's easy enough to add once this is fixed.

[Cobrand]

After some testing I can confirm that this test that fails:

---- audio::test::test_audio_cvt stdout ----
thread 'audio::test::test_audio_cvt' panicked at 'assertion failed: `(left == right)`
  left: `2040`,
 right: `510`', src/sdl2\audio.rs:830:9

is caused by upgrading from 2.0.5 to 2.0.6 or higher (in our case 2.0.8). I'm trying to figure out if this is a bug, if the test needs some changes or if the bindings need some changes themselves now.

It has been tested with the master branch, by changing the bundled version from 2.0.5 (which works) to 2.0.6 (which fails). Note that the SDL2.so libraries on my system were messing with dynamic linking, so I had to resort to static linking, but the result should be the same.

[Cobrand]

The implementation of the source definitely changed... I think I've got an idea to fix this test, however there is still something that surprises me with the converted audio:

#[cfg(test)]
mod test {
    use super::{AudioCVT, AudioFormat};

    #[test]
    fn test_audio_cvt() {
        use std::iter::repeat;

        // 0,1,2,3, ...
        let buffer: Vec<u8> = (0..255).collect();

        // 0,0,1,1,2,2,3,3, ...
        let new_buffer_expected: Vec<u8> = (0..255).flat_map(|v| repeat(v).take(2)).collect();

        let cvt = AudioCVT::new(AudioFormat::U8, 1, 44100, AudioFormat::U8, 2, 44100).unwrap();
        assert!(cvt.is_conversion_needed());
        // assert_eq!(cvt.capacity(255), 255*2);

        let new_buffer = cvt.convert(buffer);
        assert_eq!(new_buffer.len(), new_buffer_expected.len());
        assert_eq!(new_buffer[0..256], new_buffer_expected[0..256]);

The test above fails with:

left: `[0, 0, 1, 1, 2, 2, 3, 3, 4, 4, 5, 5, 6, 6, 7, 7, 8, 8, 9, 9, 10, 10, 11, 11, 12, 12, 13, 13, 14, 14, 15, 15, 16, 16, 17, 17, 18, 18, 19, 19, 20, 20, 21, 21, 22, 22, 23, 23, 24, 24, 25, 25, 26, 26, 27, 27, 28, 28, 29, 29, 30, 30, 31, 31, 32, 32, 33, 33, 34, 34, 35, 35, 36, 36, 37, 37, 38, 38, 39, 39, 40, 40, 41, 41, 42, 42, 43, 43, 44, 44, 45, 45, 46, 46, 47, 47, 48, 48, 49, 49, 50, 50, 51, 51, 52, 52, 53, 53, 54, 54, 55, 55, 56, 56, 57, 57, 58, 58, 59, 59, 60, 60, 61, 61, 62, 62, 63, 63, 64, 64, 64, 64, 65, 65, 66, 66, 67, 67, 68, 68, 69, 69, 70, 70, 71, 71, 72, 72, 73, 73, 74, 74, 75, 75, 76, 76, 77, 77, 78, 78, 79, 79, 80, 80, 81, 81, 82, 82, 83, 83, 84, 84, 85, 85, 86, 86, 87, 87, 88, 88, 89, 89, 90, 90, 91, 91, 92, 92, 93, 93, 94, 94, 95, 95, 96, 96, 97, 97, 98, 98, 99, 99, 100, 100, 101, 101, 102, 102, 103, 103, 104, 104, 105, 105, 106, 106, 107, 107, 108, 108, 109, 109, 110, 110, 111, 111, 112, 112, 113, 113, 114, 114, 115, 115, 116, 116, 117, 117, 118, 118, 119, 119, 120, 120, 121, 121, 122, 122, 123, 123, 124, 124, 125, 125, 126, 126]`,
 right: `[0, 0, 1, 1, 2, 2, 3, 3, 4, 4, 5, 5, 6, 6, 7, 7, 8, 8, 9, 9, 10, 10, 11, 11, 12, 12, 13, 13, 14, 14, 15, 15, 16, 16, 17, 17, 18, 18, 19, 19, 20, 20, 21, 21, 22, 22, 23, 23, 24, 24, 25, 25, 26, 26, 27, 27, 28, 28, 29, 29, 30, 30, 31, 31, 32, 32, 33, 33, 34, 34, 35, 35, 36, 36, 37, 37, 38, 38, 39, 39, 40, 40, 41, 41, 42, 42, 43, 43, 44, 44, 45, 45, 46, 46, 47, 47, 48, 48, 49, 49, 50, 50, 51, 51, 52, 52, 53, 53, 54, 54, 55, 55, 56, 56, 57, 57, 58, 58, 59, 59, 60, 60, 61, 61, 62, 62, 63, 63, 64, 64, 65, 65, 66, 66, 67, 67, 68, 68, 69, 69, 70, 70, 71, 71, 72, 72, 73, 73, 74, 74, 75, 75, 76, 76, 77, 77, 78, 78, 79, 79, 80, 80, 81, 81, 82, 82, 83, 83, 84, 84, 85, 85, 86, 86, 87, 87, 88, 88, 89, 89, 90, 90, 91, 91, 92, 92, 93, 93, 94, 94, 95, 95, 96, 96, 97, 97, 98, 98, 99, 99, 100, 100, 101, 101, 102, 102, 103, 103, 104, 104, 105, 105, 106, 106, 107, 107, 108, 108, 109, 109, 110, 110, 111, 111, 112, 112, 113, 113, 114, 114, 115, 115, 116, 116, 117, 117, 118, 118, 119, 119, 120, 120, 121, 121, 122, 122, 123, 123, 124, 124, 125, 125, 126, 126, 127, 127]`

(while it succeeded before). AudioConvert now outputs 4 times "64" instead of 2 times, but I don't see any reason why it does that. As a result the last two 127, 127 are truncated while they shoudn't be. I tested with 48000 as a frequency to make sure this wasn't that. I will try to post this "bug" upstream, but in the meantime be aware that AudioCVT may not work very well. You may want to use something like https://crates.io/crates/sample to help you convert audio if you're not confident SDL2 does a good job with it.

[Cobrand]

I posted this weird behavior upstream, I hope we get an answer soon! https://discourse.libsdl.org/t/change-of-behavior-in-audiocvt-sdl-convertaudio-from-2-0-5-to-2-0-6/24682 I'll try to fix the test suite in the meantime.