depends: Pin clang search paths for darwin host #19683

dongcarl commented at 5:41 PM on August 7, 2020: contributor

Hello clang/lib/frontend, I search your headers once again. Because it's time for some housekeeping, Within the code I was tweaking, And the targets I was making with my build, Are unfulfilled, It's just language compliance.

In reference works I scroll alone Pages cribbed from holy tomes In the details of a template My code's behaviour has now found its fate When my hopes were dashed as a note left it as described: As undefined It's not in compliance

And from the standard text I saw Ten thousand errors, maybe more Threading used without locking Pointers referenced after freeing Linters writing warnings that coders will never fix But still they tick The box that claims compliance

"Fools," said I, "you do not know" Errors, like a cancer, grow Hear my words that I might reach you Use -Wall and it might teach you But my words and compiler errors fade. Schedules forbade compliance.

And the people bowed and prayed With static checking torn and frayed The markets flashed out their warning In the words that they were forming As recruiters said "The search for more profits leads to writing stuff in CSS, And node.js. Without a need for compliance"

Many thanks to ajtowns for the above contribution!

This PR is ready for review!

When cross-compiling for macOS, the SDK gives us the entire context/sysroot on which we should base the build. This means that we can be extremely specific w/re our search path ordering in order to avoid build problems that arise out of a user's specific environment/system setup and improve the robustness of our macOS toolchain. This PR does 2 things to this end:

Unset environment variables which are known to alter search paths.
Makes us (in the case of macOS builds) explicitly specify the list of system include search paths and its ordering, rather than rely on clang's unreliable autodetection routine. Here is the rabbit-hole gist.

See the added comments in depends/hosts/darwin.mk for more details:

https://github.com/bitcoin/bitcoin/blob/8b8296dc70a0aa5ca86d11ba5d3151fc56208e25/depends/hosts/darwin.mk#L37-L60

We can be this specific only because macOS builds are neatly contained in an SDK, and we are cross-compiling. Native toolchains should rely on the environment/distro/user to know how best to build for the running system.

Note: Although the -u flag of env is not a POSIX standard flag, it seems like it is useful enough to be implemented in coreutils, busybox, FreeBSD.

dongcarl added the label Build system on Aug 7, 2020

dongcarl requested review from theuni on Aug 7, 2020

dongcarl added this to the "PRs" column in a project

fanquake commented at 7:55 AM on August 10, 2020: member

Concept ACK - did not read the @ajtowns poem. Did you want to link to your other recent gist here as well?

dongcarl commented at 3:08 PM on August 10, 2020: contributor

Did you want to link to your other recent gist here as well?

Added to description! It's also in the comments added to darwin.mk.

fanquake cross-referenced this on Aug 19, 2020 from issue depends: Allow building with system clang by dongcarl

dongcarl added the label Needs gitian build on Aug 21, 2020

dongcarl added the label Needs Guix build on Aug 21, 2020

DrahtBot commented at 10:06 AM on August 25, 2020: contributor

Guix builds

File	commit 197450f80868fe752c6107955e5da80704212b34<br>(master)	commit d9bf0c9bde76e883de238806b64c0c1e966fc0a6<br>(master and this pull)
*-aarch64-linux-gnu-debug.tar.gz	`8943872ac3d5df7c...`	`d794e47112269afb...`
*-aarch64-linux-gnu.tar.gz	`7307c4ee3d4934bd...`	`e1c0d9e7ec4595c4...`
*-arm-linux-gnueabihf-debug.tar.gz	`fccc26d7c7dfa90b...`	`c91689dbc8d561cf...`
*-arm-linux-gnueabihf.tar.gz	`34e1140f65a0f606...`	`315c0695d8e6b05e...`
*-riscv64-linux-gnu-debug.tar.gz	`a94385d318571b00...`	`59c84856272e8ed9...`
*-riscv64-linux-gnu.tar.gz	`3abb016b01e33949...`	`d67c08201b63b914...`
*-win-unsigned.tar.gz	`f35d661c7f1bbdae...`	`cfe56e013cffd22d...`
*-win64-debug.zip	`62437d10c94cfef6...`	`4e99dd7d12a58df5...`
*-win64-setup-unsigned.exe	`5e2b6831dd645b79...`	`5f881cdc73dff930...`
*-win64.zip	`c9829996dc3386f5...`	`b19b255203e71057...`
*-x86_64-linux-gnu-debug.tar.gz	`effc996453960003...`	`e7f7bf96c01bda79...`
*-x86_64-linux-gnu.tar.gz	`7ef18c4f4cd4f2e2...`	`831d078480a6c38e...`
*.tar.gz	`5c60a590f5e9971c...`	`26ced5e629e9442e...`
guix_build.log	`3eca0392f5149475...`	`d622e2144f1d27de...`
guix_build.log.diff		`41f7c68911c50f38...`

