build: Use pkg-config in BITCOIN_QT_CONFIGURE for all hosts including Windows

hebasto commented at 2:00 am on March 9, 2020: member

This PR makes bitcoin_qt.m4 to use pkg-config for all hosts and removes non-pkg-config paths from it. This is a step towards the idea which was clear stated by Cory Fields:

I believe the consensus is to treat Windows like the others and require pkg-config across the board. We can drop all of the non-pkg-config paths, and simply AC_REQUIRE(PKG_PROG_PKG_CONFIG)

There are two unsolved problems with this PR. If depends is built with DEBUG=1 the configure script fails to pickup Qt:

for macOS host (similar to, but not the same as #16391)
for Windows host (regression)

The fix is ~~on its way~~ submitted in #18298 (as a followup).

Also this PR picks some small improvements from #17820.

build: Fix mingw pkgconfig file and dependency naming

This change adds the correct suffix to debug mode .pc filenames for
MinGW and also to the Qt libraries listed in the `Requires` field.
The filename adjustment fixes the accidental overwriting of release
mode .pc files with the debug mode variant which required the wrong
variant of the libraries when `debug_and_release` is active.
Note that macOS also supports the `debug_and_release' configuration
but may use the regular library names together with DYLD_IMAGE_SUFFIX.
Creation of *_debug.pc files is turned off as they're identical to their
non-debug counterparts.
More info:
- QTBUG-4155
- Qt commit a0d8fb4ac3cb7bafdb39f340055eacee4f957513

492971de35

fanquake added the label Build system on Mar 9, 2020

hebasto commented at 2:05 am on March 9, 2020: member

May I ask for gitian builds?

fanquake added the label Needs gitian build on Mar 9, 2020

DrahtBot commented at 5:30 am on March 9, 2020: member

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#18750 (build: optionally skip external warnings by vasild)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

hebasto force-pushed on Mar 9, 2020

hebasto commented at 9:19 am on March 9, 2020: member

Updated f71b56047bc319184c1f771e0757faeff26cf242 -> e7cfcf79ea9992e2b844ffdcd64ee02af4f9fc07 (pr18297.01 -> pr18297.02, compare):

removed redundant include and lib tracking for MinGW in config.site

DrahtBot removed the label Needs gitian build on Mar 11, 2020

build: Use pkg-config in BITCOIN_QT_CONFIGURE for all hosts

This change adds to the BITCOIN_QT_CONFIGURE script ability to use
pkg-config for MinGW. All of the non-pkg-config paths are removed as
needless.
If depends is built with DEBUG=1 the configure script fails to pickup
Qt:
- for macOS host (similar, but not the same as issue 16391)
- for Windows host (regression)

ddbb419310

build: Fix indentation in bitcoin_qt.m4 05a93d5d96

build: Remove duplicated QT_STATICPLUGIN define

QT_STATICPLUGIN is defined in BITCOIN_QT_CONFIGURE macro.

fded4f48c3

build: Remove extra tokens warning 9123ec15db

build: Fix m4 escaping 8a26848c46

hebasto force-pushed on Mar 16, 2020

hebasto commented at 9:14 am on March 16, 2020: member

Updated e7cfcf79ea9992e2b844ffdcd64ee02af4f9fc07 -> 8a26848c460160e1279f26bc413f693a34e33b9d (pr18297.02 -> pr18297.03, diff):

removed unused now internal functions _BITCOIN_QT_CHECK_QT5 and _BITCOIN_QT_CHECK_QT58

MarcoFalke deleted a comment on Mar 19, 2020

MarcoFalke added the label Needs gitian build on Mar 19, 2020

DrahtBot removed the label Needs gitian build on Mar 20, 2020

hebasto commented at 1:54 pm on March 24, 2020: member

Just noted that some dead code remains:https://github.com/bitcoin/bitcoin/blob/8a26848c460160e1279f26bc413f693a34e33b9d/build-aux/m4/bitcoin_qt.m4#L299-L301

The bitcoin_cv_qt58 variable is always unset (see #17821).

This code path runs for static Qt builds, including gitian builds. On xenial with Qt 5.5.1 (the minimal supported version) builds against system libs are ok (tested locally + Travis).

Going to remove this dead code in a followup or with other changes requested by reviewers.

laanwj added this to the "Blockers" column in a project

laanwj commented at 2:13 pm on June 4, 2020: member

Code review ACK 8a26848c460160e1279f26bc413f693a34e33b9d

laanwj commented at 2:15 pm on June 4, 2020: member

This makes significant changes to the build system and introduces a Qt patch. @dongcarl @theuni can you take a look at this plase?

MarcoFalke deleted a comment on Jun 4, 2020

MarcoFalke added the label Needs gitian build on Jun 4, 2020

MarcoFalke added the label Needs Guix build on Jun 4, 2020

theuni commented at 9:53 pm on June 5, 2020: member

@laanwj Thanks for the ping, will review thoroughly in the next few days.

DrahtBot commented at 6:28 am on June 7, 2020: member

Gitian builds

File	commit 17cfa52d380673eaec3d428383b163ca4eb83f04(master)	commit 61fe2c614d38ecf678eb001d7eb4acaedd3ec5dd(master and this pull)
*-aarch64-linux-gnu-debug.tar.gz	`1c72e8e4fc35859a...`	`89515a9227643763...`
*-aarch64-linux-gnu.tar.gz	`b30e3199e8acfa25...`	`4fc7b7ba50988d45...`
*-arm-linux-gnueabihf-debug.tar.gz	`8e836573afc2d1ae...`	`a508ed33f3e55610...`
*-arm-linux-gnueabihf.tar.gz	`be966fe2599f3c9f...`	`b2c1523cdf385c79...`
*-osx-unsigned.dmg	`db0084dc80753606...`	`b48ee8f744c28379...`
*-osx64.tar.gz	`825419899dbec827...`	`a1da373d7ed6caac...`
*-riscv64-linux-gnu-debug.tar.gz	`becb75bee8ee9eb5...`	`67eb20fb8eac295e...`
*-riscv64-linux-gnu.tar.gz	`2de2cde85b0c26bd...`	`3c230ab7cab5656a...`
*-win64-debug.zip	`d7b65f642b3fb503...`	`bcbc103170e4f8d2...`
*-win64-setup-unsigned.exe	`9810912dcfaa9c1a...`	`13ebeaa3b21eac35...`
*-win64.zip	`adb8a6aa49116574...`	`cee1bbe4ceab905e...`
*-x86_64-linux-gnu-debug.tar.gz	`fdd050cd554445f4...`	`03a6912ab8474bfa...`
*-x86_64-linux-gnu.tar.gz	`70a422b24c1d11a2...`	`1ac74df3aa638f7f...`
*.tar.gz	`7ea084c8d5c43474...`	`0c762d096a48f3e4...`
bitcoin-core-linux-0.21-res.yml	`bf589606c87f1ef8...`	`6ba31c52f2426198...`
bitcoin-core-osx-0.21-res.yml	`d882ce2c1d471957...`	`a805b41ee34a7e27...`
bitcoin-core-win-0.21-res.yml	`e3b7dec01dc025d2...`	`6760ee0762cd4a74...`
linux-build.log	`18119261df2a62e9...`	`ad266b3f1d65acc8...`
osx-build.log	`86f016a18d174458...`	`b27a527d34c3350d...`
win-build.log	`a256aaeed3dd26c2...`	`d6ae318cec88d163...`
bitcoin-core-linux-0.21-res.yml.diff		`44023913fc31ddeb...`
bitcoin-core-osx-0.21-res.yml.diff		`98f1b79aa8eb8eac...`
bitcoin-core-win-0.21-res.yml.diff		`6078c048e92d93dc...`
linux-build.log.diff		`fc46cecc6125f163...`
osx-build.log.diff		`d6a15b82e2abf05e...`
win-build.log.diff		`e7076213c81ffb67...`

DrahtBot removed the label Needs gitian build on Jun 7, 2020

in depends/patches/qt/fix_qt_pkgconfig.patch:8 in 492971de35 outdated

0@@ -1,11 +1,23 @@
1 --- old/qtbase/mkspecs/features/qt_module.prf
2 +++ new/qtbase/mkspecs/features/qt_module.prf
3-@@ -245,7 +245,7 @@
4+@@ -264,7 +264,7 @@
5  load(qt_targets)
6  
7  # this builds on top of qt_common
8 -!internal_module:!lib_bundle:if(unix|mingw) {
9-+unix|mingw {

dongcarl commented at 5:15 pm on June 12, 2020:

Do you or @jonasschnelli or @theuni understand why this original patch was necessary?

theuni commented at 6:07 pm on June 12, 2020:

@dongcarl I don’t recall, but from a quick glance, it looks like it was necessary to get qt to generate pkg-config files for internal/bundled libs (png/harfbuzz/pcre, etc).

dongcarl commented at 5:15 pm on June 12, 2020: member

Code Review ACK 8a26848c460160e1279f26bc413f693a34e33b9d

theuni approved

theuni commented at 6:14 pm on June 12, 2020: member

Code review ACK 8a26848c460160e1279f26bc413f693a34e33b9d

fanquake merged this on Jun 13, 2020

fanquake closed this on Jun 13, 2020

hebasto deleted the branch on Jun 13, 2020

sidhujag referenced this in commit c8addbe7ea on Jun 13, 2020

fanquake removed this from the "Blockers" column in a project

fanquake referenced this in commit 7d9008f43e on Jul 3, 2020

MarcoFalke removed the label Needs Guix build on Jul 29, 2020

fanquake referenced this in commit 38c13a4a60 on Aug 24, 2020

sidhujag referenced this in commit dafb919f5c on Aug 24, 2020

xdustinface referenced this in commit e2d3235eb2 on Feb 17, 2021

xdustinface referenced this in commit 3e8687603d on Feb 17, 2021

gades referenced this in commit ac855be7fd on Jun 27, 2021

gades referenced this in commit 0e12d032a0 on Jun 27, 2021

DrahtBot locked this on Feb 15, 2022

build: Use pkg-config in BITCOIN_QT_CONFIGURE for all hosts including Windows #18297

Conflicts

Gitian builds