Skip to content

Conversation

practicalswift
Copy link
Contributor

@practicalswift practicalswift commented Mar 10, 2020

Make it possible to build Boost dependency using a clang toolset (./b2 toolset=clang).

Submitted as a separate pull as required by MarcoFalke in #18288 (comment).

Fixes #15914.

Copy link
Member

@hebasto hebasto left a comment

Choose a reason for hiding this comment

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

Tested 43b1bb5 on Linux Mint 19.3:

$ g++ --version | grep g++
g++ (Ubuntu 7.5.0-3ubuntu1~18.04) 7.5.0
$ clang++ --version | grep clang
clang version 6.0.0-1ubuntu2 (tags/RELEASE_600/final)
$ rm -rf depends/built/x86_64-pc-linux-gnu/boost
$ make -C depends NO_QT=1 NO_UPNP=1 NO_WALLET=1 NO_QR=1 NO_ZMQ=1 > ~/default-log
$ rm -rf depends/built/x86_64-pc-linux-gnu/boost
$ make -C depends NO_QT=1 NO_UPNP=1 NO_WALLET=1 NO_QR=1 NO_ZMQ=1 CC=clang CXX=clang++ > ~/clang-log
$ diff -u ~/default-log ~/clang-log 
--- /home/hebasto/default-log	2020-03-10 20:54:11.728978765 +0200
+++ /home/hebasto/clang-log	2020-03-10 20:51:48.789642887 +0200
@@ -87,49 +87,45 @@
 
         mkdir -p "bin.v2/libs/filesystem/build"
     
-common.mkdir bin.v2/libs/system
-
-        mkdir -p "bin.v2/libs/system"
-    
 common.mkdir bin.v2/libs/filesystem/build/gcc-7.5.0
 
         mkdir -p "bin.v2/libs/filesystem/build/gcc-7.5.0"
     
-common.mkdir bin.v2/libs/system/build
+common.mkdir bin.v2/libs/system
 
-        mkdir -p "bin.v2/libs/system/build"
+        mkdir -p "bin.v2/libs/system"
     
 common.mkdir bin.v2/libs/filesystem/build/gcc-7.5.0/release
 
         mkdir -p "bin.v2/libs/filesystem/build/gcc-7.5.0/release"
     
-common.mkdir bin.v2/libs/system/build/gcc-7.5.0
+common.mkdir bin.v2/libs/system/build
 
-        mkdir -p "bin.v2/libs/system/build/gcc-7.5.0"
+        mkdir -p "bin.v2/libs/system/build"
     
 common.mkdir bin.v2/libs/filesystem/build/gcc-7.5.0/release/link-static
 
         mkdir -p "bin.v2/libs/filesystem/build/gcc-7.5.0/release/link-static"
     
-common.mkdir bin.v2/libs/system/build/gcc-7.5.0/release
+common.mkdir bin.v2/libs/system/build/gcc-7.5.0
 
-        mkdir -p "bin.v2/libs/system/build/gcc-7.5.0/release"
+        mkdir -p "bin.v2/libs/system/build/gcc-7.5.0"
     
 common.mkdir bin.v2/libs/filesystem/build/gcc-7.5.0/release/link-static/threading-multi
 
         mkdir -p "bin.v2/libs/filesystem/build/gcc-7.5.0/release/link-static/threading-multi"
     
-common.mkdir bin.v2/libs/system/build/gcc-7.5.0/release/link-static
+common.mkdir bin.v2/libs/system/build/gcc-7.5.0/release
 
-        mkdir -p "bin.v2/libs/system/build/gcc-7.5.0/release/link-static"
+        mkdir -p "bin.v2/libs/system/build/gcc-7.5.0/release"
     
 common.mkdir bin.v2/libs/filesystem/build/gcc-7.5.0/release/link-static/threading-multi/visibility-hidden
 
         mkdir -p "bin.v2/libs/filesystem/build/gcc-7.5.0/release/link-static/threading-multi/visibility-hidden"
     
-common.mkdir bin.v2/libs/system/build/gcc-7.5.0/release/link-static/threading-multi
+common.mkdir bin.v2/libs/system/build/gcc-7.5.0/release/link-static
 
-        mkdir -p "bin.v2/libs/system/build/gcc-7.5.0/release/link-static/threading-multi"
+        mkdir -p "bin.v2/libs/system/build/gcc-7.5.0/release/link-static"
     
 gcc.compile.c++ bin.v2/libs/filesystem/build/gcc-7.5.0/release/link-static/threading-multi/visibility-hidden/codecvt_error_category.o
 
