Skip to content

cmake: Do not interfere with Qt plugin handling#103

Merged
hebasto merged 3 commits into
cmake-stagingfrom
240225-cmake-BS
Mar 8, 2024
Merged

cmake: Do not interfere with Qt plugin handling#103
hebasto merged 3 commits into
cmake-stagingfrom
240225-cmake-BS

Conversation

@hebasto
Copy link
Copy Markdown
Owner

@hebasto hebasto commented Feb 25, 2024

  1. When using CMake, each plugin comes with a C++ stub file that automatically initializes the static plugin. Consequently, any target that links against a plugin has this C++ file added to its SOURCES, which makes the removed code redundant and duplicates the included plugins. For example, on the staging branch @ 1a976ca:
2024-03-01T10:51:26Z Bitcoin Core version v26.99.0-1a976ca427ca (release build)
2024-03-01T10:51:26Z Qt 5.15.11 (static), plugin=windows (static)
2024-03-01T10:51:26Z Static plugins:
2024-03-01T10:51:26Z  QWindowsIntegrationPlugin, version 331520
2024-03-01T10:51:26Z  QWindowsVistaStylePlugin, version 331520
2024-03-01T10:51:26Z  QWindowsVistaStylePlugin, version 331520
2024-03-01T10:51:26Z  QWindowsIntegrationPlugin, version 331520
2024-03-01T10:51:26Z Style: windowsvista / QWindowsVistaStyle
2024-03-01T10:51:26Z System: Windows 11 Version 2009, x86_64-little_endian-llp64
2024-03-01T10:51:26Z Screen: \\.\DISPLAY1 1920x1080, pixel ratio=1.0

Initially noticed in #101 (review).

Here are LogQtInfo() outputs in the debug.log:

  • x86_64-pc-linux-gnu:
2024-02-25T10:15:18Z Qt 5.15.11 (static), plugin=xcb (static)
2024-02-25T10:15:18Z Static plugins:
2024-02-25T10:15:18Z  QXcbIntegrationPlugin, version 331520
2024-02-25T10:15:18Z Style: fusion / QFusionStyle
2024-02-25T10:15:18Z System: Ubuntu 22.04.4 LTS, x86_64-little_endian-lp64
2024-02-25T10:15:18Z Screen: XWAYLAND0 1920x1080, pixel ratio=1.0
  • x86_64-w64-mingw32
2024-02-25T10:44:54Z Qt 5.15.11 (static), plugin=windows (static)
2024-02-25T10:44:54Z Static plugins:
2024-02-25T10:44:54Z  QWindowsVistaStylePlugin, version 331520
2024-02-25T10:44:54Z  QWindowsIntegrationPlugin, version 331520
2024-02-25T10:44:54Z Style: windowsvista / QWindowsVistaStyle
2024-02-25T10:44:54Z System: Windows 11 Version 2009, x86_64-little_endian-llp64
2024-02-25T10:44:54Z Screen: \\.\DISPLAY1 1600x900, pixel ratio=1.0

  1. Apparently, Qt tries to link its default set of static plugins to every suitable target. When building on Windows, it leads to an insane list of plugins:
2024-03-01T19:11:04Z Qt 5.15.10 (static), plugin=windows (dynamic)
2024-03-01T19:11:04Z Static plugins:
2024-03-01T19:11:04Z  QGifPlugin, version 331520
2024-03-01T19:11:04Z  QICNSPlugin, version 331520
2024-03-01T19:11:04Z  QICOPlugin, version 331520
2024-03-01T19:11:04Z  QJpegPlugin, version 331520
2024-03-01T19:11:04Z  QSvgIconPlugin, version 331520
2024-03-01T19:11:04Z  QSvgPlugin, version 331520
2024-03-01T19:11:04Z  QTgaPlugin, version 331520
2024-03-01T19:11:04Z  QTiffPlugin, version 331520
2024-03-01T19:11:04Z  QWbmpPlugin, version 331520
2024-03-01T19:11:04Z  QWebpPlugin, version 331520
2024-03-01T19:11:04Z  QWindowsIntegrationPlugin, version 331520
2024-03-01T19:11:04Z  QWindowsVistaStylePlugin, version 331520

Therefore, in this PR, we prevent Qt from such a behaviour.

@hebasto hebasto marked this pull request as draft February 25, 2024 10:13
@hebasto hebasto marked this pull request as ready for review February 25, 2024 10:16
@hebasto
Copy link
Copy Markdown
Owner Author

hebasto commented Feb 28, 2024

Rebased.

Added the "qt, build: Drop QT_STATICPLUGIN macro" commit, which fixes the wrong plugin linkage report for MSVC static builds:

2024-02-28T13:17:46Z Qt 5.15.10 (static), plugin=windows (dynamic)
2024-02-28T13:17:46Z Static plugins:
...

Copy link
Copy Markdown

@pablomartin4btc pablomartin4btc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tACK 456eb98

