From 9281179bb1675da807a95571fbcfef34563e2bec Mon Sep 17 00:00:00 2001 From: Mike Waychison Date: Sun, 18 Apr 2021 07:58:44 -0700 Subject: [PATCH 1/6] Add debug variant of "bundled" (try #2) This adds back the ability to compile the debug version of SDL2 when the cargo build is a debug build. Previous attempt (PR #1081) broke dynamic linking bundled builds on Windows, forgetting to copy the resulting DLL correctly and misspecifying the name of the library in that case. --- changelog.md | 2 ++ sdl2-sys/build.rs | 25 ++++++++++++++++++++----- 2 files changed, 22 insertions(+), 5 deletions(-) diff --git a/changelog.md b/changelog.md index 84a7dc637ea..16de6c2113c 100644 --- a/changelog.md +++ b/changelog.md @@ -3,6 +3,8 @@ when upgrading from a version of rust-sdl2 to another. ### Unreleased +* [PR #1092](https://github.com/Rust-SDL2/rust-sdl2/pull/1092) Add debug builds to "bundled", second attempt. + * Rollback PR #1081: Broke dynamic linking on Windows #1088 ### v0.34.4 diff --git a/sdl2-sys/build.rs b/sdl2-sys/build.rs index 8192d70c1d9..d9f99eef41f 100644 --- a/sdl2-sys/build.rs +++ b/sdl2-sys/build.rs @@ -293,7 +293,6 @@ fn patch_sdl2(sdl2_source_path: &Path) { #[cfg(feature = "bundled")] fn compile_sdl2(sdl2_build_path: &Path, target_os: &str) -> PathBuf { let mut cfg = cmake::Config::new(sdl2_build_path); - cfg.profile("release"); // Override __FLTUSED__ to keep the _fltused symbol from getting defined in the static build. // This conflicts and fails to link properly when building statically on Windows, likely due to @@ -367,6 +366,20 @@ fn compute_include_paths() -> Vec { include_paths } +/// There's no easy way to extract this suffix from `cmake::Config` so we have to emulate their +/// behaviour here (see the source for `cmake::Config::build`). +fn debug_postfix() -> &'static str { + match ( + &env::var("OPT_LEVEL").unwrap_or_default()[..], + &env::var("PROFILE").unwrap_or_default()[..], + ) { + ("1", _) | ("2", _) | ("3", _) | ("s", _) | ("z", _) => "", + ("0", _) => "d", + (_, "debug") => "d", + (_, _) => "", + } +} + fn link_sdl2(target_os: &str) { #[cfg(all(feature = "use-pkgconfig", not(feature = "bundled")))] { @@ -403,6 +416,8 @@ fn link_sdl2(target_os: &str) { if cfg!(feature = "bundled") || cfg!(not(feature = "use-pkgconfig")) { if cfg!(feature = "use_mac_framework") && target_os == "darwin" { println!("cargo:rustc-flags=-l framework=SDL2"); + } else if target_os.contains("windows") { + println!("cargo:rustc-flags=-l SDL2{}", debug_postfix()); } else if target_os != "emscripten" { println!("cargo:rustc-flags=-l SDL2"); } @@ -414,8 +429,8 @@ fn link_sdl2(target_os: &str) { if cfg!(feature = "bundled") || (cfg!(feature = "use-pkgconfig") == false && cfg!(feature = "use-vcpkg") == false) { - println!("cargo:rustc-link-lib=static=SDL2main"); - println!("cargo:rustc-link-lib=static=SDL2"); + println!("cargo:rustc-link-lib=static=SDL2main{}", debug_postfix()); + println!("cargo:rustc-link-lib=static=SDL2{}", debug_postfix()); } // Also linked to any required libraries for each supported platform @@ -552,9 +567,9 @@ fn copy_dynamic_libraries(sdl2_compiled_path: &PathBuf, target_os: &str) { // copy sdl2.dll out of its build tree and down to the top level cargo // binary output directory. if target_os.contains("windows") { - let sdl2_dll_name = "SDL2.dll"; + let sdl2_dll_name = format!("SDL2{}.dll", debug_postfix()); let sdl2_bin_path = sdl2_compiled_path.join("bin"); - let src_dll_path = sdl2_bin_path.join(sdl2_dll_name); + let src_dll_path = sdl2_bin_path.join(&sdl2_dll_name); // Copy the dll to: // * target dir: as a product ship product of the build, From 960d88ca9ca12056f1c19b3006cf1a0c1214233c Mon Sep 17 00:00:00 2001 From: Mike Waychison Date: Fri, 23 Apr 2021 08:50:29 -0700 Subject: [PATCH 2/6] Expand bundled testing to linux and macos --- .github/workflows/CI.yml | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/.github/workflows/CI.yml b/.github/workflows/CI.yml index 59175211588..732feff75a7 100644 --- a/.github/workflows/CI.yml +++ b/.github/workflows/CI.yml @@ -33,12 +33,13 @@ jobs: build-windows: name: build windows bundled - runs-on: windows-latest + runs-on: ${{ matrix.os }} strategy: fail-fast: false matrix: feature: ["", "static-link"] build_mode: ["", "--release"] + os: [macos-latest, ubuntu-latest, windows-latest] steps: - uses: actions/checkout@v2 - name: Build SDL2 @@ -56,12 +57,10 @@ jobs: cargo test --features "${CI_BUILD_FEATURES} ${{matrix.feature}}" ${{matrix.build_mode}} build-linux: - name: build linux + name: build linux pkg-config runs-on: ubuntu-20.04 strategy: fail-fast: false - matrix: - feature: ["", "bundled"] steps: - uses: actions/checkout@v2 - name: Install dependencies @@ -77,6 +76,6 @@ jobs: set -xeuo pipefail rustc --version cargo --version - cargo build --features "${CI_BUILD_FEATURES},${{matrix.feature}}" - cargo build --examples --features "${CI_BUILD_FEATURES},${{matrix.feature}}" - cargo test --features "${CI_BUILD_FEATURES},${{matrix.feature}}" + cargo build --features "${CI_BUILD_FEATURES}" + cargo build --examples --features "${CI_BUILD_FEATURES}" + cargo test --features "${CI_BUILD_FEATURES}" From 731550f5b1a4fdbcbbf0fbffeae4161d4e06217f Mon Sep 17 00:00:00 2001 From: Mike Waychison Date: Fri, 23 Apr 2021 08:51:32 -0700 Subject: [PATCH 3/6] Rename test It isn't just Windows anymore. --- .github/workflows/CI.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/CI.yml b/.github/workflows/CI.yml index 732feff75a7..63c54621cc3 100644 --- a/.github/workflows/CI.yml +++ b/.github/workflows/CI.yml @@ -32,7 +32,7 @@ jobs: cargo test --features "${CI_BUILD_FEATURES}" build-windows: - name: build windows bundled + name: build bundled runs-on: ${{ matrix.os }} strategy: fail-fast: false From b24a6f87d9fd295b4b459f0b84f12d8d746473bf Mon Sep 17 00:00:00 2001 From: Mike Waychison Date: Fri, 23 Apr 2021 09:01:24 -0700 Subject: [PATCH 4/6] Extend use of debug_postfix to all != emscripten --- sdl2-sys/build.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/sdl2-sys/build.rs b/sdl2-sys/build.rs index d9f99eef41f..1b0cf8280dc 100644 --- a/sdl2-sys/build.rs +++ b/sdl2-sys/build.rs @@ -416,10 +416,8 @@ fn link_sdl2(target_os: &str) { if cfg!(feature = "bundled") || cfg!(not(feature = "use-pkgconfig")) { if cfg!(feature = "use_mac_framework") && target_os == "darwin" { println!("cargo:rustc-flags=-l framework=SDL2"); - } else if target_os.contains("windows") { - println!("cargo:rustc-flags=-l SDL2{}", debug_postfix()); } else if target_os != "emscripten" { - println!("cargo:rustc-flags=-l SDL2"); + println!("cargo:rustc-flags=-l SDL2{}", debug_postfix()); } } } From b393f046cd3f7cb7d748b726030d07720df24dc6 Mon Sep 17 00:00:00 2001 From: Mike Waychison Date: Mon, 26 Apr 2021 09:40:11 -0700 Subject: [PATCH 5/6] Force use-pkgconfig to always use release build --- sdl2-sys/build.rs | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/sdl2-sys/build.rs b/sdl2-sys/build.rs index 17d06bf640f..6a31d9543a7 100644 --- a/sdl2-sys/build.rs +++ b/sdl2-sys/build.rs @@ -369,14 +369,19 @@ fn compute_include_paths() -> Vec { /// There's no easy way to extract this suffix from `cmake::Config` so we have to emulate their /// behaviour here (see the source for `cmake::Config::build`). fn debug_postfix() -> &'static str { - match ( - &env::var("OPT_LEVEL").unwrap_or_default()[..], - &env::var("PROFILE").unwrap_or_default()[..], - ) { - ("1", _) | ("2", _) | ("3", _) | ("s", _) | ("z", _) => "", - ("0", _) => "d", - (_, "debug") => "d", - (_, _) => "", + // pkgconfig is always a release build. + if cfg!(feature = "use-pkgconfig") { + "" + } else { + match ( + &env::var("OPT_LEVEL").unwrap_or_default()[..], + &env::var("PROFILE").unwrap_or_default()[..], + ) { + ("1", _) | ("2", _) | ("3", _) | ("s", _) | ("z", _) => "", + ("0", _) => "d", + (_, "debug") => "d", + (_, _) => "", + } } } From 5dc7077c2b19fd18e58a33012a8c242e31a1a056 Mon Sep 17 00:00:00 2001 From: Mike Waychison Date: Mon, 26 Apr 2021 09:51:39 -0700 Subject: [PATCH 6/6] Invert logic to hinge on "bundled" feature --- sdl2-sys/build.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sdl2-sys/build.rs b/sdl2-sys/build.rs index 6a31d9543a7..3b49ccf0f8d 100644 --- a/sdl2-sys/build.rs +++ b/sdl2-sys/build.rs @@ -369,8 +369,8 @@ fn compute_include_paths() -> Vec { /// There's no easy way to extract this suffix from `cmake::Config` so we have to emulate their /// behaviour here (see the source for `cmake::Config::build`). fn debug_postfix() -> &'static str { - // pkgconfig is always a release build. - if cfg!(feature = "use-pkgconfig") { + // default, and use-pkgconfig are always a release build. + if cfg!(not(feature = "bundled")) { "" } else { match (