build: Add ability to build qt in depends with -stdlib=libc++ #22814
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
Guix builds: |
|
Also #22815 (comment) |
|
GUIX hashes mine match @hebasto: |
fanquake
reviewed
Sep 28, 2021
depends/packages/qt.mk
Outdated
| @@ -140,7 +140,11 @@ $(package)_config_opts_linux += -no-feature-vulkan | |||
| $(package)_config_opts_linux += -dbus-runtime | |||
| $(package)_config_opts_arm_linux += -platform linux-g++ -xplatform bitcoin-linux-g++ | |||
| $(package)_config_opts_i686_linux = -xplatform linux-g++-32 | |||
| ifneq (,$(findstring clang++ -stdlib=libc++,$($(1)_cxx))) | |||
There was a problem hiding this comment.
As mentioned in the comment from #22815 (comment):
Looking only at the argument (which should directly follow the compiler name) looks kind of fragile and will not catch every case where someone links against libc++.
To expand on that, as-is this also wont work when you're using a versioned clang, i.e clang++-12, which we often do in our CIs.
7ae424b to
33796a9
Compare
|
Updated 7ae424b -> 33796a9 (pr22814.01 -> pr22814.02): |
Guix builds: |
|
Also this PR fixes the |
|
Concept ACK |
|
contributing guix hashes, mine match @hebasto second round of hashes: |
fanquake
approved these changes
Dec 30, 2021
There was a problem hiding this comment.
ACK 33796a9 - While this sort of string matching is fragile, I think the risk of this causing any actual issues is low.
Anyone who was previously building with -stdlib=libc++ in their CXXFLAGS (us in CI) now gets a working Qt build.
Anywhere that was using a Clang which uses -stdlib=libc++ by default (i.e no passing in flags), will get the same (xplatform linux-g++-64) build. If they start passing -stdlib=libc++ they'll, if anything, get a "more correct" Qt build.
Anyone who is building with GCC / anything else / doesn't care about this, still gets the same -xplatform linux-g++-64 build.
Note that this is also scoped to Linux, so can't cause issues with the macOS builds which use libc++.
sidhujag
pushed a commit
to syscoin/syscoin
that referenced
this pull request
Dec 30, 2021
…stdlib=libc++ 33796a9 build: Add ability to build qt in depends with -stdlib=libc++ (Hennadii Stepanov) Pull request description: This PR makes possible to build the `qt` package in depends against `libc++` for x86_64 platform. Fixes bitcoin#22344. Required for bitcoin#22815. Also this PR [fixes](bitcoin#23060 (comment)) the `[no wallet] [bionic]` task on CI: - on master (a8bbd4c), https://api.cirrus-ci.com/v1/task/5558609250615296/logs/ci.log: ``` Options used to compile and link: external signer = yes multiprocess = no with libs = yes with wallet = no with gui / qt = no ``` - this PR, https://api.cirrus-ci.com/v1/task/5502605561430016/logs/ci.log: ``` Options used to compile and link: external signer = yes multiprocess = no with libs = yes with wallet = no with gui / qt = yes ``` ACKs for top commit: fanquake: ACK 33796a9 - While this sort of string matching is fragile, I think the risk of this causing any actual issues is low. Tree-SHA512: 586dde2e9864cec7a49aeb4f2b77fb8c4ae96bd10b51f9c6de0cfe8512ad61db15bb7f8d1b0eb6a5a66fd2deee52ac52218f01eb6be107ac12f1a956190de54b
PastaPastaPasta
pushed a commit
to PastaPastaPasta/dash
that referenced
this pull request
Apr 7, 2022
…stdlib=libc++ 33796a9 build: Add ability to build qt in depends with -stdlib=libc++ (Hennadii Stepanov) Pull request description: This PR makes possible to build the `qt` package in depends against `libc++` for x86_64 platform. Fixes bitcoin#22344. Required for bitcoin#22815. Also this PR [fixes](bitcoin#23060 (comment)) the `[no wallet] [bionic]` task on CI: - on master (a8bbd4c), https://api.cirrus-ci.com/v1/task/5558609250615296/logs/ci.log: ``` Options used to compile and link: external signer = yes multiprocess = no with libs = yes with wallet = no with gui / qt = no ``` - this PR, https://api.cirrus-ci.com/v1/task/5502605561430016/logs/ci.log: ``` Options used to compile and link: external signer = yes multiprocess = no with libs = yes with wallet = no with gui / qt = yes ``` ACKs for top commit: fanquake: ACK 33796a9 - While this sort of string matching is fragile, I think the risk of this causing any actual issues is low. Tree-SHA512: 586dde2e9864cec7a49aeb4f2b77fb8c4ae96bd10b51f9c6de0cfe8512ad61db15bb7f8d1b0eb6a5a66fd2deee52ac52218f01eb6be107ac12f1a956190de54b
PastaPastaPasta
pushed a commit
to PastaPastaPasta/dash
that referenced
this pull request
Apr 11, 2022
…stdlib=libc++ 33796a9 build: Add ability to build qt in depends with -stdlib=libc++ (Hennadii Stepanov) Pull request description: This PR makes possible to build the `qt` package in depends against `libc++` for x86_64 platform. Fixes bitcoin#22344. Required for bitcoin#22815. Also this PR [fixes](bitcoin#23060 (comment)) the `[no wallet] [bionic]` task on CI: - on master (a8bbd4c), https://api.cirrus-ci.com/v1/task/5558609250615296/logs/ci.log: ``` Options used to compile and link: external signer = yes multiprocess = no with libs = yes with wallet = no with gui / qt = no ``` - this PR, https://api.cirrus-ci.com/v1/task/5502605561430016/logs/ci.log: ``` Options used to compile and link: external signer = yes multiprocess = no with libs = yes with wallet = no with gui / qt = yes ``` ACKs for top commit: fanquake: ACK 33796a9 - While this sort of string matching is fragile, I think the risk of this causing any actual issues is low. Tree-SHA512: 586dde2e9864cec7a49aeb4f2b77fb8c4ae96bd10b51f9c6de0cfe8512ad61db15bb7f8d1b0eb6a5a66fd2deee52ac52218f01eb6be107ac12f1a956190de54b
gades
pushed a commit
to cosanta/cosanta-core
that referenced
this pull request
May 12, 2022
…stdlib=libc++ 33796a9 build: Add ability to build qt in depends with -stdlib=libc++ (Hennadii Stepanov) Pull request description: This PR makes possible to build the `qt` package in depends against `libc++` for x86_64 platform. Fixes bitcoin#22344. Required for bitcoin#22815. Also this PR [fixes](bitcoin#23060 (comment)) the `[no wallet] [bionic]` task on CI: - on master (a8bbd4c), https://api.cirrus-ci.com/v1/task/5558609250615296/logs/ci.log: ``` Options used to compile and link: external signer = yes multiprocess = no with libs = yes with wallet = no with gui / qt = no ``` - this PR, https://api.cirrus-ci.com/v1/task/5502605561430016/logs/ci.log: ``` Options used to compile and link: external signer = yes multiprocess = no with libs = yes with wallet = no with gui / qt = yes ``` ACKs for top commit: fanquake: ACK 33796a9 - While this sort of string matching is fragile, I think the risk of this causing any actual issues is low. Tree-SHA512: 586dde2e9864cec7a49aeb4f2b77fb8c4ae96bd10b51f9c6de0cfe8512ad61db15bb7f8d1b0eb6a5a66fd2deee52ac52218f01eb6be107ac12f1a956190de54b
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.
This PR makes possible to build the
qtpackage in depends againstlibc++for x86_64 platform.Fixes #22344.
Required for #22815.
Also this PR fixes the
[no wallet] [bionic]task on CI: