-
Notifications
You must be signed in to change notification settings - Fork 74
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
fix includes for framework libs #630
base: main
Are you sure you want to change the base?
Conversation
It seems that I have bit more work to do on it |
Also note how previously we battled adding support for frameworks from homebrew installs here c6ac928 Which route of Qt install are you trying to improve here ? As i thought the remaining issues were around using the Qt official binaries ? Or have you found another scenario of frameworks that doesn't work ? 😅 Maybe it would be useful if you could post the error that occurs on your machine with |
I was trying to fix primarily the use of Qt 6.5 from the official installer. So, the use of framework libs, so adding a few specific args to the compiler. So, my method was :
I'll be able to recreate the initial problem at home tonight. Yet, it was the same error than in #450 |
Awesome thanks ! There are just so many different combinations it's useful to know which is being investigated :-) I find it fun that we solved use of frameworks from a homebrew install but not from a the Qt official installer, so it will be interesting to see the difference :-) Any help on macOS or Windows builds is much appreciated as we are mostly Linux devs 😅 |
I'm a cross-platform Qt dev and DevOps, so I know a little bit about such issues. I'll try to help, but must be clear about the platforms and installs I can play with:
I found out reading the code, that my first problem was to make qmake visible. So adding its folder to the PATH is a prerequise before using cxx-qt. |
Awesome :-)
Right, we also have a |
I would use another name for the variable like |
State of local tests:Do not merge before the completion of this list !
|
I compared the Qt installation from homebrew and Qt official installation. Homebrew installs Qt quite traditionally, plus add links like /opt/homebrew/include/QtCore linking to /opt/Cellar/qt/6.5.1_2/include/QtCore . So, somewhere, certainly this line, the good "include" path from homebrew was added. Yet, the work done was a necessary step which I was able to build on. In the Qt official installation, "packages" containing the libraries and the headers are found in [...]/Qt/6.5.2/macos/lib/[module name].framework I added the include paths like [...]/Qt/6.5.2/macos/lib/QtCore.framework/Headers |
Yes this change was to add support for framework style libs and specifically what homebrew was producing, but were not yet checked for what the Qt installer produces (which appears to be subtly different as you've stated). If you get a chance, checking that Qt 5.15 works via the Qt installer on macOS could would be good in case that is different again. |
Tested against Qt 5.15.2 (Official install), it compiles fine. |
It passes CI now 🥳 I'll let @Be-ing comment on the changes to the build side as well as he has been working on this side the most. |
Thank you for your help @ahayzen-kdab |
It seems that there are problem with the VCPKG version of Qt (told here) with this branch. So please wait for proper testing. I have troubles installing VCPKG's Qt on Linux & MacOS due to "ICU" not finding "autoconf-archive". Investigating... Sure, it's outside our scope. |
for qt_module in &self.qt_modules { | ||
paths.push(format!("{root_path}/Qt{qt_module}")); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this correct when Qt is installed as a framework? This allows using include paths like #include <QString>
instead of #include <QtCore/QString>
. Should we skip this part for frameworks?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I never included using #include <QtCore/QString>
, always using #include <QString>
like it's shown in the Qt documentation. So, I don't see the problem. Or I misunderstand your comment ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this code should likely be moved into an else
block of the if self.has_framework_libs
statement. If it wasn't sufficient before to get the include paths with frameworks, it's probably okay to not add these include paths when using frameworks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did a few researches :
The module-style includes (#include <QObject>
)are essential in MacOS framework style. Using the old #include <QtCore/qobject.h> or #include <QtCore/QObject> is not possible any longer. The old style still runs well on Windows and Linux thought.
I was testing only on qml_minimal example project, so this problem didn't appear.
On a hunch, I ran QMAKE=/Users/cyril/Qt/6.5.2/macos/bin/qmake cargo run -p qml-minimal-no-cmake
, so Qt's official install on MacOs with framework style
It gave me these errors:
cargo:warning=In file included from /Users/cyril/Devel/cxx-qt/target/debug/build/cxx-qt-lib-c19809693f328b69/out/cxxbridge/sources/cxx-qt-lib/src/core/qlist/qlist_i32.rs.cc:1:
cargo:warning=/Users/cyril/Devel/cxx-qt/target/debug/build/cxx-qt-lib-c19809693f328b69/out/cxx-qt-lib/qlist.h:11:10: fatal error: 'QtCore/QList' file not found
cargo:warning=#include <QtCore/QList>
cargo:warning= ^~~~~~~~~~~~~~
cargo:warning=1 error generated.
exit status: 1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Huh curious, i found doing #include <QtCore/QObject>
useful as it allows you to quickly determine which modules you need to include and link or why a library needs eg QtQuick as you can search for it quickly.
Guess we'll have to drop doing that if this doesn't work in macOS anymore (i can create a separate PR to do this), unless there is a different include folder that can be used that does include the module name + the header name ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I looked at the CI output from another projecting using the Qt official binary installer and the include paths are:
-isystem /Users/build/Qt/online/6.3.2/macos/lib/QtCore.framework/Headers
-isystem /Users/build/Qt/online/6.3.2/macos/lib/QtCore.framework/Headers/6.3.2
-isystem /Users/build/Qt/online/6.3.2/macos/lib/QtCore.framework/Headers/6.3.2/QtCore
So it seems we need:
if self.has_framework_libs {
let lib_root_path: String = self.qmake_query("QT_INSTALL_LIBS");
let qt_version = &self.qt_version;
for qt_module in &self.qt_modules {
paths.push(format!("{lib_root_path}/Qt{qt_module}.framework/Headers"));
paths.push(format!("{lib_root_path}/Qt{qt_module}.framework/Headers/{qt_version}"));
paths.push(format!("{lib_root_path}/Qt{qt_module}.framework/Headers/{qt_version}/Qt{qt_module}"));
}
}
Please try that code with both #include <QString>
and #include <QtCore/QString>
style includes.
I also saw -isystem /Users/build/Qt/online/6.3.2/macos/include
in the include paths, so forget what I was saying about moving the QT_INSTALL_HEADERS
path into an else
block; leave that code as it is.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello,
I'm back, I'll be able to test that this week.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jacquetc Have you had a chance to test the code I posted above?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No yet, I'll take time tomorrow at noon
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested, exactly the same error.
Rename QtBuild::is_framework to QtBuild::has_framwork_libs
e61d693
to
0f3e6e8
Compare
Please check that this branch also builds with Qt installed from Homebrew, which is also shipped as frameworks. You can test that by setting the QMAKE environment variable, for example |
Tested successfully with homebrew's Qt |
Co-authored-by: Be <[email protected]>
0f3e6e8
to
cfff8bc
Compare
Hello,
This is a tentative fix for using Qt's "framework" libs on MacOS. See #450