DrahtBot removed the label Needs Guix build on Aug 25, 2020

DrahtBot commented at 7:51 AM on August 28, 2020: contributor

Gitian builds

File	commit 93ab136a33e46080c8aa02d59fb7c2a8d03a3387<br>(master)	commit bdde84c90190ce8e836f8e3000e6dcaad0ef45dd<br>(master and this pull)
*-aarch64-linux-gnu-debug.tar.gz	`d9677652e379d0fb...`	`8f31f5cfa71cbf24...`
*-aarch64-linux-gnu.tar.gz	`ccea6adc4185f412...`	`9bff9b0232aac533...`
*-arm-linux-gnueabihf-debug.tar.gz	`a6eb099566fbfaf7...`	`bb3623af1913ff5e...`
*-arm-linux-gnueabihf.tar.gz	`a778747de3c362d3...`	`5d0db25bd4d4a4ae...`
*-osx-unsigned.dmg	`7f2fe092602c5f24...`	`60478ecfe49cb20c...`
*-osx64.tar.gz	`572ad9470570b795...`	`1c0f22743bb14727...`
*-riscv64-linux-gnu-debug.tar.gz	`e3c3530a641bf3ff...`	`f8fc251b5c0a5865...`
*-riscv64-linux-gnu.tar.gz	`9e35a022bcbb24b5...`	`93e7e066ba6ff587...`
*-win64-debug.zip	`183ec1685b274201...`	`1af6f04e68b5e7fd...`
*-win64-setup-unsigned.exe	`b0345c819e39cf5b...`	`48940692c654388b...`
*-win64.zip	`ee5279efd7b75303...`	`ec16942779035be9...`
*-x86_64-linux-gnu-debug.tar.gz	`1528d365bd4947b0...`	`6b1c825a3530c37a...`
*-x86_64-linux-gnu.tar.gz	`721e213b9f0ada44...`	`9b7b9b889bd607b0...`
*.tar.gz	`5e4e2d09bfcdc38c...`	`9a328246657d6bbc...`
bitcoin-core-linux-0.21-res.yml	`0366af80fb44ac95...`	`7c30769618819c28...`
bitcoin-core-osx-0.21-res.yml	`1635b70517fa98f9...`	`9d8ac5adfad89196...`
bitcoin-core-win-0.21-res.yml	`25db96826213443f...`	`e8272d520bb4d12b...`
linux-build.log	`6c3144ff05037c6e...`	`04796c1da003d08b...`
osx-build.log	`ce3a375d3051d5ba...`	`4f55ee0961daa8e9...`
win-build.log	`3a0f70b25edb7239...`	`fae8119f5c5fdfd5...`
bitcoin-core-linux-0.21-res.yml.diff		`e48921ddf9c03320...`
bitcoin-core-osx-0.21-res.yml.diff		`1c8900aa1d2ee134...`
bitcoin-core-win-0.21-res.yml.diff		`1f91673bd536ecbf...`
linux-build.log.diff		`9a5a4b928e8c6355...`
osx-build.log.diff		`b21f11c25e247b41...`
win-build.log.diff		`6d3c16143aec17b4...`

DrahtBot removed the label Needs gitian build on Aug 28, 2020

dongcarl force-pushed on Sep 25, 2020

dongcarl renamed this:
~~WIP: depends: Explicitly specify clang search paths for darwin host~~
depends: Pin clang search paths for darwin host
on Sep 25, 2020

dongcarl marked this as ready for review on Sep 25, 2020

dongcarl commented at 9:12 PM on September 25, 2020: contributor

This PR is now based on #20019, please review that first. I've added some more details to the original description and I believe this is in a good state to be reviewed and merged (after #20019 ofc).

ryanofsky cross-referenced this on Sep 30, 2020 from issue depends: Set CMAKE_INSTALL_RPATH for native packages by ryanofsky

DrahtBot commented at 9:27 PM on October 24, 2020: contributor

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

#18298 (build: Fix Qt processing of configure script for depends with DEBUG=1 by hebasto)

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.

DrahtBot cross-referenced this on Oct 25, 2020 from issue gitian-linux: Build binaries for 64-bit POWER by luke-jr

