-
Notifications
You must be signed in to change notification settings - Fork 88
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
qttypes: Support C++ toolchains on Windows other than MSVC #154
Comments
https://doc.rust-lang.org/reference/conditional-compilation.html#target_env Maybe we want |
No good. It can be set to "gnu", even though everything else is MSVC, including This is my setup right now:
As you can see, I've been through some MS s**t this weekend. But at least certainly learned something new... Still, qttypes build script produces this output:
Not bad, and even compiles and runs. But is definitely not what we expected. I print-debugged the Oh, wait, i know why! It's because build scripts are special. They are being built as a binaries for the target platform of a host. Hence the target env matches my toolchain. Easy. What we need instead are Environment variables Cargo sets for build scripts, namely And... boom! Tetris for Jeff. We got our
|
Simple cfg!(target_* = "...") doesn't work in build scripts the way it does in crate's code, because build scripts are being compiled for the toolchain's target triple -- not the default-host nor the cargo's eventual target. Instead, such configuration should be loaded from provided environment variables at run-time. While initially trying to fix linkage issues for Windows + MinGW target (first reported by @stuartZhang in #150), I came to realize in the process that cfg!(target_os = ...), although being used only for OS X at this moment, is likely screwed up in the same way (except, who would ever cross-compile to or from Mac, and why?) https://doc.rust-lang.org/cargo/reference/environment-variables.html#environment-variables-cargo-sets-for-build-scripts Fixes #154
Simple cfg!(target_* = "...") doesn't work in build scripts the way it does in crate's code, because build scripts are being compiled for the toolchain's target triple -- not the default-host nor the cargo's eventual target. Instead, such configuration should be loaded from provided environment variables at run-time. While initially trying to fix linkage issues for Windows + MinGW target (first reported by @stuartZhang in #150), I came to realize in the process that cfg!(target_os = ...), although being used only for OS X at this moment, is likely screwed up in the same way (except, who would ever cross-compile to or from Mac, and why?) https://doc.rust-lang.org/cargo/reference/environment-variables.html#environment-variables-cargo-sets-for-build-scripts Fixes #154
Summary
Build.rs script of qttypes assumes that on Windows the toolchain is always MSVC, which leads to linkage errors with e.g. MinGW toolchain.
Description
As discussed in #150, there are lines in qttypes/build.rs which only work for MSVC setup:
qmetaobject-rs/qttypes/build.rs
Lines 146 to 151 in d3df3a9
It produces this output:
which instructs cargo to pass to a linked a set of libs specifically built for debugging with MSVC toolchain. Libraries with a
d
suffix are only shipped with Qt MSVC builds.Note that since there is an
if debug
variable check involved in condition, this bug does not happen on--release
mode.Also, I don't think
msvcrt
/msvcrtd
is really needed outside of MSVC ecosystem? Sounds like a compiler's runtime library, similar tocompiler-rt
in LLVM land.Steps to reproduce
x86_64-pc-windows-gnu
ori686-pc-windows-gnu
. To avoid passing around--target
arguments, default target can be saved in Cargo configuration.--release
).Expected result
Everything should be OK.
Actual result
Now what?
Add to qttypes build script a check for current toolchain being used, and only add compiler runtime and
d
suffix if target is related to MSCV.Version
My best guess is qmetaobject-rs starting from v0.2. It doesn't happen on published v0.1.4, i.e. before factoring out qttypes into its own crate.
The text was updated successfully, but these errors were encountered: