build: Fix Qt processing of configure script for depends with DEBUG=1 by hebasto · Pull Request #18298 · bitcoin/bitcoin

hebasto · 2020-03-09T12:58:08Z

This PR:

makes the configure script correctly pickup Qt if depends is built with DEBUG=1:
- for Windows -- fix build: mingw32 issues with Qt if depends is built with DEBUG=1 #19266
- for macOS -- fix build: configure doesn't pickup Qt if depends is built with DEBUG=1 #16391
is an alternative to build: Fix Qt link issue for macOS target with DEBUG=1 #18117 (without downsides)

DrahtBot · 2020-03-09T13:32:39Z

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: Qt 5.15.x #19716 (build: Qt 5.15.x by fanquake)

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 · 2020-03-16T11:40:59Z

Updated fad898c -> 597445c (pr18298.01 -> pr18298.02, diff):

rebased on top of build: Make the help string for --with-gui configure option unambiguous #18361
OP updated

…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

hebasto · 2020-06-13T12:07:54Z

Rebased 597445c -> 49ab294 (pr18298.02 -> pr18298.03) due to the conflict with #18297.

…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

Sjors · 2020-06-16T12:25:28Z

Tested 49ab294 on macOS 10.15.5; works like a charm.

Sjors · 2020-06-29T19:40:31Z

It also works on macOS Big Sur 11.0 beta (after rebase).

cc @fanquake @theuni

hebasto · 2020-08-06T10:10:20Z

@fanquake Mind reviewing this PR insofar as it fixes #16391 submitted by you :)

hebasto · 2020-08-20T20:27:41Z

Rebased 49ab294 -> e25cb1d (pr18298.03 -> pr18298.04) to prevent the merge conflict with #19689.

fanquake · 2020-09-15T08:49:04Z

@hebasto Sorry for the delay. Can you drop the two already-merged commits out of here?

hebasto · 2020-09-15T14:07:26Z

Rebased e25cb1d -> 924900e (pr18298.04 -> pr18298.05).

@fanquake

Can you drop the two already-merged commits out of here?

Done. (it was the intention of the previous rebasing, but something gone wrong)

hebasto · 2020-11-25T14:39:08Z

@fanquake @laanwj Mind adding 22.0 milestone?

fanquake · 2020-12-02T10:07:09Z

@hebasto taking a look

dongcarl · 2020-12-11T15:39:37Z

Concept ACK

theuni · 2020-12-11T15:40:48Z

From IRC discussion today:

Concept ACK. It would be nice if qt gave the option to disable adding the suffix, but since @hebasto says no such option exists, this seems like a reasonable solution.

Now, if depends is built with DEBUG=1, the configure script correctly finds Qt for macOS and Windows.

hebasto · 2021-01-08T08:51:19Z

Rebased 924900e -> 76f52e3 (pr18298.05 -> pr18298.06) due to the conflict with #19683.

…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

… depends with DEBUG=1 Now, if depends is built with DEBUG=1, the configure script correctly finds Qt for macOS and Windows.

fanquake

ACK 76f52e3. Tested native darwin, and darwin/win cross compile with DEBUG=1.

… depend…

…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

… depends with DEBUG=1 Now, if depends is built with DEBUG=1, the configure script correctly finds Qt for macOS and Windows.

hebasto mentioned this pull request Mar 9, 2020

build: Fix Qt link issue for macOS target with DEBUG=1 #18117

Closed

fanquake added the Build system label Mar 9, 2020

This was referenced Mar 9, 2020

build: bitcoin_qt.m4 fixes and improvements #17820

Closed

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

Merged

This was referenced Mar 9, 2020

build: add --enable-isystem and change --enable-werror to enable -Werror #18149

Closed

WIP: Qt: add QML based mobile GUI #16883

Closed

build: Check QT library version #15706

Closed

This was referenced Mar 9, 2020

build: Require pkg-config for all of the hosts #18307

Merged

build: various build system improvements #17768

Closed

This was referenced Mar 10, 2020