dongcarl force-pushed on Nov 23, 2020

dongcarl commented at 12:02 AM on November 24, 2020: contributor

Updated so that it no longer depends on #20019.

MarcoFalke commented at 7:13 AM on November 24, 2020: member

Build failure. Verbose build follows.
Making all in src
make[1]: Entering directory '/tmp/cirrus-ci-build/ci/scratch/build/bitcoin-x86_64-apple-darwin18/src'
make[2]: Entering directory '/tmp/cirrus-ci-build/ci/scratch/build/bitcoin-x86_64-apple-darwin18/src'
/bin/bash ../libtool  --tag=CXX --preserve-dup-deps  --mode=compile /usr/bin/ccache env -u C_INCLUDE_PATH -u CPLUS_INCLUDE_PATH -u OBJC_INCLUDE_PATH -u OBJCPLUS_INCLUDE_PATH -u CPATH -u LIBRARY_PATH clang++ --target=x86_64-apple-darwin18 -mmacosx-version-min=10.14 -B/tmp/cirrus-ci-build/depends/x86_64-apple-darwin18/native/bin -mlinker-version=530 --sysroot=/tmp/cirrus-ci-build/depends/SDKs/Xcode-11.3.1-11C505-extracted-SDK-with-libcxx-headers -stdlib=libc++ -nostdinc++ -Xclang -cxx-isystem/tmp/cirrus-ci-build/depends/SDKs/Xcode-11.3.1-11C505-extracted-SDK-with-libcxx-headers/usr/include/c++/v1 -Xclang -internal-externc-isystem/tmp/cirrus-ci-build/depends/x86_64-apple-darwin18/native/lib/clang/8.0.0/include -Xclang -internal-externc-isystem/tmp/cirrus-ci-build/depends/SDKs/Xcode-11.3.1-11C505-extracted-SDK-with-libcxx-headers/usr/include -std=c++17 -DHAVE_CONFIG_H -I. -I../src/config   -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -I./obj -I./secp256k1/include -DBUILD_BITCOIN_INTERNAL -I/tmp/cirrus-ci-build/depends/x86_64-apple-darwin18/include/  -DHAVE_BUILD_INFO -D__STDC_FORMAT_MACROS -DMAC_OSX -DOBJC_OLD_DISPATCH_PROTOTYPES=0  -Wstack-protector -fstack-protector-all -fcf-protection=full   -Werror=gnu -Werror=vla -Werror=shadow-field -Werror=switch -Werror=thread-safety -Werror=range-loop-analysis -Werror=unused-variable -Werror=date-time -Werror=return-type -Werror=conditional-uninitialized -Werror=sign-compare -Werror=unreachable-code-loop-increment    -pipe -O2  -fvisibility=hidden -c -o support/libbitcoinconsensus_la-cleanse.lo `test -f 'support/cleanse.cpp' || echo './'`support/cleanse.cpp
libtool: compile:  /usr/bin/ccache env -u C_INCLUDE_PATH -u CPLUS_INCLUDE_PATH -u OBJC_INCLUDE_PATH -u OBJCPLUS_INCLUDE_PATH -u CPATH -u LIBRARY_PATH clang++ --target=x86_64-apple-darwin18 -mmacosx-version-min=10.14 -B/tmp/cirrus-ci-build/depends/x86_64-apple-darwin18/native/bin -mlinker-version=530 --sysroot=/tmp/cirrus-ci-build/depends/SDKs/Xcode-11.3.1-11C505-extracted-SDK-with-libcxx-headers -stdlib=libc++ -nostdinc++ -Xclang -cxx-isystem/tmp/cirrus-ci-build/depends/SDKs/Xcode-11.3.1-11C505-extracted-SDK-with-libcxx-headers/usr/include/c++/v1 -Xclang -internal-externc-isystem/tmp/cirrus-ci-build/depends/x86_64-apple-darwin18/native/lib/clang/8.0.0/include -Xclang -internal-externc-isystem/tmp/cirrus-ci-build/depends/SDKs/Xcode-11.3.1-11C505-extracted-SDK-with-libcxx-headers/usr/include -std=c++17 -DHAVE_CONFIG_H -I. -I../src/config -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -I./obj -I./secp256k1/include -DBUILD_BITCOIN_INTERNAL -I/tmp/cirrus-ci-build/depends/x86_64-apple-darwin18/include/ -DHAVE_BUILD_INFO -D__STDC_FORMAT_MACROS -DMAC_OSX -DOBJC_OLD_DISPATCH_PROTOTYPES=0 -Wstack-protector -fstack-protector-all -fcf-protection=full -Werror=gnu -Werror=vla -Werror=shadow-field -Werror=switch -Werror=thread-safety -Werror=range-loop-analysis -Werror=unused-variable -Werror=date-time -Werror=return-type -Werror=conditional-uninitialized -Werror=sign-compare -Werror=unreachable-code-loop-increment -pipe -O2 -fvisibility=hidden -c support/cleanse.cpp  -fno-common -DPIC -o support/.libs/libbitcoinconsensus_la-cleanse.o
/usr/bin/env: ‘clang++’: No such file or directory
Makefile:14178: recipe for target 'support/libbitcoinconsensus_la-cleanse.lo' failed

