"bundled" in v0.34.3 is broken on Windows

[Eric-Arz]

Compilation of the crate fails with the following error:

failed to run custom build command for sdl2-sys v0.34.3

Explicity requesting v0.3.42 for sdl2-sys fixes the issue:

[dependencies.sdl2-sys]
version = "=0.34.2"

Everything then compiles & runs as it should.

[Cobrand]

If you can add details on what version of rust you use, what compiler (msvc or gnu) and more importantly what the complete error message is, I'm sure it will be helpful.

[Eric-Arz]

If you can add details on what version of rust you use, what compiler (msvc or gnu) and more importantly what the complete error message is, I'm sure it will be helpful.

I am using:

stable-x86_64-pc-windows-msvc (default)
rustc 1.43.0 (4fb7144ed 2020-04-20)

Full error message:

error: failed to run custom build command for `sdl2-sys v0.34.3`                                                          

Caused by:
  process didn't exit successfully: `C:\Users\username\Desktop\VSCode-Rust\target\debug\build\sdl2-sys-5c0ed5eddf780972\build-script-build` (exit code: 101)
--- stderr
CMake Warning:
  Manually-specified variables were not used by the project:

    CMAKE_ASM_FLAGS
    CMAKE_ASM_FLAGS_RELEASE


thread 'main' panicked at '
command did not execute successfully, got: exit code: 1

[Cobrand]

We don't know which command is failing so that will need a bit more debugging.

I don't have windows so either you'll have to wait until someone fixes that for you, or you'll have to fix that yourself I'm afraid.

I think you should try running the CMake file directly without the build.rs file, maybe that will give you a clue.

[aclysma]

