-
Notifications
You must be signed in to change notification settings - Fork 38.7k
Fix pkg-config issues for 0.13 #8314
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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
Contributor
|
Gitian was happy with this PR: https://bitcoin.jonasschnelli.ch/pulls/8314/ |
Member
|
Works for me - retried #8228 and get an error now, as expected: |
laanwj
added a commit
that referenced
this pull request
Jul 8, 2016
24 tasks
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.
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
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
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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)