@@ -159,6 +155,10 @@
 
     "g++" "-m64"   -fvisibility-inlines-hidden -std=c++11 -fvisibility=hidden   -fPIC   -I/home/hebasto/GitHub/bitcoin/depends/x86_64-pc-linux-gnu/include     -m64 -pthread -O3 -finline-functions -Wno-inline -Wall -fvisibility=hidden  -DBOOST_ALL_NO_LIB=1 -DBOOST_FILESYSTEM_STATIC_LINK=1 -DNDEBUG  -I"." -c -o "bin.v2/libs/filesystem/build/gcc-7.5.0/release/link-static/threading-multi/visibility-hidden/windows_file_codecvt.o" "libs/filesystem/src/windows_file_codecvt.cpp"
 
+common.mkdir bin.v2/libs/system/build/gcc-7.5.0/release/link-static/threading-multi
+
+        mkdir -p "bin.v2/libs/system/build/gcc-7.5.0/release/link-static/threading-multi"
+    
 common.mkdir bin.v2/libs/system/build/gcc-7.5.0/release/link-static/threading-multi/visibility-hidden
 
         mkdir -p "bin.v2/libs/system/build/gcc-7.5.0/release/link-static/threading-multi/visibility-hidden"

It seems clang++, even CXX=clang++ is passed, is not used.

@practicalswift
Copy link
Contributor Author

@hebasto Try passing in boost_toolset_linux='clang' boost_cxx='clang++' :) See usage example in #18288.

@hebasto
Copy link
Member

hebasto commented Mar 10, 2020

@practicalswift

Try passing in boost_toolset_linux='clang' boost_cxx='clang++' :) See usage example in #18288.

Thank you. Now it works.

But, from my understanding, this PR couldn't claim to be a fix of #15914 as the case was with usage of CC and CXX variables.

Copy link
Member

@hebasto hebasto left a comment

Choose a reason for hiding this comment

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

ACK 43b1bb5, tested on Linux Mint 19.3: depends compiled with boost_toolset_linux='clang' boost_cxx='clang++' command line options, then make && make check.

@practicalswift
Copy link
Contributor Author

practicalswift commented Mar 11, 2020

But, from my understanding, this PR couldn't claim to be a fix of #15914 as the case was with usage of CC and CXX variables.

I see your point. I opened #15914 and my issue is fixed as long as I have a way to build Boost using Clang by setting environment variables :) I've now updated #15914 to clarify that.

I don't know enough about how the depends build system is supposed to work so I'll let someone else tackle the interaction between boost_toolset_linux <> CC and boost_cxx <> CXX. Note that one complication is that boost_toolset_linux must be gcc or clang also when the compiler is say gcc-8 or clang-10.

@maflcko
Copy link
Member

maflcko commented Mar 11, 2020

cc @fanquake , @dongcarl

@laanwj
Copy link
Member

laanwj commented Mar 11, 2020

Concept ACK.

I understand that this solves your immediate problem, but also think that heeding CXX/CC instead of needing boost-specific environment magic (please document this!) would be more consistent.

@maflcko
Copy link
Member

maflcko commented Mar 11, 2020

This seems reasonable to do if it doesn't break anything like our gitian or guix builds, which is why I pinged fanquake and dongcarl.

For the next release, 0.21, there won't be any boost hopefully.

@practicalswift
Copy link
Contributor Author

practicalswift commented Mar 11, 2020

@hebasto @laanwj Pushed another commit. The following should now work:

$ make -C depends/ clean
$ make -C depends/ NO_QT=1 NO_UPNP=1 NO_WALLET=1 NO_QR=1 NO_ZMQ=1 V=1 CXX=clang++-8

Please note that clang is special-cased due to b2 wanting toolset=clang (and not toolset=clang-8, toolset=clang++-8, etc.). Leaving the gcc handling as-is.

@practicalswift practicalswift changed the title depends: Make it possible to build Boost dependency using a toolset other than gcc (./b2 toolset=gcc) depends: Make it possible to build Boost dependency using a clang toolset (./b2 toolset=clang) Mar 11, 2020
@Sjors
Copy link
Member

Sjors commented Mar 12, 2020

On macOS 10.15.3 I don't see any difference with 0f111aa, which I assume is good news, because it always uses clang. src/bitcoind also still compiles.

cd depends
make
....
/Users/sjors/dev/bitcoin-depends/depends/work/download/boost-1_70_0/boost_1_70_0.tar.bz2.temp: OK
Extracting boost...
/Users/sjors/dev/bitcoin-depends/depends/sources/boost_1_70_0.tar.bz2: OK
Preprocessing boost...
Configuring boost...
Building Boost.Build engine with toolset darwin... tools/build/src/engine/bin.macosxx86_64/b2
Unicode/ICU support for Boost.Regex?... disabled.
Generating Boost.Build configuration in project-config.jam for darwin...