I looked at it a little the other night and it fails here:

        for modified_file in patch_set.modified_files() {
            use std::io::{Write, BufRead};

            let file_path = sdl2_source_path.join(modified_file.path());
            let old_path = sdl2_source_path.join(format!("{}_old", modified_file.path()));
>>          fs::rename(&file_path, &old_path)
                .expect(&format!(
                    "Rename of {} to {} failed",
                    file_path.to_string_lossy(),
                    old_path.to_string_lossy()));

The filename, directory name, or volume label syntax is incorrect. (os error 123) on the fs::rename call

format!("{}_old" might should be old_{} but that doesn't fix it. The odd thing is, I made a throw-away program that called fs::rename on the same paths and it worked fine. Really seems like it would be easy to fix but I didn't figure out a working solution in the short amount of time I messed with it.

[ElhamAryanpur]

no fixes yet, still doesn't work on latest windows. mine is latest rust, I also have gcc 9.2 installed.

[aclysma]

For anyone looking for a quick workaround, you can use 0.34.2 by running this command:
cargo update --package sdl2-sys --precise 0.34.2

EDIT: sdl2-sys, not sdl2

[therocode]

The error I'm getting is that it seems like vcpkg is complaining about a casing difference between release and Release.

Vcpkg is unable to link because we cannot decide between Release and Debug libraries. Please define the property VcpkgConfiguration to be 'Release' or 'Debug' (currently 'release').

I assume from here -> https://github.com/Rust-SDL2/rust-sdl2/blob/master/sdl2-sys/build.rs#L273

?

[uhuntu]

I was switching to 0.34.3 at neovide on Windows, it was working very well, but I was using msvc.

[uhuntu]

Can some one hint me how to reproduce the issue ?

[Lokathor]

Hello everyone.

You're getting hit by this bug. It's the fault of SDL2 itself.

You can apply the patch there to the bundled copy of 2.0.12, or you can just wait a bit and it should be fixed in 2.0.14 "fairly soon".

Also, the reason that not everyone can reproduce it is because it only affects building the lib using VS2019. If you're using VS2017 to build you won't see the bug.

[uhuntu]

Really ? I just use VS2019

[Lokathor]

Are you saying you use VS2019 and do not see the bug?

Oh, also you have to be building for dynamic linking for the bug to happen. A static link build can avoid the issue.

[uhuntu]

@Lokathor I guess you are right, static link should not have issue.

[LunarLambda]

I looked at it a little the other night and it fails here:

        for modified_file in patch_set.modified_files() {
            use std::io::{Write, BufRead};

            let file_path = sdl2_source_path.join(modified_file.path());
            let old_path = sdl2_source_path.join(format!("{}_old", modified_file.path()));
>>          fs::rename(&file_path, &old_path)
                .expect(&format!(
                    "Rename of {} to {} failed",
                    file_path.to_string_lossy(),
                    old_path.to_string_lossy()));

The filename, directory name, or volume label syntax is incorrect. (os error 123) on the fs::rename call

format!("{}_old" might should be old_{} but that doesn't fix it. The odd thing is, I made a throw-away program that called fs::rename on the same paths and it worked fine. Really seems like it would be easy to fix but I didn't figure out a working solution in the short amount of time I messed with it.

AFAICT this is because modified_file.path() returns something like a/src/blah/file.c which is a weird quirk of diff formats

So it tries to rename a file in a non-existent sub-directory a

EDIT: This is odd behaviour on unidiff's part, as it only emits the a/ prefix if the patch doesn't obey a certain format: Source

[nylanderdev]

AFAICT this is because modified_file.path() returns something like a/src/blah/file.c which is a weird quirk of diff formats

So it tries to rename a file in a non-existent sub-directory a

EDIT: This is odd behaviour on unidiff's part, as it only emits the a/ prefix if the patch doesn't obey a certain format: Source

I seem to have a different, but similar issue when building. On my machine, the PatchSet::parse function doesn't properly handle \r characters in the \r\n line endings of the patch details (don't know what they're called), so at one point I end up with a path looking something like "CMakeLists.txt\r" (since the patch details contain the substring [...]"CMakeLists.txt\r\n"[...]).

[uhuntu]

https://www.libsdl.org/download-2.0.php, looks like SDL version 2.0.14 (stable) had been released, probably we can try to achieve 0.34.4 now ?

FYR: https://bugzilla.libsdl.org/show_bug.cgi?id=5112
665b9c8#diff-86201b3a6ce8e9c38643b7cceca385b9331f33b6066a8f9d45dc6a9534ee3146

[uhuntu]

363c764#diff-86201b3a6ce8e9c38643b7cceca385b9331f33b6066a8f9d45dc6a9534ee3146

@aclysma

[LunarLambda]

https://www.libsdl.org/download-2.0.php, looks like SDL version 2.0.14 (stable) had been released, probably we can try to achieve 0.34.4 now ?

FYR: https://bugzilla.libsdl.org/show_bug.cgi?id=5112
665b9c8#diff-86201b3a6ce8e9c38643b7cceca385b9331f33b6066a8f9d45dc6a9534ee3146

Certainly we should try, I'm still waiting for feedback from @Cobrand at this point

[LunarLambda]

AFAICT this is because modified_file.path() returns something like a/src/blah/file.c which is a weird quirk of diff formats
So it tries to rename a file in a non-existent sub-directory a
EDIT: This is odd behaviour on unidiff's part, as it only emits the a/ prefix if the patch doesn't obey a certain format: Source

I seem to have a different, but similar issue when building. On my machine, the PatchSet::parse function doesn't properly handle \r characters in the \r\n line endings of the patch details (don't know what they're called), so at one point I end up with a path looking something like "CMakeLists.txt\r" (since the patch details contain the substring [...]"CMakeLists.txt\r\n"[...]).

Interesting, maybe that's why patching kept failing. If people have "check out as CRLF, commit as LF" checked on their windows git install, maybe unidiff is choking on that. That's something to be reported to them

[Cobrand]

@LunarLambda my bad, I had totally forgotten about that, end of year holidays and all. I'll answer in the appropriate thread. ( mostly #1059 )

[nylanderdev]

AFAICT this is because modified_file.path() returns something like a/src/blah/file.c which is a weird quirk of diff formats
So it tries to rename a file in a non-existent sub-directory a
EDIT: This is odd behaviour on unidiff's part, as it only emits the a/ prefix if the patch doesn't obey a certain format: Source

I seem to have a different, but similar issue when building. On my machine, the PatchSet::parse function doesn't properly handle \r characters in the \r\n line endings of the patch details (don't know what they're called), so at one point I end up with a path looking something like "CMakeLists.txt\r" (since the patch details contain the substring [...]"CMakeLists.txt\r\n"[...]).

Interesting, maybe that's why patching kept failing. If people have "check out as CRLF, commit as LF" checked on their windows git install, maybe unidiff is choking on that. That's something to be reported to them

@LunarLambda The unidiff dev(s) seem to have fixed this on git (presumably in this commit), though it doesn't seem to be on cargo yet. When using unidiff from git I am able to build successfully. Though I have yet to try actually using the compiled library.

EDIT: It seems that the "\r" issue only affects me when trying to build rust-sdl2 myself and not when using it as a cargo dependency. Though it seems unrelated to the issue's author's problem. I get the same issue as the author when I use "bundled" without "static-link".

[Eric-Arz]

Over 6 months later, this is still broken. Is there no one that cares about this getting fixed?

[nylanderdev]

If it's of any use to anyone: I never fail if I use the bundled feature together with the static-link one, for whatever reason. Is there something weird with the library binaries or something?