bitcoin-qt.exe will build but won't run due to missing QT5
dependencies.
This patch adds the required dependencies.
Depends on D2666
Details
Details
- Reviewers
deadalnix - Group Reviewers
Restricted Project
Prerequisite:
Follow the doc/build-windows.md and depends/README.md documentation
and install the dependencies for i686-w64-mingw32 and
x86_64-w64-mingw32.
# 32-bit version mkdir buildcmake && cd buildcmake cmake -GNinja .. -DBUILD_BITCOIN_SEEDER=OFF \ -DCMAKE_TOOLCHAIN_FILE=../cmake/platforms/Win32.cmake ninja
Check the build succeeds.
Check the generated src/qt/bitcoin-qt.exe binary runs as expected.
Operate the same for Win64.
# 64-bit version rm -rf * cmake -GNinja .. -DBUILD_BITCOIN_SEEDER=OFF \ -DCMAKE_TOOLCHAIN_FILE=../cmake/platforms/Win64.cmake ninja
Check the build succeeds.
Check the generated src/qt/bitcoin-qt.exe binary runs as expected.
Diff Detail
Diff Detail
- Repository
- rABC Bitcoin ABC
- Branch
- cmake_windows_bitcoin_qt
- Lint
Lint Passed - Unit
No Test Coverage - Build Status
Buildable 5176 Build 8415: Bitcoin ABC Buildbot (legacy) Build 8414: arc lint + arc unit
Event Timeline
| src/qt/CMakeLists.txt | ||
|---|---|---|
| 162 | Actually find_package searches for a Config.cmake suffix, and these files are only provided for high level modules. | |
| src/qt/CMakeLists.txt | ||
|---|---|---|
| 164 ↗ | (On Diff #7683) | There is a variation of this for each plateform. |
| 203 ↗ | (On Diff #7683) | Why is that required ? If it is include on other plateform automatically, why isn't it on windows ? If it isn't, why is it necessary specifically on windows ? |
| 162 | Qt should be able to set it's dependencies properly, or to provide target for these dependencies. | |
Comment Actions
Fix bugs in the contrib/convert-prl-libs-to-cmake.pl. This allows for
converting the windows plugin prl script rather than manually linking
the dependencies.
| src/qt/CMakeLists.txt | ||
|---|---|---|
| 162 | There is no package nor config for these targets. Intended that we already use a script to parse the .prl files and convert them to cmake, I will use that to link the dependencies. | |
| contrib/qt/convert-prl-libs-to-cmake.pl | ||
|---|---|---|
| 88 ↗ | (On Diff #8256) | If there is no INTERFACE_LINK_LIBRARIES for this target, get_target_property set _link_libs to _link_libs_NOTFOUND. |
| 119 ↗ | (On Diff #8256) | The QT directory structure is so that the .a library files are located under a lib directory. This lib directory is referenced in the .prl files as $$[QT_INSTALL_LIBS]$$. The replacement here used to assume that the .prl files are also located under lib, but there are exceptions to this rule: the plugins are located in a plugin subdirectory, plugin being at the same level as lib. When parsing plugins .prl files, the script would have linked to static libs from the wrong directory. |
| contrib/qt/convert-prl-libs-to-cmake.pl | ||
|---|---|---|
| 61 ↗ | (On Diff #8256) | Te case of the prl not being in the lib repository is handled here. |
| 119 ↗ | (On Diff #8256) | I'm not sure what you are trying to do but. 1/ The assumptions stated in this comment are erroneous, so I highly doubt the code is correct. The actual assumption made is that the prl is either in the same directory as the library, or in its parent directory. |
| src/qt/CMakeLists.txt | ||
| 32 ↗ | (On Diff #8256) | This makes even more assumptions than the previous way, relying on private variable being set the way you expect and being the same for all modules, which none are guaranteed. |
| 46 ↗ | (On Diff #8256) | It looks like you should add this to QT_REQUIRED_COMPONENTS as it clearly is a component from Qt that is required in this case. It's also probably a good idea to port whatever autotool is doing for these libraries instead of hacking each plateform one by one. |
| 165 ↗ | (On Diff #8256) | What problem is this solving ? Presumably, this should be set the proper way by the cmake files provided by Qt and if not, then actually figuring out what the problem is seems like the first step here. |
| 178 ↗ | (On Diff #8256) | Would that be a Qt component that is required ? And if so, why isn't QT_REQUIRED_COMPONENTS used ? |
| contrib/qt/convert-prl-libs-to-cmake.pl | ||
|---|---|---|
| 61 ↗ | (On Diff #8256) | This does not handle the case where the file is in another directory, only a special case where there is no "lib" directory. |
| 119 ↗ | (On Diff #8256) | 1/ The assumptions are what the previous version did. This code removes these assumptions by using an independent variable. This is actually a fix. |
| src/qt/CMakeLists.txt | ||
| 32 ↗ | (On Diff #8256) | The QT directory structure is fixed, so I think this is safe. It does not make more assumptions as it breaks the previous one which is known to be wrong. |
| 46 ↗ | (On Diff #8256) | You can't run the find_package on this one, as there is no package definition for it. This is why it is not part of the required components. |
| 165 ↗ | (On Diff #8256) | There is an #if defined(QT_STATICPLUGIN) in the code to include <QTPlugin>. Cmake has no way to determine this. |
| 178 ↗ | (On Diff #8256) | See my comment above. |
| src/qt/CMakeLists.txt | ||
|---|---|---|
| 32 ↗ | (On Diff #8256) | I still do not know what previous assumption was wrong. It is however highly suspicious that leaving Qt's install tree is ever a good idea, because by definition you aren't in Qt there anymore are are making assumption as to what, at least, the top folder is. This is also making the assumptions that you can rely on internal variable from Qt which previous use is clearly marked with
Which isn't exactly a brilliant endorsement of the practice. I would keep arguing here but you already *KNOW* that all of this is wrong because you commented on it by yourself. So do the right thing. |
| 46 ↗ | (On Diff #8256) | Here is your problem here. You notice bitcoin-qt doesn't work on win32. You try to fix it. That's good except, bitcoin-qt work on win32 on the good old build system. Therefore, porting that good old build system, or at least the missing part, is the way to go, unless you want to be playing "where is waldo?" trying to figure out why both build do not match in obscure conditions. Note that i already pointed to this in https://reviews.bitcoinabc.org/D2667?id=7683#inline-16405 . |
| 165 ↗ | (On Diff #8256) | I could get to the bottom of this by grepping into the autotool build, but we are looping back to https://reviews.bitcoinabc.org/D2667#inline-17832 and code review shouldn't involve playing Where is Waldo. |