[qt5-base] fix building with mac dynamic triplet #32500
Conversation
|
Please update port version. Note: I will be converting your PR to draft status. When you respond, please revert to "ready for review". That way, I can be aware that you've responded since you can't modify the tags. |
Adela0814
changed the title
ports/qt5-base/portfile.cmake
Outdated
| @@ -289,6 +289,7 @@ elseif(VCPKG_TARGET_IS_LINUX) | |||
| list(APPEND DEBUG_OPTIONS "PSQL_LIBS=${PSQL_DEBUG} ${PSQL_PORT_DEBUG} ${PSQL_TYPES_DEBUG} ${PSQL_COMMON_DEBUG} ${SSL_DEBUG} ${EAY_DEBUG} -ldl -lpthread") | |||
| endif() | |||
| elseif(VCPKG_TARGET_IS_OSX) | |||
| list(APPEND DEBUG_OPTIONS -no-framework) | |||
There was a problem hiding this comment.
Why is this option needed at all? In vcpkg CI, I didn't see any frameworks installed.
Why is this option only needed for DEBUG_OPTIONS?
There was a problem hiding this comment.
See the linked issue. Qt does not support frameworks for debug.
And frameworks are the default when using the dynamic triplet.
I could add an "if dynamic".
There was a problem hiding this comment.
And frameworks are the default when using the dynamic triplet.
So this is the difference to vcpkg CI.
Frameworks aren't desired in vcpkg, so the parameter should simply go to the common options for osx, not just debug.
There was a problem hiding this comment.
Cool! No problem. Will do. Found some rpath problems, too, that I already fixed locally.
Adela0814
added
the
category:port-bug
jpfeuffer
changed the title
| # Avoid frameworks for vcpkg | ||
| list(APPEND CORE_OPTIONS -no-framework) | ||
| # Such that Qt executables like moc find their libs. The default path is ../Frameworks | ||
| list(APPEND DEBUG_OPTIONS -R ${CURRENT_INSTALLED_DIR}/debug/lib) |
There was a problem hiding this comment.
This seems like it will embed an absolute path which is bad?
There was a problem hiding this comment.
Sorry, but I cannot get it to work with relative paths.
Happy to try something though if you have an idea. All combinations of "@executable_path/../lib" are completely ignored by their build system. Not sure if the @ needs to be escaped somehow.. no idea..
However, I still think it's an improvement.
I think the only problem is that the vcpkg folder would not be relocatable for the Qt binaries under these very specific circumstances. Potentially users can even fix this themselves after relocating.
Better than not being able to build at all.
There was a problem hiding this comment.
the vcpkg folder would not be relocatable for the Qt binaries under these very specific circumstances.
It isn't "very specific circumstances", it is "all binary cache hits".
Moreover, in general customers expect to be able to deploy what comes out of a build with their application, which is almost never going to be "identical to a vcpkg installation tree in the build lab" in terms of location.
If it won't work with their build system I think you need to fix it to work with patchelf or similar.
There was a problem hiding this comment.
Upon further reflection, this is bad, but it's no worse than the status quo (that being "totally broken") so I'm weakly in favor of merging it. Going to ask other maintainers.
There was a problem hiding this comment.
For the installed tree, there is #32200.
There was a problem hiding this comment.
For the installed tree, there is #32200.
Would this change be needed in addition to that or does that obsolete this?
There was a problem hiding this comment.
So, I actually found a way to include relative RPaths. The problem is now:
Qt tries to run/test/use its /Users/jpfeuffer/.clion-vcpkg/vcpkg/buildtrees/qt5-base/x64-osx-dynamic-dbg/bin/moc binary during the build.
But the libs its looking for are already installed in /Users/jpfeuffer/build/vcpkg_installed/x64-osx-dynamic/debug/lib/libbz2d.dylib. Apparently, the loading needs work during the build process already since the binary is used or tested there. I probably can include both paths during build, since I don't know yet how to differentiate between build and install rpaths with qmake.
There was a problem hiding this comment.
For the installed tree, there is #32200.
Would this change be needed in addition to that or does that obsolete this?
In addition. That PR is only for the installed tree, but here fixes are needed already during the build AFAIU.
There was a problem hiding this comment.
OK, we'll merge on the grounds that everything with osx-dynamic is toast (and we shouldn't have even added that as a community triplet) and this just brings up to parity with everything else there.
There was a problem hiding this comment.
I see that it can be a lot of work but it was very helpful for us.
We want to use vcpkg also for deployment, including all dependencies and there are so many problems with static libraries:
- first of all Qt's LGPL license which requires additional steps to fulfill it when linking statically
- then the size of the executables
- and more minor caveats in CMake and the code itself to correctly handle transitive dependencies in a complex project
I'm just hoping it won't get removed ;)
Thanks for the help.
fixes #32497
./vcpkg x-add-version --alland committing the result.