Skip to content

depends: remove redundant stdlib option #27721

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
May 25, 2023
Merged

depends: remove redundant stdlib option #27721

merged 1 commit into from
May 25, 2023

Conversation

theuni
Copy link
Member

@theuni theuni commented May 22, 2023

Like #27628, this is another dependency of #21778, though it doesn't become obvious until used with a newer clang.

This should be a no-op.

Use of -stdlib++-isystem gets rid of any system c++ header include paths and negates the need for this option. In newer versions of clangs the combo produces an annoying warning that actually causes problems during configure.

@theuni
depends: remove redundant stdlib option
4fe5f3c 
Use of -stdlib++-isystem gets rid of any system c++ header include paths and
negates the need for this option. In newer versions of clangs the combo
produces a warning.
@DrahtBot
Copy link
Contributor

DrahtBot commented May 22, 2023 •
edited
Loading

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Reviews

See the guideline for information on the review process.

Type Reviewers
ACK fanquake

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

@fanquake
Copy link
Member

Concept ACK

@fanquake
Copy link
Member

Guix Build:

ed1f83a77d00:/bitcoin# find guix-build-$(git rev-parse --short=12 HEAD)/output/ -type f -print0 | env LC_ALL=C sort -z | xargs -r0 sha256sum
b2fc48fff5ef2c15a520582845a8af73c2af71f4f59a0d0b92b04db8530373b0  guix-build-4fe5f3c46752/output/aarch64-linux-gnu/SHA256SUMS.part
d074ce3bdc89db1d7d0e4fcbde818b9a2183c9a23541e2dabcb2401e88635959  guix-build-4fe5f3c46752/output/aarch64-linux-gnu/bitcoin-4fe5f3c46752-aarch64-linux-gnu-debug.tar.gz
890d8535d8590ceb5d17b0cbd254c4c17e3791930b34413e7ba5e69983bb0efb  guix-build-4fe5f3c46752/output/aarch64-linux-gnu/bitcoin-4fe5f3c46752-aarch64-linux-gnu.tar.gz
17da41c41dc5fbbcf8f8822bb735af1208e42c9b7c9f26d895b583eb3781ebc2  guix-build-4fe5f3c46752/output/arm-linux-gnueabihf/SHA256SUMS.part
91203aceac6c0d025fe2946a24b33cc1f672f35abce2a4cf355dd7b0c29729ce  guix-build-4fe5f3c46752/output/arm-linux-gnueabihf/bitcoin-4fe5f3c46752-arm-linux-gnueabihf-debug.tar.gz
9ad48abf7f342f3f1d681fc8aef36b5e7b0b6509776a22bafa944869e7f4d450  guix-build-4fe5f3c46752/output/arm-linux-gnueabihf/bitcoin-4fe5f3c46752-arm-linux-gnueabihf.tar.gz
c6c0784a4e4c3a6b2ee8c570d36f2f2423569d50101c235e3268eb69d2b3ddc8  guix-build-4fe5f3c46752/output/arm64-apple-darwin/SHA256SUMS.part
ef2f70d33ac5a6e14aefe956b505f2947d3d0812c8ca757e8d0a54a1ad89a6eb  guix-build-4fe5f3c46752/output/arm64-apple-darwin/bitcoin-4fe5f3c46752-arm64-apple-darwin-unsigned.dmg
4e9afc179fdeaa6b27755f2490db66e7ce4ecb148a6da34ffce96e79edc57c23  guix-build-4fe5f3c46752/output/arm64-apple-darwin/bitcoin-4fe5f3c46752-arm64-apple-darwin-unsigned.tar.gz
ae08e9d9f7fed75eff54c06074898869292a16bbff76c492990f557991b4f26d  guix-build-4fe5f3c46752/output/arm64-apple-darwin/bitcoin-4fe5f3c46752-arm64-apple-darwin.tar.gz
e2f10f598eca972c80a3e9c9cef1b6825fdea80b24bca3bd3792366621c707e2  guix-build-4fe5f3c46752/output/dist-archive/bitcoin-4fe5f3c46752.tar.gz
2b6d41256afaec1d8ce37e7160c59b58fd2588585392de2274b1136654df9695  guix-build-4fe5f3c46752/output/powerpc64-linux-gnu/SHA256SUMS.part
ee74b47acbb72d0d48670426b197700a89e856b3475fea71f447c44962615dcb  guix-build-4fe5f3c46752/output/powerpc64-linux-gnu/bitcoin-4fe5f3c46752-powerpc64-linux-gnu-debug.tar.gz
f25e8063fcb3132b55f72599770abb7d69cfb8f55102cc2581dd2ea809264149  guix-build-4fe5f3c46752/output/powerpc64-linux-gnu/bitcoin-4fe5f3c46752-powerpc64-linux-gnu.tar.gz
4797a634c6fdcff0b1023e25f4a3a6edf2f9da28349e99d4d264cf4da794b7c7  guix-build-4fe5f3c46752/output/powerpc64le-linux-gnu/SHA256SUMS.part
72491d6a821925156e708baf47723f51ea785011e2ea224de3b41e42a37200f5  guix-build-4fe5f3c46752/output/powerpc64le-linux-gnu/bitcoin-4fe5f3c46752-powerpc64le-linux-gnu-debug.tar.gz
e54da3394a3b4f0f836868e0990eb7d03628b17b4226e8a6e27708e89030e56a  guix-build-4fe5f3c46752/output/powerpc64le-linux-gnu/bitcoin-4fe5f3c46752-powerpc64le-linux-gnu.tar.gz
7dc2a0b048bd1bc44c87c49c745d3b1f1f25d8e61911f3d7128c621d447c0b0f  guix-build-4fe5f3c46752/output/riscv64-linux-gnu/SHA256SUMS.part
833f7c288abf386c6f4d81e33dc6616aedfcf7b00731b7636a72618e60b7e09c  guix-build-4fe5f3c46752/output/riscv64-linux-gnu/bitcoin-4fe5f3c46752-riscv64-linux-gnu-debug.tar.gz
867dd0ef016b230fd568b00887f78a9c314dbe14b26dfea30ecce3a0b276b8ac  guix-build-4fe5f3c46752/output/riscv64-linux-gnu/bitcoin-4fe5f3c46752-riscv64-linux-gnu.tar.gz
7e94a9ed36e806ffd4bfd944d626b418583630b2b13623f8b082dc5f39d7e1e3  guix-build-4fe5f3c46752/output/x86_64-apple-darwin/SHA256SUMS.part
992b53920c86f6962eef7938d4a0dfa45ae04fcd76971f7f00f4bc4acde01a58  guix-build-4fe5f3c46752/output/x86_64-apple-darwin/bitcoin-4fe5f3c46752-x86_64-apple-darwin-unsigned.dmg
7244946e0629ba4eaa956b50f9072d9ea1563dc1fbdab1e1e63f6d87b03a8d0c  guix-build-4fe5f3c46752/output/x86_64-apple-darwin/bitcoin-4fe5f3c46752-x86_64-apple-darwin-unsigned.tar.gz
8c6d1e89c5541fac79d1b9c8c51f775f1a87b963951c1b2df8da9e12c6567921  guix-build-4fe5f3c46752/output/x86_64-apple-darwin/bitcoin-4fe5f3c46752-x86_64-apple-darwin.tar.gz
3f64b6bc2c4316143a8845f0eaaba11a0fbd4d44d8164a450ceb01e97795dae7  guix-build-4fe5f3c46752/output/x86_64-linux-gnu/SHA256SUMS.part
709c8afd2abb92015991e389a39f98646fa9d729f05d373b236b63f53923cc7e  guix-build-4fe5f3c46752/output/x86_64-linux-gnu/bitcoin-4fe5f3c46752-x86_64-linux-gnu-debug.tar.gz
35d4e2dd94efc6c5cf8b7671ce9c445152cc4c744d2c50e2d23217a03602518c  guix-build-4fe5f3c46752/output/x86_64-linux-gnu/bitcoin-4fe5f3c46752-x86_64-linux-gnu.tar.gz
048b450acf8e316dfad4cef242891e08f5bddc3828a14c258ef643aaa638e196  guix-build-4fe5f3c46752/output/x86_64-w64-mingw32/SHA256SUMS.part
10cffcfa5f36a4a9300b64d30f6230af4e21b3b4ffd1129a5a61e3fa39e42317  guix-build-4fe5f3c46752/output/x86_64-w64-mingw32/bitcoin-4fe5f3c46752-win64-debug.zip
14a7ea0c6be0366f3a1679b9c6214d34fb6f8d8ab8775b18abb996a49a64ea52  guix-build-4fe5f3c46752/output/x86_64-w64-mingw32/bitcoin-4fe5f3c46752-win64-setup-unsigned.exe
cfa0466943018add68791ab40c90027f18007bef56cb35a794a2bd29438f590e  guix-build-4fe5f3c46752/output/x86_64-w64-mingw32/bitcoin-4fe5f3c46752-win64-unsigned.tar.gz
7e28b91edfad8d0838c8c8bcffe4808b47c7217a646b9364bb8487c4794a605e  guix-build-4fe5f3c46752/output/x86_64-w64-mingw32/bitcoin-4fe5f3c46752-win64.zip

