From d95441b960815f7825b8a7daacac7a9cb5e29405 Mon Sep 17 00:00:00 2001 From: Andrew Hayzen Date: Wed, 28 Jun 2023 14:46:07 +0100 Subject: [PATCH 1/2] cxx-qt-build: do not use -bundle otherwise lib doesn't end up in CMake 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 Otherwise when we build with CMake the libqt-static-initializers.a is not bundled into the final libqt_minimal.a so is then missing from the final binary. Closes #597 --- CHANGELOG.md | 4 ++++ crates/cxx-qt-build/Cargo.toml | 1 + crates/cxx-qt-build/src/lib.rs | 27 +++++++++++++++++++++++---- 3 files changed, 28 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a9be4b509..5e3ed6a72 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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. diff --git a/crates/cxx-qt-build/Cargo.toml b/crates/cxx-qt-build/Cargo.toml index 279de0216..cfa20e682 100644 --- a/crates/cxx-qt-build/Cargo.toml +++ b/crates/cxx-qt-build/Cargo.toml @@ -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"] diff --git a/crates/cxx-qt-build/src/lib.rs b/crates/cxx-qt-build/src/lib.rs index e7db5fb51..27885d679 100644 --- a/crates/cxx-qt-build/src/lib.rs +++ b/crates/cxx-qt-build/src/lib.rs @@ -11,6 +11,8 @@ //! for CXX-Qt or CXX macros and generate any resulting C++ code. It also builds //! the C++ code into a binary with any cxx-qt-lib code and Qt linked. +extern crate version_check as rustc; + mod diagnostics; use diagnostics::{Diagnostic, GeneratedError}; @@ -424,10 +426,27 @@ 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) = rustc::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] { builder.cpp(true); // MSVC From 482a4212c8f50295172401a0d055d8634e4b4c4e Mon Sep 17 00:00:00 2001 From: Andrew Hayzen Date: Mon, 26 Jun 2023 10:47:07 +0100 Subject: [PATCH 2/2] cxx-qt-lib: use +whole_archive for std_types so that cargo-only works When building with cargo we need to ensure that the statics aren't optimised out for registering types with Qt 5. Otherwise we cannot use numbers in QML. Also move std_types from -lib into -build, which potentially allows us to have cxx-qt-lib separate in the future. Closes #592 --- crates/cxx-qt-build/src/lib.rs | 38 ++++++++++++++++--- .../src/std_types_qt5.cpp} | 0 crates/cxx-qt-lib/build.rs | 5 ++- 3 files changed, 36 insertions(+), 7 deletions(-) rename crates/{cxx-qt-lib/src/std_types.cpp => cxx-qt-build/src/std_types_qt5.cpp} (100%) diff --git a/crates/cxx-qt-build/src/lib.rs b/crates/cxx-qt-build/src/lib.rs index 27885d679..d91f92d28 100644 --- a/crates/cxx-qt-build/src/lib.rs +++ b/crates/cxx-qt-build/src/lib.rs @@ -11,8 +11,6 @@ //! for CXX-Qt or CXX macros and generate any resulting C++ code. It also builds //! the C++ code into a binary with any cxx-qt-lib code and Qt linked. -extern crate version_check as rustc; - mod diagnostics; use diagnostics::{Diagnostic, GeneratedError}; @@ -391,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"); @@ -428,7 +425,7 @@ impl CxxQtBuilder { cc_builder_whole_archive.link_lib_modifier("+whole-archive"); // Ensure we are not using rustc 1.69 - if let Some(version) = rustc::Version::read() { + 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 @@ -448,6 +445,7 @@ impl CxxQtBuilder { } 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"); @@ -525,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"); + 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"); } diff --git a/crates/cxx-qt-lib/src/std_types.cpp b/crates/cxx-qt-build/src/std_types_qt5.cpp similarity index 100% rename from crates/cxx-qt-lib/src/std_types.cpp rename to crates/cxx-qt-build/src/std_types_qt5.cpp diff --git a/crates/cxx-qt-lib/build.rs b/crates/cxx-qt-lib/build.rs index 8c432f4cf..b7008e0f1 100644 --- a/crates/cxx-qt-lib/build.rs +++ b/crates/cxx-qt-lib/build.rs @@ -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 @@ -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"); }