Skip to content

Conversation

@theuni
Copy link
Member

@theuni theuni commented Jul 7, 2016

For 0.13, I'd prefer to keep the requirements the same as before: All platforms require pkg-config except for Windows.

For now, I believe I've fixed up the current issues and regressions. Fixes: #8228, replaces #8242.

Post-0.13, 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)

theuni added 2 commits July 7, 2016 14:15
The non-pkg-config case can't use pkg-config to check the version.

Also, make sure that the check is properly guarded in the case of missing
pkg-config macros.
- guard PKG_PROG_PKG_CONFIG with an m4_ifdef. If not building for windows,
  require it
- add nops as necessary in case the ifdef reduces the if/then to nothing
- AC_SUBST some missing _LIBS. These were split out over time, but not all were
  properly substituted. They continued to work if pkg-config is installed
  because it does the AC_SUBST itself
@maflcko maflcko added this to the 0.13.0 milestone Jul 7, 2016
@jonasschnelli
Copy link
Contributor

@laanwj
Copy link
Member

laanwj commented Jul 8, 2016

Works for me - retried #8228 and get an error now, as expected:

configure: error: PKG_PROG_PKG_CONFIG macro not found. Please install pkg-config and re-run autogen.sh.

@laanwj laanwj merged commit b556bed into bitcoin:master Jul 8, 2016
laanwj added a commit that referenced this pull request Jul 8, 2016
b556bed build: fix Windows builds without pkg-config (Cory Fields)
0c928cb build: Fix Qt5PlatformSupport check without pkg-config (Cory Fields)
zkbot added a commit to zcash/zcash that referenced this pull request Dec 1, 2017
Build system improvements

Includes commits cherry-picked from the following upstream PRs:

- bitcoin/bitcoin#6978
  - Only the first commit (second is for QT)
- bitcoin/bitcoin#7059
- bitcoin/bitcoin#7603
  - Only the first commit (the rest are for QT)
- bitcoin/bitcoin#7954
- bitcoin/bitcoin#8314
  - Only the second commit (first is for QT)
- bitcoin/bitcoin#8504
  - Only the first commit (second was undoing something we didn't have)
- bitcoin/bitcoin#8520
- bitcoin/bitcoin#8563
- bitcoin/bitcoin#8249
- bitcoin/bitcoin#9156
- bitcoin/bitcoin#9831
- bitcoin/bitcoin#9789
- bitcoin/bitcoin#10766

Part of #2074.
zkbot added a commit to zcash/zcash that referenced this pull request Dec 15, 2017
Build system improvements

Includes commits cherry-picked from the following upstream PRs:

- bitcoin/bitcoin#6978
  - Only the first commit (second is for QT)
- bitcoin/bitcoin#7059
- bitcoin/bitcoin#7603
  - Only the first commit (without the `BITCOIN_QT_BIN` variable; the rest are for QT)
- bitcoin/bitcoin#7954
- bitcoin/bitcoin#8314
  - Only the second commit (first is for QT)
- bitcoin/bitcoin#8504
  - Only the first commit (second was undoing something we didn't have)
- bitcoin/bitcoin#8520
- bitcoin/bitcoin#8563
- bitcoin/bitcoin#8249
- bitcoin/bitcoin#9156
- bitcoin/bitcoin#9831
- bitcoin/bitcoin#9789
- bitcoin/bitcoin#10766

Part of #2074.
codablock pushed a commit to codablock/dash that referenced this pull request Dec 28, 2017
b556bed build: fix Windows builds without pkg-config (Cory Fields)
0c928cb build: Fix Qt5PlatformSupport check without pkg-config (Cory Fields)
kotodev pushed a commit to koto-dev/koto.old that referenced this pull request Jan 25, 2018
Build system improvements

Includes commits cherry-picked from the following upstream PRs:

- bitcoin/bitcoin#6978
  - Only the first commit (second is for QT)
- bitcoin/bitcoin#7059
- bitcoin/bitcoin#7603
  - Only the first commit (without the `BITCOIN_QT_BIN` variable; the rest are for QT)
- bitcoin/bitcoin#7954
- bitcoin/bitcoin#8314
  - Only the second commit (first is for QT)
- bitcoin/bitcoin#8504
  - Only the first commit (second was undoing something we didn't have)
- bitcoin/bitcoin#8520
- bitcoin/bitcoin#8563
- bitcoin/bitcoin#8249
- bitcoin/bitcoin#9156
- bitcoin/bitcoin#9831
- bitcoin/bitcoin#9789
- bitcoin/bitcoin#10766

Part of #2074.
renium9 added a commit to koto-dev/koto.old that referenced this pull request Feb 6, 2018
Build system improvements

Includes commits cherry-picked from the following upstream PRs:

- bitcoin/bitcoin#6978
  - Only the first commit (second is for QT)
- bitcoin/bitcoin#7059
- bitcoin/bitcoin#7603
  - Only the first commit (without the `BITCOIN_QT_BIN` variable; the rest are for QT)
- bitcoin/bitcoin#7954
- bitcoin/bitcoin#8314
  - Only the second commit (first is for QT)
- bitcoin/bitcoin#8504
  - Only the first commit (second was undoing something we didn't have)
- bitcoin/bitcoin#8520
- bitcoin/bitcoin#8563
- bitcoin/bitcoin#8249
- bitcoin/bitcoin#9156
- bitcoin/bitcoin#9831
- bitcoin/bitcoin#9789
- bitcoin/bitcoin#10766

Part of #2074.

# Conflicts:
#	configure.ac
#	src/Makefile.am
#	src/Makefile.gtest.include
#	src/Makefile.test.include
#	zcutil/build.sh
andvgal pushed a commit to energicryptocurrency/gen2-energi that referenced this pull request Jan 6, 2019
b556bed build: fix Windows builds without pkg-config (Cory Fields)
0c928cb build: Fix Qt5PlatformSupport check without pkg-config (Cory Fields)
fanquake added a commit that referenced this pull request Jun 13, 2020
…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
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Jun 13, 2020
…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
xdustinface pushed a commit to xdustinface/dash that referenced this pull request Feb 17, 2021
…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
gades pushed a commit to cosanta/cosanta-core that referenced this pull request Jun 27, 2021
…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
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

No error reporting when pkg-config missing in depends build

4 participants