@hebasto
Copy link
Member

hebasto commented May 23, 2023

This should be a no-op.

I cannot confirm it, at least for x86_64-apple-darwin

$ git fetch origin pull/27721/head
$ git checkout FETCH_HEAD
$ make -C depends clean-all
$ make -C depends HOST=x86_64-apple-darwin
$ find depends/built/x86_64-apple-darwin -name '*.hash' | sort | xargs cat
afc3a7c266402a88b9c2bbbaa11a4dfceca5b1d4d33290080b9c31c8f5c65d25  bdb-4.8.30-d31fb2c14af.tar.gz
460b8455718a04f7d25d84179149c2d9b98b3318eedb3f89f50f93b6cff3d8ec  boost-1.81.0-6596e037cc7.tar.gz
7381828160516b0b7da1bb9b30db2c1277d7c0a4186ea5fbe153c73ee3c121a9  libevent-2.1.12-stable-a35fe3bb413.tar.gz
78fec0a98519b82cf3203993553aed32b21e1bebd24c789551aa7c81e057a21c  libnatpmp-07004b97cf691774efebe70404cf22201e4d330d-da547d4c547.tar.gz
96eae74b40650db7cde906b605c627ae0fccc5162115f851349dbd4de9ddcee5  miniupnpc-2.2.2-ef25d875ac5.tar.gz
73a993340fe4fa7df7df677c8278dc6b25af85ce000212c91006de38a7b48782  native_cctools-2ef2e931cf641547eb8a68cfebde61003587c9fd-c67166e4e09.tar.gz
300b13fa3db495f3ad33f729a2fa2688e84d928a17ef8f4eb1164e062ad924c6  native_clang-10.0.1-3cc795e4f28.tar.gz
04851409e9da17f438b08c8d433f069026695b1378b9b3cebc7940cf08210268  native_ds_store-1.3.0-b4c73f323c4.tar.gz
d962cb0f803b030aa6926134bcaf5e40ff75058bf6ac2023051c7fee58dfb40f  native_libtapi-664b8414f89612f2dfd35a9b679c345aa5389026-3fed744906b.tar.gz
3e996c0b752eb1835ee47f34366edb74fde9696b9d51a81234f4332e656f1e4d  native_mac_alias-2.2.0-62c154fe273.tar.gz
13bcf84a3cc0421b22c2ad233675b6d05509ca29bc764ec8ae95cbd4974e174f  qrencode-4.1.1-869c4a8f002.tar.gz
1a403554eebfed1372e6e47ea37e177265a56f13fd8f5bbec02e79a70c0ffade  qt-5.15.5-aedab57c844.tar.gz
9d6bf457d86d9222163262c896bf4dbfae7c4b122f7f6f37ee55c59fc34f3986  sqlite-3380500-b5e03f3fdf6.tar.gz
46b3fa988645c30d92ebdcdb91a1baee6431f304a03a573c30cdadffc745c0fb  zeromq-4.3.4-ac6301234b5.tar.gz
$ git checkout HEAD~1
$ make -C depends clean-all
$ make -C depends HOST=x86_64-apple-darwin
$ find depends/built/x86_64-apple-darwin -name '*.hash' | sort | xargs cat
965f6557940a743c4a59830655129f4edd4ed0a70fed538a1d8989ab88284c9d  bdb-4.8.30-f3cdf5a97f8.tar.gz
460b8455718a04f7d25d84179149c2d9b98b3318eedb3f89f50f93b6cff3d8ec  boost-1.81.0-282e4907c35.tar.gz
a1856045a9606448d4fb0c9e50794dbdcd556ac49fd0c2fc456238d01fcc735f  libevent-2.1.12-stable-00599096a1c.tar.gz
094db5ccfcfc47be70ed381d9b0a0e6d549a6a7c3442bea2677931219c80187c  libnatpmp-07004b97cf691774efebe70404cf22201e4d330d-d642acc3dfd.tar.gz
e72db1b41fc5562bb5262a8bf7afffb1726048440827e8299faff99e7bdb0f48  miniupnpc-2.2.2-a5e12b30b76.tar.gz
db7d4a6b6f3ab7f7477f5513b9f586ea756e237d1fecdca6717949938e735a04  native_cctools-2ef2e931cf641547eb8a68cfebde61003587c9fd-1e8c3bffab2.tar.gz
300b13fa3db495f3ad33f729a2fa2688e84d928a17ef8f4eb1164e062ad924c6  native_clang-10.0.1-a7086a951bd.tar.gz
04851409e9da17f438b08c8d433f069026695b1378b9b3cebc7940cf08210268  native_ds_store-1.3.0-52262340e3d.tar.gz
81e12478d2822f521b1a275f1b0b9a145874c5310f802941ff6ca4568d69c066  native_libtapi-664b8414f89612f2dfd35a9b679c345aa5389026-2061ab7cec6.tar.gz
3e996c0b752eb1835ee47f34366edb74fde9696b9d51a81234f4332e656f1e4d  native_mac_alias-2.2.0-80a24b1d943.tar.gz
cee38ee08586f73fc821723c6217fcef9da0b15da372f4afd7d4ca2bde32a34a  qrencode-4.1.1-0fd25b82bc0.tar.gz
a7d331be51291f8102ec24fcc61536c700ba4b319f19b7b8eb6c42515164d34c  qt-5.15.5-5012034142d.tar.gz
14c836b58943998f3b0287ae994cc905de6baf83dc29e3b00d8c8cafa98f3b57  sqlite-3380500-550dac04df6.tar.gz
c8938851cc72a6bdb84e9491658c42a0e23c891a25d56245ca8d6ad1e9dd4409  zeromq-4.3.4-2a2f249fd02.tar.gz

