build: bitcoin_qt.m4 fixes and improvements #17820

hebasto · 2019-12-28T20:55:50Z

Part A

There are two occurrences of AC_DEFINE(QT_STATICPLUGIN...) macro:

Lines 320 to 322 in b931f61

    
             if test "x$bitcoin_cv_static_qt" = xyes; then 
        
               AC_DEFINE(QT_STATICPLUGIN, 1, [Define this symbol for static Qt plugins]) 
        
             fi

and

bitcoin/build-aux/m4/bitcoin_qt.m4

Lines 115 to 118 in b931f61

    
             _BITCOIN_QT_IS_STATIC 
        
             if test "x$bitcoin_cv_static_qt" = xyes; then 
        
               _BITCOIN_QT_FIND_STATIC_PLUGINS 
        
               AC_DEFINE(QT_STATICPLUGIN, 1, [Define this symbol if qt plugins are static])

The former could be removed safely.

Part B (from #17836)

It is expected that --with-gui behaves the same as --with-gui=auto.

But if Qt dependencies not found, configure --with-gui fails rather than warns. See: #17813

Ref: laanwj's comment from #6938

This PR fixes #17813

Part C (from #17837)

This PR removes code that repeats three times.
Also currently on master (8830cb5) some parts are brittle:

bitcoin/build-aux/m4/bitcoin_qt.m4

Line 290 in 8830cb5

#if QT_VERSION_MINOR < 8

bitcoin/build-aux/m4/bitcoin_qt.m4

Line 387 in 8830cb5

#if QT_VERSION < 0x050600 || QT_VERSION_MINOR < 6

Other small improvements:

"qt" -> "Qt" in messages and comments
fixed m4 escaping
fixed a warning:

$ cat config.log | grep -A 2 'extra tokens'
conftest.cpp:76:28: warning: extra tokens at end of #ifndef directive
         #ifndef QT_VERSION OR QT_VERSION_STR
                            ^~

QT_STATICPLUGIN is set in BITCOIN_QT_CONFIGURE macro.

DrahtBot · 2019-12-28T22:05:03Z

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

build: Use pkg-config in BITCOIN_QT_CONFIGURE for all hosts including Windows #18297 (build: Use pkg-config in BITCOIN_QT_CONFIGURE for all hosts including Windows by hebasto)
build: Check QT library version #15706 (build: Check QT library version by lucayepa)

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.

DrahtBot · 2019-12-29T16:37:19Z

Gitian builds

File	commit `b931f61` (master)	commit af67e3ae055d29e09c34c48f794a588a80099e12 (master and this pull)
bitcoin-0.19.99-aarch64-linux-gnu-debug.tar.gz	`bd435f6124e72f0c...`	`dbd6755e006e2af5...`
bitcoin-0.19.99-aarch64-linux-gnu.tar.gz	`41d2c0cb10194046...`	`6ebc2fd617875d16...`
bitcoin-0.19.99-arm-linux-gnueabihf-debug.tar.gz	`5b08cc3321086263...`	`8594bdc78e8662e5...`
bitcoin-0.19.99-arm-linux-gnueabihf.tar.gz	`3ac49a265aff0310...`	`80cc4634a529de3f...`
bitcoin-0.19.99-i686-pc-linux-gnu-debug.tar.gz	`7d287dda279f01b7...`	`48dfd840d6eba881...`
bitcoin-0.19.99-i686-pc-linux-gnu.tar.gz	`cb49ef13cd296474...`	`28d77fdf86d14fea...`
bitcoin-0.19.99-osx-unsigned.dmg	`549ad30b82cbeba7...`	`7c837c6ba9f84a6d...`
bitcoin-0.19.99-osx64.tar.gz	`bfdb4994c18efe8d...`	`81c1724c72cbd655...`
bitcoin-0.19.99-riscv64-linux-gnu-debug.tar.gz	`d675e9ab03aa713f...`	`92b7b70e6c2f3a6a...`
bitcoin-0.19.99-riscv64-linux-gnu.tar.gz	`6476b681fc8a7542...`	`5193694a1e7f860c...`
bitcoin-0.19.99-win64-debug.zip	`1df9ceed90aedebb...`	`89a3db8577dc6421...`
bitcoin-0.19.99-win64-setup-unsigned.exe	`733aea8d6afd08f6...`	`9386a8e80451a417...`
bitcoin-0.19.99-win64.zip	`57b56968540bfe59...`	`367eb32d382fe06d...`
bitcoin-0.19.99-x86_64-linux-gnu-debug.tar.gz	`4a93ba7677cd6a4f...`	`e650c3c06076efc5...`
bitcoin-0.19.99-x86_64-linux-gnu.tar.gz	`daa0bc9b20804016...`	`04efa75f071e884b...`
bitcoin-0.19.99.tar.gz	`564530cb6e3a2812...`	`d2caf9b3b9d8ed7f...`
bitcoin-core-linux-0.20-res.yml	`dde346c55ae15b86...`	`648f2bbc0dc0056d...`
bitcoin-core-osx-0.20-res.yml	`a99a6702f12b7e90...`	`7d4cde6e19b21ab4...`
bitcoin-core-win-0.20-res.yml	`022eb186471e2949...`	`f03cc2ebf0a55a09...`
linux-build.log	`dd0ec95099445d40...`	`4b0a7de2e3f084c3...`
osx-build.log	`30a8c6bd679d3bb6...`	`f6c447f8637a0366...`
win-build.log	`9719b499e8176a68...`	`5e9f98a7e7381d05...`
bitcoin-core-linux-0.20-res.yml.diff		`864a95a7a4459617...`
bitcoin-core-osx-0.20-res.yml.diff		`e788a5fb731b0dc6...`
bitcoin-core-win-0.20-res.yml.diff		`14d8cebe46707d67...`
linux-build.log.diff		`d613568256860f58...`
osx-build.log.diff		`1645dd79c95a8f8a...`
win-build.log.diff		`c49352b653894f02...`

hebasto · 2019-12-30T18:38:46Z

The related @theuni's comment:

... we define QT_STATICPLUGIN elsewhere. Let's just delete this.

Now --with-gui without argument behaves the same as --with-gui=auto, i.e., if Qt is unavailable, it rather warns than fails.

hebasto · 2019-12-31T01:21:04Z

Two related PRs are combined into this one: #17836 and #17837.

The title and OP have been updated.

DrahtBot · 2020-01-01T03:22:55Z

Gitian builds

File	commit `8830cb5` (master)	commit 493aa142c0b8ad4f95ba53afc8d96e91bd04ac7e (master and this pull)
bitcoin-0.19.99-aarch64-linux-gnu-debug.tar.gz	`ee3afaebcccbf974...`	`ac6a9a969b9c2be5...`
bitcoin-0.19.99-aarch64-linux-gnu.tar.gz	`53eb839c3839f32d...`	`47b4623947730dc6...`
bitcoin-0.19.99-arm-linux-gnueabihf-debug.tar.gz	`62d69db284fcebea...`	`805b0685af63d38f...`
bitcoin-0.19.99-arm-linux-gnueabihf.tar.gz	`ad569636143408b8...`	`0893fb1f64c8ac29...`
bitcoin-0.19.99-i686-pc-linux-gnu-debug.tar.gz	`ee96491a66eacffd...`	`64d6100d7e602e02...`
bitcoin-0.19.99-i686-pc-linux-gnu.tar.gz	`4d4a72b2f7cee148...`	`12ff07229e16aa1b...`
bitcoin-0.19.99-osx-unsigned.dmg	`3d6c509abd99dc13...`	`13b0c4094186a7f6...`
bitcoin-0.19.99-osx64.tar.gz	`ce1bc5f99820547c...`	`74c1e75579823eed...`
bitcoin-0.19.99-riscv64-linux-gnu-debug.tar.gz	`feedb697bd7409f1...`	`b91c3308ab80bce7...`
bitcoin-0.19.99-riscv64-linux-gnu.tar.gz	`50c8d4d1a0714689...`	`c35e1dbf91b4c609...`
bitcoin-0.19.99-win64-debug.zip	`171779a3f1818166...`	`719ecfe7aa1e28b7...`
bitcoin-0.19.99-win64-setup-unsigned.exe	`6e0c7959ceda4156...`	`9d1b1970d6ac29f6...`
bitcoin-0.19.99-win64.zip	`155e546e8eee35b0...`	`e9e5459c91f6d0ba...`
bitcoin-0.19.99-x86_64-linux-gnu-debug.tar.gz	`118716702d22fbe8...`	`eac8efc230001a6c...`
bitcoin-0.19.99-x86_64-linux-gnu.tar.gz	`120a0036a86ff050...`	`b6073c385487e1dc...`
bitcoin-0.19.99.tar.gz	`28be22863a966fd9...`	`335222e52966f5f5...`
bitcoin-core-linux-0.20-res.yml	`e89e71495e718e4d...`	`945e76fa5ec9f475...`
bitcoin-core-osx-0.20-res.yml	`2571488485be95ea...`	`83bec79a0065eb41...`
bitcoin-core-win-0.20-res.yml	`cdfab5ec62c485d1...`	`6df3e62a0b98f809...`
linux-build.log	`808b856809ef9fe5...`	`20dca3e543df2422...`
osx-build.log	`d0d966aa02921336...`	`52dc85cbe5bdb5c5...`
win-build.log	`d352cbdd46249391...`	`18ce8e5b8612e753...`
bitcoin-core-linux-0.20-res.yml.diff		`add0f55791ebdc83...`
bitcoin-core-osx-0.20-res.yml.diff		`0d5cc7bcbd9c5543...`
bitcoin-core-win-0.20-res.yml.diff		`572f91045795c987...`
linux-build.log.diff		`b74ab458637b55c6...`
osx-build.log.diff		`ca7667e9bda08d92...`
win-build.log.diff		`046a919aad0f8380...`

hebasto · 2020-03-09T13:00:37Z

Closed in favor of #18298.

…osts including Windows 8a26848 build: Fix m4 escaping (Hennadii Stepanov) 9123ec1 build: Remove extra tokens warning (Hennadii Stepanov) fded4f4 build: Remove duplicated QT_STATICPLUGIN define (Hennadii Stepanov) 05a93d5 build: Fix indentation in bitcoin_qt.m4 (Hennadii Stepanov) ddbb419 build: Use pkg-config in BITCOIN_QT_CONFIGURE for all hosts (Hennadii Stepanov) 492971d build: Fix mingw pkgconfig file and dependency naming (Hennadii Stepanov) Pull request description: 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](#8314 (comment)) 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. ACKs for top commit: theuni: Code review ACK 8a26848 dongcarl: Code Review ACK 8a26848 laanwj: Code review ACK 8a26848 Tree-SHA512: 3b25990934b939121983df7707997b31d61063b1207d909f539d69494c7cb85212f353092956d09ecffebb9fef28b869914dd1216a596d102fcb9744bb5487f7

…r all hosts including Windows 8a26848 build: Fix m4 escaping (Hennadii Stepanov) 9123ec1 build: Remove extra tokens warning (Hennadii Stepanov) fded4f4 build: Remove duplicated QT_STATICPLUGIN define (Hennadii Stepanov) 05a93d5 build: Fix indentation in bitcoin_qt.m4 (Hennadii Stepanov) ddbb419 build: Use pkg-config in BITCOIN_QT_CONFIGURE for all hosts (Hennadii Stepanov) 492971d build: Fix mingw pkgconfig file and dependency naming (Hennadii Stepanov) Pull request description: 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](bitcoin#8314 (comment)) 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 bitcoin#16391) - for Windows host (regression) The fix is ~on its way~ submitted in bitcoin#18298 (as a followup). Also this PR picks some small improvements from bitcoin#17820. ACKs for top commit: theuni: Code review ACK 8a26848 dongcarl: Code Review ACK 8a26848 laanwj: Code review ACK 8a26848 Tree-SHA512: 3b25990934b939121983df7707997b31d61063b1207d909f539d69494c7cb85212f353092956d09ecffebb9fef28b869914dd1216a596d102fcb9744bb5487f7

hebasto added 2 commits December 28, 2019 22:33

build: Remove duplicated QT_STATICPLUGIN define

3fb692f

QT_STATICPLUGIN is set in BITCOIN_QT_CONFIGURE macro.

doc: Proper capitalize terms

13a81d8

fanquake added Build system Needs gitian build labels Dec 28, 2019

fanquake changed the title ~~build, qt: Remove duplicated AC_DEFINE(QT_STATICPLUGIN...) macro~~ build: Remove duplicated AC_DEFINE(QT_STATICPLUGIN...) macro Dec 28, 2019

DrahtBot removed the Needs gitian build label Dec 29, 2019

hebasto added 4 commits December 31, 2019 03:05

build: Fix --with-gui configure option without arg

cd70b72

Now --with-gui without argument behaves the same as --with-gui=auto, i.e., if Qt is unavailable, it rather warns than fails.

build: Fix m4 escaping

795b493

build: Refactor out Qt version check macro

5513fd1

build: Remove extra tokens warning

f217456

This was referenced Dec 31, 2019

build: Fix --with-gui configure option without argument #17836

Closed

build: Refactor out Qt version check macro #17837

Closed

fanquake added the Needs gitian build label Dec 31, 2019

hebasto changed the title ~~build: Remove duplicated AC_DEFINE(QT_STATICPLUGIN...) macro~~ build: bitcoin_qt.m4 fixes and improvements Dec 31, 2019

DrahtBot removed the Needs gitian build label Jan 1, 2020

DrahtBot mentioned this pull request Feb 11, 2020

build: Check QT library version #15706

Closed

This was referenced Mar 9, 2020

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

Merged

build: Fix Qt processing of configure script for depends with DEBUG=1 #18298

Merged

hebasto closed this Mar 9, 2020

bitcoin locked as resolved and limited conversation to collaborators Feb 15, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

build: bitcoin_qt.m4 fixes and improvements #17820

build: bitcoin_qt.m4 fixes and improvements #17820

Uh oh!

hebasto commented Dec 28, 2019 •

edited

Loading

Uh oh!

DrahtBot commented Dec 28, 2019 •

edited

Loading

Uh oh!

DrahtBot commented Dec 29, 2019

Uh oh!

hebasto commented Dec 30, 2019

Uh oh!

hebasto commented Dec 31, 2019

Uh oh!

DrahtBot commented Jan 1, 2020

Uh oh!

hebasto commented Mar 9, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	if test "x$bitcoin_cv_static_qt" = xyes; then
	AC_DEFINE(QT_STATICPLUGIN, 1, [Define this symbol for static Qt plugins])
	fi

	_BITCOIN_QT_IS_STATIC
	if test "x$bitcoin_cv_static_qt" = xyes; then
	_BITCOIN_QT_FIND_STATIC_PLUGINS
	AC_DEFINE(QT_STATICPLUGIN, 1, [Define this symbol if qt plugins are static])

build: bitcoin_qt.m4 fixes and improvements #17820

build: bitcoin_qt.m4 fixes and improvements #17820

Uh oh!

Conversation

hebasto commented Dec 28, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DrahtBot commented Dec 28, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Conflicts

Uh oh!

DrahtBot commented Dec 29, 2019

Gitian builds

Uh oh!

hebasto commented Dec 30, 2019

Uh oh!

hebasto commented Dec 31, 2019

Uh oh!

DrahtBot commented Jan 1, 2020

Gitian builds

Uh oh!

hebasto commented Mar 9, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

hebasto commented Dec 28, 2019 •

edited

Loading

DrahtBot commented Dec 28, 2019 •

edited

Loading