build: Optionally enable -Wzero-as-null-pointer-constant #15112

Closed

Build: enable -Wdocumentation via isystem #14920

Closed

hebasto force-pushed the 20200309-depends-debug branch from fad898c to 597445c Compare March 16, 2020 11:36

This was referenced Mar 17, 2020

build: Ambiguous help string for --with-gui configure option #17813

Closed

build: Make the help string for --with-gui configure option unambiguous #18361

Closed

DrahtBot mentioned this pull request Apr 23, 2020

build: optionally skip external warnings #18750

Merged

hebasto mentioned this pull request Jun 13, 2020

build: mingw32 issues with Qt if depends is built with DEBUG=1 #19266

Closed

hebasto force-pushed the 20200309-depends-debug branch from 597445c to 49ab294 Compare June 13, 2020 12:04

hebasto mentioned this pull request Jun 13, 2020

build: configure doesn't pickup Qt if depends is built with DEBUG=1 #16391

Closed

hebasto closed this Jun 13, 2020

hebasto reopened this Jun 13, 2020

hebasto closed this Jun 16, 2020

hebasto reopened this Jun 16, 2020

hebasto force-pushed the 20200309-depends-debug branch from 49ab294 to e25cb1d Compare August 20, 2020 20:25

This was referenced Aug 20, 2020

build: Qt 5.15.x #19716

Closed

build, qt: Add SVG support, and replace bitcoin PNG image with SVG one #19780

Closed

hebasto force-pushed the 20200309-depends-debug branch from e25cb1d to 924900e Compare September 15, 2020 14:03

DrahtBot mentioned this pull request Dec 12, 2020

depends: Pin clang search paths for darwin host #19683

Merged

DrahtBot mentioned this pull request Dec 24, 2020

guix: Build support for macOS #17920

Merged

DrahtBot added the Needs rebase label Jan 8, 2021

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

76f52e3

Now, if depends is built with DEBUG=1, the configure script correctly finds Qt for macOS and Windows.

hebasto force-pushed the 20200309-depends-debug branch from 924900e to 76f52e3 Compare January 8, 2021 08:49

DrahtBot removed the Needs rebase label Jan 8, 2021

xdustinface mentioned this pull request Feb 17, 2021

depends|build: Make sure configuration detects Qt built with DEBUG=1 dashpay/dash#4005

Merged

fanquake approved these changes Mar 3, 2021

View reviewed changes

fanquake merged commit fca3e98 into bitcoin:master Mar 3, 2021

hebasto deleted the 20200309-depends-debug branch March 3, 2021 07:53

sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Mar 3, 2021

Merge bitcoin#18298: build: Fix Qt processing of configure script for…

bd78b3a

… depend…

CodeIsTheKey mentioned this pull request Mar 19, 2022

Windows Build Raptor3um/raptoreum#133

Merged

bitcoin locked as resolved and limited conversation to collaborators Aug 16, 2022

Conversation

hebasto commented Mar 9, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DrahtBot commented Mar 9, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Conflicts

Uh oh!

hebasto commented Mar 16, 2020

Uh oh!

hebasto commented Jun 13, 2020

Uh oh!

Sjors commented Jun 16, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Sjors commented Jun 29, 2020

Uh oh!

hebasto commented Aug 6, 2020

Uh oh!

hebasto commented Aug 20, 2020

Uh oh!

fanquake commented Sep 15, 2020

Uh oh!

hebasto commented Sep 15, 2020

Uh oh!

hebasto commented Nov 25, 2020

Uh oh!

fanquake commented Dec 2, 2020

Uh oh!

dongcarl commented Dec 11, 2020

Uh oh!

theuni commented Dec 11, 2020

Uh oh!

hebasto commented Jan 8, 2021

Uh oh!

fanquake left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

hebasto commented Mar 9, 2020 •

edited

Loading

DrahtBot commented Mar 9, 2020 •

edited

Loading

Sjors commented Jun 16, 2020 •

edited

Loading