dongcarl force-pushed on Dec 7, 2020

dongcarl cross-referenced this on Dec 7, 2020 from issue guix: Build support for macOS by dongcarl

dongcarl force-pushed on Dec 11, 2020

DrahtBot cross-referenced this on Dec 12, 2020 from issue build: Fix Qt processing of configure script for depends with DEBUG=1 by hebasto

DrahtBot cross-referenced this on Dec 12, 2020 from issue net: Add NAT-PMP port forwarding support by hebasto

dongcarl commented at 5:25 PM on December 17, 2020: contributor

Currently figuring out how to make ccache work properly with this change. Looks like CCACHE_PREFIX should be set somehow.

dongcarl force-pushed on Dec 21, 2020

dongcarl commented at 8:01 PM on December 22, 2020: contributor

Pushed fd12a7e2a8932620e7db528e8b0f149507a9f131 -> 549e2ae5e62dd0e3d6916023ba88f5c2138671ca

Restored previous behavior of pinning CC/CXX/AR/etc when generating config.site, but with a (more flexible) env PATH= wrapper rather than prefixing the bindir

I also evaluated the recommended alternative of calling ccache with clang as argv[1] and measured the effects on ccache's hits and misses.

My procedure:

Compile depends
Do an initial build
Clear ccache statistics, but not cache
Do a second build (presumable that should have quite a few cache hits from the initial build)

clang as argv[1]:

cache directory                     /home/dongcarl/.ccache
primary config                      /home/dongcarl/.ccache/ccache.conf
secondary config      (readonly)    /etc/ccache.conf
stats updated                       Fri Dec 18 16:47:22 2020
stats zeroed                        Fri Dec 18 16:47:07 2020
cache hit (direct)                   478
cache hit (preprocessed)               1
cache miss                             1
cache hit rate                     99.79 %
called for link                        7
cleanups performed                     0
files in cache                      1175
cache size                          60.5 MB
max cache size                       5.0 GB

This PR:

cache directory                     /home/dongcarl/.ccache
primary config                      /home/dongcarl/.ccache/ccache.conf
secondary config      (readonly)    /etc/ccache.conf
stats updated                       Fri Dec 18 01:38:46 2020
stats zeroed                        Fri Dec 18 01:38:29 2020
cache hit (direct)                   471
cache hit (preprocessed)               0
cache miss                             1
cache hit rate                     99.79 %
called for link                        6
cleanups performed                     0
files in cache                       888
cache size                          57.9 MB
max cache size                       5.0 GB

Given that the difference in number of cache hits is so small, and the code required to make "clang as argv[1]" work is somewhat complex, I decided it was not worth it.

dongcarl force-pushed on Jan 7, 2021

depends: Delay expansion of per-package vars