@hebasto
Copy link
Member

hebasto commented May 23, 2023

This should be a no-op.

I cannot confirm it...

Well, diffoscope shows only diffs in object files timestamps.

@theuni
Copy link
Member Author

theuni commented May 23, 2023

Sorry, I guess I should've said it's a behavioral no-op.

A simple test-case shows that search-paths are unchanged with or without the -stdlib=libc++ flag:
env -u C_INCLUDE_PATH -u CPLUS_INCLUDE_PATH -u OBJC_INCLUDE_PATH -u OBJCPLUS_INCLUDE_PATH -u CPATH -u LIBRARY_PATH /home/cory/dev/llvm-project/build3/bin/clang++ --target=x86_64-apple-darwin -mmacosx-version-min=11.0 -B/home/cory/dev/bitcoin2/depends/x86_64-apple-darwin/native/bin -mlinker-version=609 -isysroot/home/cory/dev/bitcoin2/depends/SDKs/Xcode-12.2-12B45b-extracted-SDK-with-libcxx-headers -stdlib=libc++ -stdlib++-isystem/home/cory/dev/bitcoin2/depends/SDKs/Xcode-12.2-12B45b-extracted-SDK-with-libcxx-headers/usr/include/c++/v1 -Xclang -internal-externc-isystem -Xclang /home/cory/dev/llvm-project/build/lib/clang/11.0.1/include -Xclang -internal-externc-isystem -Xclang /home/cory/dev/bitcoin2/depends/SDKs/Xcode-12.2-12B45b-extracted-SDK-with-libcxx-headers/usr/include -I/home/cory/dev/bitcoin2/depends/x86_64-apple-darwin/include -D_FORTIFY_SOURCE=3 test.cpp -v

