Prl file parsing fixes

[jimmyvanhest]

The following three issues with prl file parsing have been fixed:

• previously QT_INSTALL_PREFIX would be treated the same as QT_INSTALL_LIBS which is not correct.
• some prl files names(mainly from android) would not follow the assumed patern.
• building for wasm requires that the libqwasm.prl must be included.

[Be-ing]

Thanks for working on build fixes for Android and emscripten! We haven't been trying these platforms ourselves, so this is a very welcome contribution!

[ahayzen-kdab]

(Note that you might want to rebase your branches onto main as i noticed when merging one of the others it was based on v0.5.3 which is a bit old now, so you could get conflicts)

[jimmyvanhest]

hmm latest changes failed on macOS Qt6 with duplicate symbol errors on the qml_extension_plugin example. Strange that only this platform would pull in additional object files that are now also handled by cxx-qt.

The following change might fix this, but I have no mac to test this on and would not like to spam the pipeline with things that might not actually do anything. For Linux Qt6 this would not make a difference though

examples/qml_extension_plugin/plugin/rust/Cargo.toml:

[build-dependencies]
-cxx-qt-build.workspace = true
+cxx-qt-build = { workspace = true, features = [ "link_qt_external" ] }

It might even be better to place this feature on on all crates that use cxx-qt-build and CMake for building, or invert the logic for this feature, since almost everything uses CMake here.

[jimmyvanhest]

What we are doing here now would technically be more correct but would break compatibility for some system and Qt installations, thus requiring major, or since this is still in major 0 a minor, version change.

[Be-ing]

hmm latest changes failed on macOS Qt6 with duplicate symbol errors on the qml_extension_plugin example. Strange that only this platform would pull in additional object files that are now also handled by cxx-qt.

macOS isn't the issue. I can reproduce this on Linux with a static build of Qt6. The issue is that both the Cargo side and the CMake side are both linking those .o files... 🤔 Maybe we have to only link Qt with CMake when linking dynamically? I'll experiment with that tomorrow.

[Be-ing]

Oof, we're in quite a maze of conflicting requirements and I can't think of a great solution that Just Works automatically in every case. The duplicate symbol errors come from Cargo linking the .o files into the staticlib and linking Qt's CMake targets also telling CMake to link the .o files. We can't have both Cargo and CMake linking the .o files.

There might be a way to tell CMake to not link the .o files when the Qt targets are static libraries. However, I think that would get quite messy with reaching into unstable implementation details of Qt's CMake modules. Perhaps we could try to get in touch with someone in the Qt Company that works on Qt's build system and ask them to add an option for not linking those .o files. If they agree, our downstream users would need to add a line to their CMakeLists.txt to enable that very niche option. Explaining why that option would need to be turned on would be awkward. Likely some downstream users would skip that and their end users would run into puzzling link errors when trying to link Qt statically.

We want Cargo to link the .o files when Cargo is building an executable, whether that's a bin crate or cargo test executables. Unfortunately Cargo doesn't provide any means for build.rs to determine what type of crate its building nor whether Cargo is building a test executable.

We could use a Cargo feature to toggle whether to link the .o files. That would allow Cargo to successfully link bin crates. We could make cargo test work by passing --no-default-features/--feature skip-linking-objects (depending on whether it's default on or off). Because we run cargo test indirectly from CMake in our repository, I think this may be the least worst option. Downstream users would only need to worry about enabling the feature via CLI when running cargo test on a staticlib crate and statically linking Qt. For bin crates, downstream users will need to always enable the feature in Cargo.toml.

[Be-ing]

I made another PR for your fork adding back the Cargo feature for linking .o files and enabling it for our bin crate and cargo test: https://github.com/jimmyvanhest/cxx-qt/pull/2. I tested merging that with #513 and linking to a static build of Qt 6 on Linux and cmake & ctest both work.

[Be-ing]

I made a new PR #605 with your commits plus a few of my own on top of those. We've merged that to unblock #513. Thank you for contributing this!

[jimmyvanhest]

Ah great that it's merged.

I did not have much free time this week too look at it hence the silence from me.

I will tests all of it out with my setup for wasm and android.

And a quick question, is there any indication when a new release will be made? I would need that since I'd like to move away from using this crate as a submodule in my project and just use cargo.

[ahayzen-kdab]

And a quick question, is there any indication when a new release will be made? I would need that since I'd like to move away from using this crate as a submodule in my project and just use cargo.

We are currently in the middle of an API transition to 0.6, currently projecting for maybe end of August for the 0.6 series.

However I can try to do a backport to the 0.5.x series and make a 0.5.4 release if this doesn't regress / change the behaviour for 0.5.x 🤔

[jimmyvanhest]

We are currently in the middle of an API transition to 0.6, currently projecting for maybe end of August for the 0.6 series.

neat.

However I can try to do a backport to the 0.5.x series and make a 0.5.4 release if this doesn't regress / change the behaviour for 0.5.x thinking

While handy I won't require it very much. Using a submodule till then would not be bothersome for me, so don't bother with it(unless you really want of course). It was just a curiosity because the last release was a while ago.

[ahayzen-kdab]

Cool, thanks for the info :-) I think we'll continue focusing on the 0.6 series for now unless there is a serious bug fix we need to backport to 0.5.x (then we could also include these changes).

Let us know if you do need a release for something though :-)