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

(not ready) Add debug variant of "bundled" (try #2) #1092

Draft
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

waych
Copy link
Contributor

@waych waych commented Apr 18, 2021

Includes CI testing for Windows bundled builds (both dynamic and static-link).

This re-enables "debug" build of "bundled" variants, for both static and dynamic link "bundled" builds.

Status:

@waych waych changed the title Add missing debug_postfix when copying Windows DLL (not ready) Add debug variant of "bundled" (try #2) Apr 18, 2021
This adds back the ability to compile the debug version of SDL2 when the
cargo build is a debug build.  Previous attempt (PR Rust-SDL2#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.
@waych waych marked this pull request as draft April 26, 2021 16:24
@TonalidadeHidrica
Copy link
Contributor

TonalidadeHidrica commented Oct 12, 2022

@waych What further work do you think is needed? I encountered an issue of copying dll because SDL2d.dll instead of SDL2.dll was generated, and adopting this PR resolved the issue. (I'm still having another issue though...)

@waych
Copy link
Contributor Author

waych commented Oct 21, 2022

@TonalidadeHidrica Unfortunately it has been a long time since I looked at this.

I think my main hangup with release vs not release is that the linkage on Windows in particular is messy, depending on what other C++/C dependencies you are linking into your app. Some crates will always link the release version of their C++/C deps, whereas others will seemingly change depending on whether it was a debug or release build in progress. These cannot be mixed in practice, at least on Windows.

I haven't found a good way in the cargo community for how this should be resolved (as it needs to be uniform across all linked dependencies). I think maybe people care less about this because this is more a Windows problem than anything else.

The off chance that this change breaks stuff as-is for people I think was the main reason I was hesitant to go ahead and request the pull. Perhaps better, would be for this functionality to be hidden behind yet another feature flag, something along the lines of sys_dep_debug_is_debug or something, so that the linkage behavior becomes an opt in.

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.

2 participants