fanquake
fanquake approved these changes May 25, 2023
View reviewed changes
Copy link
Member

@fanquake fanquake left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK 4fe5f3c

@fanquake fanquake merged commit 25202ca into bitcoin:master May 25, 2023
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request May 26, 2023
@fanquake
Merge bitcoin#27721: depends: remove redundant stdlib option
4acbb43 
4fe5f3c depends: remove redundant stdlib option (Cory Fields)

Pull request description:

  Like bitcoin#27628, this is another dependency of bitcoin#21778, though it doesn't become obvious until used with a newer clang.

  This should be a no-op.

  Use of -stdlib++-isystem gets rid of any system c++ header include paths and negates the need for this option. In newer versions of clangs the combo produces an annoying warning that actually causes problems during configure.

ACKs for top commit:
  fanquake:
    ACK 4fe5f3c

Tree-SHA512: 904072f2b13dffbbeab2bc9ff20a74969888502fa1ea35d9030784dd397c6751e31233e6ec7dc34e9fd42574950be733b25d6653c2a93e214cc5e4eef2e0133a
Fabcien pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this pull request Mar 1, 2024
@theuni @PiRK
[depends] darwin.mk backports to support newer clang versions
082d9ee 
Summary:
> depends: fix osx build with clang 16
>
> For some reason the previous syntax worked with clang 15 and below, but
> clang 16 requires that the option and value are properly separated.

> depends: remove redundant stdlib option
>
> Use of -stdlib++-isystem gets rid of any system c++ header include paths and
> negates the need for this option. In newer versions of clangs the combo
> produces a warning.

> depends: modernize clang flags
>
> Fixes builds with llvm >= 11 in guix by working around the problem. As a bonus,
> this is much cleaner and more maintainable than what we had before.

This is a backport of [[bitcoin/bitcoin#27328 | core#27328]], [[bitcoin/bitcoin#27721 | core#27721]] and [[bitcoin/bitcoin#27798 | core#27798]]

The change to ci/test/00_setup_env_mac.sh in [[bitcoin/bitcoin#27798 | core#27798]] to ignore -Wunused-command-line-argument warnings seems equivalent to D3792

Test Plan: check that gitian-osx still works

Reviewers: #bitcoin_abc, Fabien

Reviewed By: #bitcoin_abc, Fabien

Differential Revision: https://reviews.bitcoinabc.org/D15579
@bitcoin bitcoin locked and limited conversation to collaborators May 24, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@fanquake fanquake fanquake approved these changes

Assignees
No one assigned
Labels
Build system
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@theuni @DrahtBot @fanquake @hebasto