git clone --depth=1 --branch 240117-cmake-S https://github.com/hebasto/bitcoin.git
cd bitcoin
cmake -B build
cmake --build build -j $(nproc)
./build/src/qt/bitcoin-qt

debug.log:

2024-02-29T02:58:23Z Bitcoin Core version v26.99.0-7f4babc752c6 (release build)
2024-02-29T02:58:23Z Qt 5.15.3 (dynamic), plugin=xcb (dynamic)
2024-02-29T02:58:23Z No static plugins.
2024-02-29T02:58:23Z Style: fusion / QFusionStyle
2024-02-29T02:58:23Z System: Ubuntu 22.04.3 LTS, x86_64-little_endian-lp64
2024-02-29T02:58:23Z Screen: XWAYLAND0 1920x1200, pixel ratio=1.0

@pablomartin4btc
Copy link
Copy Markdown

with building depends:

2024-02-29T14:32:13Z Bitcoin Core version v26.99.0-7f4babc752c6 (release build)
2024-02-29T14:32:13Z Qt 5.15.11 (static), plugin=xcb (static)
2024-02-29T14:32:13Z Static plugins:
2024-02-29T14:32:13Z  QXcbIntegrationPlugin, version 331520
2024-02-29T14:32:13Z  QXcbIntegrationPlugin, version 331520
2024-02-29T14:32:13Z Style: fusion / QFusionStyle
2024-02-29T14:32:13Z System: Ubuntu 22.04.3 LTS, x86_64-little_endian-lp64
2024-02-29T14:32:13Z Screen: XWAYLAND0 1920x1200, pixel ratio=1.0

@pablomartin4btc
Copy link
Copy Markdown

In the previous comment the debug.log file was from a build performed on diff cmake branch where one could notice the duplication of the static plugin.

So this PR fixes the issue:

2024-02-29T20:12:22Z Bitcoin Core version v26.99.0-456eb98868a1 (release build)
2024-02-29T20:12:22Z Qt 5.15.11 (static), plugin=xcb
2024-02-29T20:12:22Z Static plugins:
2024-02-29T20:12:22Z  QXcbIntegrationPlugin, version 331520
2024-02-29T20:12:22Z Style: fusion / QFusionStyle
2024-02-29T20:12:22Z System: Ubuntu 22.04.3 LTS, x86_64-little_endian-lp64
2024-02-29T20:12:22Z Screen: XWAYLAND0 1920x1200, pixel ratio=1.0

@hebasto
Copy link
Copy Markdown
Owner Author

hebasto commented Mar 1, 2024

@pablomartin4btc

So this PR fixes the issue

Thank you! I've updated the PR description with the notice about this issue.

hebasto added 2 commits March 1, 2024 17:43
Our `QT_STATICPLUGIN` macro is effectively equivalent to the Qt's
`QT_STATIC` macro. No need to handle both of them.
When using CMake, each plugin comes with a C++ stub file that
automatically initializes the static plugin. Consequently, any target
that links against a plugin has this C++ file added to its SOURCES,
which makes the removed code redundant.
Do not interfere with CMake plugin handling.
@hebasto hebasto added the bug Something isn't working label Mar 1, 2024
@hebasto
Copy link
Copy Markdown
Owner Author

hebasto commented Mar 1, 2024

By default, Qt automatically links against a "sane" set of default static plugins. In discussion in-person, @fanquake points out that such a behaviour could be undesirable. That is the case when building on Windows (see the updated PR description).

So, this PR has been reworked to prevent any default plugin being linked implicitly.

@hebasto
Copy link
Copy Markdown
Owner Author

hebasto commented Mar 8, 2024

I'm going to sync the staging branch shortly to accommodate the recent changes in the master branch. It would be nice to have this PR in it before syncing.

cc @TheCharlatan @m3dwards @pablomartin4btc @vasild @sipsorcery

@m3dwards
Copy link
Copy Markdown

m3dwards commented Mar 8, 2024

ACK f3b3928

Tested on Windows. I can see only three distinct plugin logging entries and no duplication

2024-03-08T11:11:52Z Bitcoin Core version v26.99.0-f3b392845aa2 (release build)
2024-03-08T11:11:52Z Qt 5.15.10 (static), plugin=windows
2024-03-08T11:11:52Z Static plugins:
2024-03-08T11:11:52Z  QMinimalIntegrationPlugin, version 331520
2024-03-08T11:11:52Z  QWindowsIntegrationPlugin, version 331520
2024-03-08T11:11:52Z  QWindowsVistaStylePlugin, version 331520
2024-03-08T11:11:52Z Style: windowsvista / QWindowsVistaStyle
2024-03-08T11:11:52Z System: Windows 11 Version 2009, x86_64-little_endian-llp64
2024-03-08T11:11:52Z Screen: \\.\DISPLAY1 1920x1200, pixel ratio=1.0
2024-03-08T11:11:53Z Script verification uses 15 additional threads
2024-03-08T11:11:53Z Using the 'standard' SHA256 implementation

@hebasto hebasto merged this pull request into cmake-staging Mar 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants