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

hebasto commented at 11:18 am on March 16, 2020: member

This PR changes the help string for --with-gui option of the configure script in the following way:

explicitly adds “yes” value
explicitly states that --with-gui is equivalent to --with-gui=qt5
adds check for --with-gui values: the configure script raises an error if a value in --with-gui=value is not expected
refactors to make ongoing changes (#18298) clear and easy to review

On master (3d28c886f077ce22fb7755fe9ec1f4e08d3d4a62):

0$ ./configure --help | grep -A 1 'with-gui\['
1  --with-gui[=no|qt5|auto]
2                          build bitcoin-qt GUI (default=auto)

With this PR:

0$ ./configure --help | grep -A 2 'with-gui\['
1  --with-gui[=yes|no|auto|qt5]
2                          build bitcoin-qt GUI (default is auto; --with-gui
3                          and --with-gui=qt5 are equivalent to --with-gui=yes)

Also the QT_LIB_PREFIX variable is removed for code simplicity and readability.

This PR does not change behavior of the configure script for known values of the --with-gui option, and raises an error otherwise.

Fix #17813 Based on #18297 (492971de35bab26346545f68365872211f458b00..8a26848c460160e1279f26bc413f693a34e33b9d)

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

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

build: Overhaul --with-gui option

This change removes discrepancies between the help string and actual
behavior of the configure script. Also "yes" value is described
explicitly.

1fe92ee955

build: Drop QT_LIB_PREFIX variable from bitcoin_qt.m4 3bca66e5fa

fanquake added the label Build system on Mar 16, 2020

DrahtBot commented at 2:49 pm on March 16, 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 renamed this:
~~build: Overhaul --with-gui option of configure script~~
build: Make the help string for --with-gui configure option unambiguous
on Mar 17, 2020

hebasto commented at 12:03 pm on March 17, 2020: member

The PR description has been updated on @ryanofsky’s request.

ryanofsky commented at 12:46 pm on March 17, 2020: member

Thanks for updating, new description makes this much more understandable. Two things I don’t understand:

Unclear what “adds check for –with-gui values” is means. Is there a new warning or error?
Paragraph “This PR does not change behavior. The configure script warns…” is confusing because it’s unclear whether the warning is new or old. If this isn’t changing warnings and errors, then I don’t think there is a reason to mention them. If this is changing warnings and errors, the differences should be listed in the list of changes at the top of the PR, not in a bottom paragraph beginning with “This PR does not change behavior.”

hebasto commented at 1:45 pm on March 17, 2020: member

Unclear what “adds check for –with-gui values” is means. Is there a new warning or error?

There is a new error if value is not expected. PR description has been updated.

Paragraph “This PR does not change behavior. The configure script warns…” is confusing because it’s unclear whether the warning is new or old. If this isn’t changing warnings and errors, then I don’t think there is a reason to mention them. If this is changing warnings and errors, the differences should be listed in the list of changes at the top of the PR, not in a bottom paragraph beginning with “This PR does not change behavior.”

PR description has been updated.

luke-jr commented at 11:21 pm on April 22, 2020: member

Rather not. Isn’t Qt 6 around the corner?

hebasto commented at 12:45 pm on June 13, 2020: member

Closing until Qt 6.

hebasto closed this on Jun 13, 2020

DrahtBot locked this on Feb 15, 2022

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

Conflicts