Bootstrapping is done. To build, run:

    ./b2
...
clang-darwin.compile.c++ bin.v2/libs/filesystem/build/clang-darwin-11.0/release/link-static/threading-multi/visibility-hidden/codecvt_error_category.o
...

@dongcarl
Copy link
Contributor

Going to look into fixing this at a more fundamental level... The depends/hosts files need some tuneup.

@practicalswift
Copy link
Contributor Author

@dongcarl Excellent news! I'd be glad to close this PR as soon as we have a better alternative. Don't hesitate to ping me for review :)

fanquake added a commit that referenced this pull request May 30, 2020
f0d7ed1 depends: Propagate only specific CLI variables to sub-makes (Carl Dong)
0a33803 depends: boost: Use clang toolset if clang in CXX (Carl Dong)
1ce74bc depends: boost: Split target-os from toolset (Carl Dong)
2d4e480 depends: boost: Specify toolset to bootstrap.sh (Carl Dong)
3d6603e depends: Propagate well-known vars into depends (Carl Dong)

Pull request description:

  From: #18308 (comment)

  The following monstrosity is quite useful when invoked inside `depends`, and reviewers can use it to compare the behaviour of this change against master.
  ```bash
  make print-{{,{host,{,{i686,x86_64,riscv64}_}linux}_}{CC,CXX},boost_{cc,cxx}}
  ```

  It would also be helpful to make sure that setting `HOST`, `CC`, and `CXX` does the right thing. The 3 hosts I found offered good coverage were: `{x86_64,i686,riscv64}-linux-gnu`. As we special-case the `x86_64` and `i686` hosts in `depends/hosts/linux.mk`, and `riscv64` is a sanity check for a non-special-cased host.

ACKs for top commit:
  hebasto:
    ACK f0d7ed1, tested on Linux Mint 19.3 (x86_64):
  practicalswift:
    ACK f0d7ed1 -- patch looks correct
  laanwj:
    Code review and concept ACK f0d7ed1
  ryanofsky:
    Code review ACK f0d7ed1. Changes since last review: adding comment explaining check for predefined make variables, dropping freetype commit, adding commit whitelisting overrides for recursive makes

Tree-SHA512: b6b8e76f713c26a0add6cd685824e2f5639109236ee9f89338f7c79cb1b1f2c3897bfb62b80b023d6d1943b5a6eb282a2f827f1f499c5e556eca015d6635fa65
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request May 31, 2020
f0d7ed1 depends: Propagate only specific CLI variables to sub-makes (Carl Dong)
0a33803 depends: boost: Use clang toolset if clang in CXX (Carl Dong)
1ce74bc depends: boost: Split target-os from toolset (Carl Dong)
2d4e480 depends: boost: Specify toolset to bootstrap.sh (Carl Dong)
3d6603e depends: Propagate well-known vars into depends (Carl Dong)

Pull request description:

  From: bitcoin#18308 (comment)

  The following monstrosity is quite useful when invoked inside `depends`, and reviewers can use it to compare the behaviour of this change against master.
  ```bash
  make print-{{,{host,{,{i686,x86_64,riscv64}_}linux}_}{CC,CXX},boost_{cc,cxx}}
  ```

  It would also be helpful to make sure that setting `HOST`, `CC`, and `CXX` does the right thing. The 3 hosts I found offered good coverage were: `{x86_64,i686,riscv64}-linux-gnu`. As we special-case the `x86_64` and `i686` hosts in `depends/hosts/linux.mk`, and `riscv64` is a sanity check for a non-special-cased host.

ACKs for top commit:
  hebasto:
    ACK f0d7ed1, tested on Linux Mint 19.3 (x86_64):
  practicalswift:
    ACK f0d7ed1 -- patch looks correct
  laanwj:
    Code review and concept ACK f0d7ed1
  ryanofsky:
    Code review ACK f0d7ed1. Changes since last review: adding comment explaining check for predefined make variables, dropping freetype commit, adding commit whitelisting overrides for recursive makes

Tree-SHA512: b6b8e76f713c26a0add6cd685824e2f5639109236ee9f89338f7c79cb1b1f2c3897bfb62b80b023d6d1943b5a6eb282a2f827f1f499c5e556eca015d6635fa65
@practicalswift practicalswift deleted the b2-toolset-clang branch April 10, 2021 19:40
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Aug 18, 2022
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.

"make depends" builds boost with g++ regardless of environment variable settings
7 participants