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

cxx-qt-lib: use +whole_archive for std_types so that cargo-only works #593

Merged
merged 2 commits into from
Jun 29, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- `#[qinvokable]` is now defined as a signature in `extern "RustQt"`
- `rust_mut` is now safe to call

### Fixed

- Do not use -bundle otherwise CMake builds are missing qt-static-initalizers (note this is broken in rustc 1.69)

### Removed

- Removed support for `cxx_type` and `cxx_return_type` and related conversion methods.
Expand Down
1 change: 1 addition & 0 deletions crates/cxx-qt-build/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ proc-macro2.workspace = true
quote.workspace = true
qt-build-utils.workspace = true
codespan-reporting = "0.11"
version_check = "0.9"

[features]
default = ["qt_gui", "qt_qml"]
Expand Down
59 changes: 53 additions & 6 deletions crates/cxx-qt-build/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -389,12 +389,11 @@ impl CxxQtBuilder {
// Ensure that the linker is setup correctly for Cargo builds
qt_build_utils::setup_linker();

let out_dir = env::var("OUT_DIR").unwrap();
// The include directory needs to be namespaced by crate name when exporting for a C++ build system,
// but for using cargo build without a C++ build system, OUT_DIR is already namespaced by crate name.
let header_root = match env::var("CXXQT_EXPORT_DIR") {
Ok(export_dir) => format!("{export_dir}/{}", env::var("CARGO_PKG_NAME").unwrap()),
Err(_) => out_dir,
Err(_) => env::var("OUT_DIR").unwrap(),
};
let generated_header_dir = format!("{header_root}/cxx-qt-gen");

Expand Down Expand Up @@ -424,11 +423,29 @@ impl CxxQtBuilder {
// to avoid bloating the binary.
let mut cc_builder_whole_archive = cc::Build::new();
cc_builder_whole_archive.link_lib_modifier("+whole-archive");
// Workaround for spurious compiler error in Rust 1.69. The bug has been fixed, but keeping this
// workaround seems to be harmless.
// https://github.com/rust-lang/rust/issues/110912
cc_builder_whole_archive.link_lib_modifier("-bundle");

// Ensure we are not using rustc 1.69
if let Some(version) = version_check::Version::read() {
let (major, minor, _) = version.to_mmp();
if major == 1 && minor == 69 {
// rustc 1.69 had a regression where +whole-archive wouldn't
// work without specifying -bundle.
// https://github.com/rust-lang/rust/pull/110917
//
// However, we need to not have -bundle for qt-static-initializers to work
// with CMake builds, otherwise the statement below occurs where it's missing
// from the final binary.
//
// When building a staticlib -bundle means that the native static library
// is simply not included into the archive and some higher level build
// system will need to add it later during linking of the final binary.
// https://doc.rust-lang.org/rustc/command-line-arguments.html#option-l-link-lib
panic!("rustc 1.69.x is not supported with CXX-Qt due to a compiler bug.\nSee: https://github.com/rust-lang/rust/pull/110917\nPlease update your compiler using 'rustup update' or use an older compiler.");
}
}

for builder in [&mut self.cc_builder, &mut cc_builder_whole_archive] {
// Note, ensure our settings stay in sync across cxx-qt-build and cxx-qt-lib
builder.cpp(true);
// MSVC
builder.flag_if_supported("/std:c++17");
Expand Down Expand Up @@ -506,6 +523,36 @@ impl CxxQtBuilder {
cc_builder_whole_archive.file(qtbuild.qrc(&qrc_file));
cc_builder_whole_archive_files_added = true;
}

// If we are using Qt 5 then write the std_types source
// This registers std numbers as a type for use in QML
//
// Note that we need this to be compiled into the whole_archive builder
// as they are stored in statics in the source.
//
// TODO: once +whole-archive and +bundle are allowed together in rlibs
// we should be able to move this into cxx-qt so that it's only built
// once rather than for every cxx-qt-build. When this happens also
// ensure that in a multi project that numbers work everywhere.
//
// Also then it should be possible to use CARGO_MANIFEST_DIR/src/std_types_qt5.cpp
// as path for cc::Build rather than copying the .cpp file
//
// https://github.com/rust-lang/rust/issues/108081
// https://github.com/KDAB/cxx-qt/pull/598
if qtbuild.version().major == 5 {
let std_types_contents = include_str!("std_types_qt5.cpp");
let std_types_path = format!(
"{out_dir}/std_types_qt5.cpp",
out_dir = env::var("OUT_DIR").unwrap()
);
let mut source =
File::create(&std_types_path).expect("Could not create std_types source");
ahayzen-kdab marked this conversation as resolved.
Show resolved Hide resolved
write!(source, "{std_types_contents}").expect("Could not write std_types source");
cc_builder_whole_archive.file(&std_types_path);
cc_builder_whole_archive_files_added = true;
}

if cc_builder_whole_archive_files_added {
cc_builder_whole_archive.compile("qt-static-initializers");
}
Expand Down
5 changes: 3 additions & 2 deletions crates/cxx-qt-lib/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -232,8 +232,6 @@ fn main() {
}
builder.file("src/qt_types.cpp");
println!("cargo:rerun-if-changed=src/qt_types.cpp");
builder.file("src/std_types.cpp");
println!("cargo:rerun-if-changed=src/std_types.cpp");
println!("cargo:rerun-if-changed=src/assertion_utils.h");

// Write this library's manually written C++ headers to files and add them to include paths
Expand All @@ -251,11 +249,14 @@ fn main() {
builder.define("CXX_QT_QML_FEATURE", None);
}

// Note, ensure our settings stay in sync across cxx-qt-build and cxx-qt-lib
builder.cpp(true);
// MSVC
builder.flag_if_supported("/std:c++17");
builder.flag_if_supported("/Zc:__cplusplus");
builder.flag_if_supported("/permissive-");
// GCC + Clang
builder.flag_if_supported("-std=c++17");

builder.compile("cxx-qt-lib");
}