Prior to this commit, when int_vars was called for packages, it would
immediately expand the "single-dollar variables", which may be defined
in terms of variables which are not yet determined (e.g. variables
defined in package/*.mk, which are included after int_vars is called).

This is required for the next commit as after that commit, for darwin
cross-builds:

0. int_vars is defined in terms of $(1)_cc
1. $(1)_cc is defined in terms of darwin_CC
2. ... which is defined in terms of clang_resource_dir
3. ... which is defined in terms of native_cctools_clang_version
4. which is undetermined at the time when int_vars is being expanded and evaluated

107f33d434

depends: Pin clang search paths for darwin host 3007339218

depends: Remove -fuse-ld line

clang warns when a command line option is unused, and some of our tests
use Werror, so unfortunately we cannot use this flag to pin our linker
for now. Leaving this commit in for future reference, as it would be
great if there's more granularity to Werror and we can be explicit about
what linker we want to use.

77b1ef89a0

depends: Quote to prevent word splitting in config.site

SC2086 is disabled in our linter script so this wasn't caught.

8033110741

dongcarl force-pushed on Jan 7, 2021

dongcarl commented at 7:06 PM on January 7, 2021: contributor

Pushed 549e2ae5e62dd0e3d6916023ba88f5c2138671ca -> b29ec8d359f626d99e13baf4a54678bdf7dedccb

Use "${var}/bar" instead of "$var/bar" to be more more explicit
Simplify how $PATH is determined

depends: Fully determine path for darwin_{CC,CXX}

Instead of doing the awkward /bin path prepending at config.site
creation time, set darwin_{CC,CXX} in a way that fully determines the
program's path (clang/clang++) similar to how AC_PATH_{TOOL,PROG} would
do.

Also see the added comment block in depends/Makefile for more context on
determining $PATH for our config.site.

880660acfa

depends: Fully determine path for darwin cctools

See previous commit for description.

949c480e52

depends: Add comment about cache invalidation 196b727649

dongcarl force-pushed on Jan 7, 2021

dongcarl commented at 7:26 PM on January 7, 2021: contributor

Pushed b29ec8d359f626d99e13baf4a54678bdf7dedccb -> 196b7276495c5d125e3799aee6cfc54be6720ec7

Added comment block about properly seeding program vars via config.site

laanwj commented at 8:33 PM on January 7, 2021: member

code review ACK 196b7276495c5d125e3799aee6cfc54be6720ec7

laanwj merged this on Jan 8, 2021

laanwj closed this on Jan 8, 2021

laanwj moved this from the "PRs" to the "Done" column in a project

sidhujag referenced this in commit 2d337cab7f on Jan 8, 2021

hebasto cross-referenced this on Apr 2, 2021 from issue ci: ccache does not work for macOS cross-compiling builds by hebasto

hebasto commented at 10:41 AM on April 2, 2021: member

The commit "depends: Pin clang search paths for darwin host" (https://github.com/bitcoin/bitcoin/pull/19683/commits/300733921863c176535806c40afdc813b99e7459) broke ccache behavior, see #21552.

kwvg referenced this in commit 5962a9b7ed on Jul 15, 2021

kwvg referenced this in commit 3af13157e8 on Jul 15, 2021

kwvg referenced this in commit 173e6cae27 on Jul 20, 2021

kwvg referenced this in commit a3b549cacb on Jul 20, 2021

kwvg referenced this in commit 7f2f6a3601 on Jul 20, 2021

kwvg referenced this in commit 6bbe705b9e on Jul 20, 2021

kwvg referenced this in commit a33c95b352 on Aug 1, 2021

kwvg referenced this in commit e02b47f538 on Aug 24, 2021

kwvg referenced this in commit 9ddcf29a56 on Aug 24, 2021

kwvg referenced this in commit 62be3e26e1 on Aug 24, 2021

kwvg referenced this in commit 8d1f21e473 on Aug 24, 2021

kwvg referenced this in commit 81389cd48e on Aug 25, 2021

kwvg referenced this in commit 158b9a2295 on Aug 25, 2021

kwvg referenced this in commit e99ad035d3 on Aug 26, 2021

kwvg referenced this in commit 8e1a0c18c5 on Aug 26, 2021

kwvg referenced this in commit e66c4ad05e on Aug 26, 2021

kwvg referenced this in commit 7e3f734c14 on Aug 27, 2021

kwvg referenced this in commit 6d24126aae on Aug 30, 2021

kwvg referenced this in commit 4327333c28 on Sep 1, 2021

kwvg referenced this in commit 6fa2b58a97 on Sep 1, 2021

kwvg referenced this in commit 1788e692af on Sep 1, 2021

kwvg referenced this in commit fa8747c63f on Sep 1, 2021

kwvg referenced this in commit 9408e0bf87 on Sep 2, 2021

kwvg referenced this in commit b6a51e6f47 on Sep 3, 2021

kwvg referenced this in commit 91ce135056 on Sep 3, 2021

kwvg referenced this in commit 7260597fc6 on Sep 3, 2021

AbcSxyZ cross-referenced this on Sep 22, 2021 from issue depends: Update OpenSSL to 1.0.2u and fix issues it introduces by patricklodder

hebasto cross-referenced this on Jan 30, 2022 from issue build: Replace `which` command with `command -v` by hebasto

str4d cross-referenced this on Apr 15, 2022 from issue Backport upstream macOS build changes from 2021 by str4d

gades referenced this in commit 74a5f17c3a on May 4, 2022

bitcoin locked this on Aug 16, 2022