From 0d7be1f9ff6a7d594f94fbfd2c594a29b22066dd Mon Sep 17 00:00:00 2001 From: Leon Matthes Date: Thu, 23 May 2024 11:30:33 +0200 Subject: [PATCH 1/2] fix(ci): Remove dependency on Rust 1.77 Closes #958 --- .github/workflows/github-cxx-qt-tests.yml | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/.github/workflows/github-cxx-qt-tests.yml b/.github/workflows/github-cxx-qt-tests.yml index 24f9b9cd1..b4baead5c 100644 --- a/.github/workflows/github-cxx-qt-tests.yml +++ b/.github/workflows/github-cxx-qt-tests.yml @@ -193,13 +193,8 @@ jobs: # Ensure clippy and rustfmt is installed, they should come from github runner # # Note we still need rustfmt for the cxx-qt-gen tests - # - # TODO: Remove the `rustup default 1.77` selection. - # This is a workaround for Github Actions, which cannot currently compile CXX-Qt with Rust 1.78. - # - # See: https://github.com/KDAB/cxx-qt/issues/958 - name: "Install Rust toolchain" - run: rustup default 1.77 && rustup component add clippy rustfmt + run: rustup component add clippy rustfmt - name: "Rust tools cache" uses: actions/cache@v4 From 4e7f2f31b815d9eb42850648f091f950f92af126 Mon Sep 17 00:00:00 2001 From: Leon Matthes Date: Thu, 23 May 2024 16:13:52 +0200 Subject: [PATCH 2/2] fix(ci): Use whole-archive only for initializers Previously, we linked the entire staticlib generated by the Rust compiler into our CMake target. This was part of what caused the recent issues in CI (see #958). Cargo already only linked the plugin&resources library as whole-archive, which is where where it is needed. Now we simply export the initializers library into the CXXQT_EXPORT_DIR, so we can only link the initializers with whole-archive. This is a lot cleaner and should lead to fewer linker failures. Closes #958 --- crates/cxx-qt-build/src/lib.rs | 45 ++++++++++++++++++-------- examples/demo_threading/CMakeLists.txt | 4 ++- examples/qml_features/CMakeLists.txt | 5 ++- examples/qml_minimal/CMakeLists.txt | 11 +++++-- 4 files changed, 47 insertions(+), 18 deletions(-) diff --git a/crates/cxx-qt-build/src/lib.rs b/crates/cxx-qt-build/src/lib.rs index ae7c28af1..68c15be48 100644 --- a/crates/cxx-qt-build/src/lib.rs +++ b/crates/cxx-qt-build/src/lib.rs @@ -255,13 +255,21 @@ fn generate_cxxqt_cpp_files( generated_file_paths } -/// The include directory needs to be namespaced by crate name when exporting for a C++ build system, +/// The export directory, if one was specified through the environment. +fn export_dir() -> Option { + env::var("CXXQT_EXPORT_DIR") + .ok() + .map(|export_dir| format!("{export_dir}/{}", crate_name())) +} + +/// The export 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. fn header_root() -> String { - match env::var("CXXQT_EXPORT_DIR") { - Ok(export_dir) => format!("{export_dir}/{}", env::var("CARGO_PKG_NAME").unwrap()), - Err(_) => env::var("OUT_DIR").unwrap(), - } + export_dir().unwrap_or_else(|| env::var("OUT_DIR").unwrap()) +} + +fn crate_name() -> String { + env::var("CARGO_PKG_NAME").unwrap() } fn panic_duplicate_file_and_qml_module( @@ -570,8 +578,17 @@ impl CxxQtBuilder { // from within main when static linking, which would result in discarding those static variables. // Use a separate cc::Build for the little amount of code that needs to be linked with +whole-archive // to avoid bloating the binary. - let mut cc_builder_whole_archive = cc::Build::new(); - cc_builder_whole_archive.link_lib_modifier("+whole-archive"); + let mut cc_builder_static_initializers = cc::Build::new(); + if let Some(export_dir) = export_dir() { + // We have an export dir, so output the static-initializers library there, but avoid + // linking to it + cc_builder_static_initializers.out_dir(export_dir); + } else { + // We don't have an export dir, so this is likely a cargo-only build. + // Therefore make sure we link to the static-initializers library with +whole-archive, + // which ensures everything is linked, even without any references. + cc_builder_static_initializers.link_lib_modifier("+whole-archive"); + } // Ensure we are not using rustc 1.69 if let Some(version) = version_check::Version::read() { @@ -593,7 +610,7 @@ impl CxxQtBuilder { } } - for builder in [&mut self.cc_builder, &mut cc_builder_whole_archive] { + for builder in [&mut self.cc_builder, &mut cc_builder_static_initializers] { // Note, ensure our settings stay in sync across cxx-qt, cxx-qt-build, and cxx-qt-lib builder.cpp(true); builder.std("c++17"); @@ -668,10 +685,10 @@ impl CxxQtBuilder { self.cc_builder .file(qml_module_registration_files.qmltyperegistrar); self.cc_builder.file(qml_module_registration_files.plugin); - cc_builder_whole_archive.file(qml_module_registration_files.plugin_init); - cc_builder_whole_archive.file(qml_module_registration_files.rcc); + cc_builder_static_initializers.file(qml_module_registration_files.plugin_init); + cc_builder_static_initializers.file(qml_module_registration_files.rcc); for qmlcachegen_file in qml_module_registration_files.qmlcachegen { - cc_builder_whole_archive.file(qmlcachegen_file); + cc_builder_static_initializers.file(qmlcachegen_file); } self.cc_builder.define("QT_STATICPLUGIN", None); cc_builder_whole_archive_files_added = true; @@ -688,7 +705,7 @@ impl CxxQtBuilder { } for qrc_file in self.qrc_files { - cc_builder_whole_archive.file(qtbuild.qrc(&qrc_file)); + cc_builder_static_initializers.file(qtbuild.qrc(&qrc_file)); // Also ensure that each of the files in the qrc can cause a change for qrc_inner_file in qtbuild.qrc_list(&qrc_file) { @@ -723,12 +740,12 @@ impl CxxQtBuilder { 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_static_initializers.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"); + cc_builder_static_initializers.compile(&format!("{}-initializers", crate_name())); } // Only compile if we have added files to the builder diff --git a/examples/demo_threading/CMakeLists.txt b/examples/demo_threading/CMakeLists.txt index c8e712633..048dc52d9 100644 --- a/examples/demo_threading/CMakeLists.txt +++ b/examples/demo_threading/CMakeLists.txt @@ -52,8 +52,10 @@ corrosion_set_env_vars(${CRATE} add_library(${APP_NAME}_lib INTERFACE) target_include_directories(${APP_NAME}_lib INTERFACE "${CXXQT_EXPORT_DIR}/${CRATE}") +target_link_directories(${APP_NAME}_lib INTERFACE "${CXXQT_EXPORT_DIR}/${CRATE}") target_link_libraries(${APP_NAME}_lib INTERFACE - "$" + "$" + ${CRATE} Qt::Core Qt::Gui Qt::Qml diff --git a/examples/qml_features/CMakeLists.txt b/examples/qml_features/CMakeLists.txt index ebdab49a4..89095b157 100644 --- a/examples/qml_features/CMakeLists.txt +++ b/examples/qml_features/CMakeLists.txt @@ -51,8 +51,11 @@ corrosion_set_env_vars(${CRATE} ) add_library(${APP_NAME}_lib INTERFACE) target_include_directories(${APP_NAME}_lib INTERFACE "${CXXQT_EXPORT_DIR}/${CRATE}") +target_link_directories(${APP_NAME}_lib INTERFACE "${CXXQT_EXPORT_DIR}/${CRATE}") + target_link_libraries(${APP_NAME}_lib INTERFACE - "$" + "$" + ${CRATE} Qt::Core Qt::Gui Qt::Qml diff --git a/examples/qml_minimal/CMakeLists.txt b/examples/qml_minimal/CMakeLists.txt index 464bebedc..74fad5780 100644 --- a/examples/qml_minimal/CMakeLists.txt +++ b/examples/qml_minimal/CMakeLists.txt @@ -72,16 +72,23 @@ corrosion_set_env_vars(${CRATE} # the include paths and linked libraries for both of those. add_library(${APP_NAME}_lib INTERFACE) -# Include the headers generated by the Rust library's build script. Each +# Include the headers & libraries generated by the Rust library's build script. Each # crate gets its own subdirectory under CXXQT_EXPORT_DIR. This allows you # to include headers generated by multiple crates without risk of one crate # overwriting another's files. target_include_directories(${APP_NAME}_lib INTERFACE "${CXXQT_EXPORT_DIR}/${CRATE}") +# If you're using qml modules or qrc resources, make sure to add the export directory +# as a link_directory and then link the ${CRATE}-initializers library with WHOLE_ARCHIVE. +target_link_directories(${APP_NAME}_lib INTERFACE "${CXXQT_EXPORT_DIR}/${CRATE}") target_link_libraries(${APP_NAME}_lib INTERFACE # WHOLE_ARCHIVE is needed for the generated QML plugin to register on startup, # otherwise the linker will discard the static variables that initialize it. - "$" + # + # If you're not using QML plugins or QRC files with cxx-qt-build, this line is not needed. + "$" + # Additionally, link to the crate's library as normal. + ${CRATE} Qt::Core Qt::Gui Qt::Qml