-
Notifications
You must be signed in to change notification settings - Fork 38.2k
doc: Document compiler configuration for native depends packages #33902
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
base: master
Are you sure you want to change the base?
Conversation
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. Code Coverage & BenchmarksFor details see: https://corecheck.dev/bitcoin/bitcoin/pulls/33902. ReviewsSee the guideline for information on the review process.
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update. ConflictsReviewers, this pull request conflicts with the following ones:
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. |
hebasto
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Concept ACK. Thanks!
|
Forgot to say, very happy to drop the second commit if wanted. |
| To ensure both native and target use Clang: | ||
|
|
||
| ``` | ||
| make build_CC=clang build_CXX=clang++ host_CC=clang host_CXX=clang++ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How is this better than make build_CC=clang build_CXX=clang++ CC=clang CXX=clang++?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They should be exactly equivalent.
I used my version as I had just described build_CC and host_CC and it would seem a little odd (to me) to exemplify with build_CC and CC, although as I re-read it now this example appears in the CC, CXX section.
I think therefore I should set make build_CC=clang build_CXX=clang++ CC=clang CXX=clang++ as the primary example for this section, and perhaps use make build_CC=clang build_CXX=clang++ host_CC=clang host_CXX=clang++ as an "ultimately explicit" final variation?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In most situations where the compiler needs to be specified, users simply run:
make CC=clang CXX=clang++
Only in rare cases (#33902 (comment), #33902 (comment), #33902 (comment)) does this need to be extended to:
make CC=clang CXX=clang++ build_CC=clang build_CXX=clang++
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only in rare cases
I don't think wanting to build without having a GCC installation should be considered rare? Other than the clang-only Nix example linked too above, we are actively trying to make that "normal" for some of our builds, i.e #30206.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They should be exactly equivalent.
Unfortunately this isn't the case. make build_CC=clang build_CXX=clang++ host_CC=clang host_CXX=clang++ doesn't work properly on a machine with no g++, because Boosts CMake will still try and find g++, and fail. Arguably this is also a CMake issue that we should fix/workaround in some way, because there's no reason for Boost to try and find a compiler, when we aren't compiling anything, and only need to copy headers. In this case, you do need to use make build_CC=clang build_CXX=clang++ CC=clang CXX=clang++.
For example, on a Chimera Linux machine:
gmake build_CC=clang build_CXX=clang++ host_CC=clang host_CXX=clang++ -C depends/ -j12 build_TAR=gtar
gmake: Entering directory '/bitcoin/depends'
Extracting native_capnp...
/bitcoin/depends/sources/capnproto-cxx-1.2.0.tar.gz: OK
Preprocessing native_capnp...
Configuring native_capnp...
-- The CXX compiler identification is Clang 21.1.4
-- Detecting CXX compiler ABI info
<snip>
done
Configuring boost...
CMake Error at /usr/share/cmake-4.1/Modules/CMakeDetermineCXXCompiler.cmake:47 (message):
Could not find compiler set in environment variable CXX:
g++.
Call Stack (most recent call first):
CMakeLists.txt:13 (project)
CMake Error: CMAKE_CXX_COMPILER not set, after EnableLanguage
-- Configuring incomplete, errors occurred!
gmake: *** [funcs.mk:344: /bitcoin/depends/aarch64-unknown-linux-musl/.boost_stamp_configured] Error 1Using CC=clang CXX=clang++:
build_CC=clang build_CXX=clang++ CC=clang CXX=clang++ -C depends/ -j12 build_TAR=gtar
gmake: Entering directory '/bitcoin/depends'
Extracting native_capnp...
/bitcoin/depends/sources/capnproto-cxx-1.2.0.tar.gz: OK
Preprocessing native_capnp...
Configuring native_capnp...
-- The CXX compiler identification is Clang 21.1.4
<snip>
done
Configuring boost...
-- The CXX compiler identification is Clang 21.1.4
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: /usr/sbin/clang++ - skipped
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- Boost: using system layout: include, bin, lib/, lib//cmake
maflcko
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
review ACK 1757764 🌴
Show signature
Signature:
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: review ACK 17577646f77e20783ffdd9f322f85e96da2265 🌴
b6tuB1ff+N8xGzUilBsW1CSxIOpk4gK7SE473C8JMS+vg4aU5H4YbP04c0HL3NEDxTZkxKFJBRcJ9RVAQ9t7DQ==
depends/README.md
Outdated
| Example that may not work as expected (native tools still use GCC): | ||
|
|
||
| ``` | ||
| make CC=clang CXX=clang++ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This example uses Makefile variable CC and CXX, but the section title refers to "Environment Variables". Both approaches work, of course, but the section could be made clearer.
depends/README.md
Outdated
|
|
||
| Setting `CC` and `CXX` environment variables will override target compilers but **NOT** native tool compilers. Native tools will still use the default GCC unless explicitly overridden with `build_*` variables. | ||
|
|
||
| Example that may not work as expected (native tools still use GCC): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This comes across as a bit discouraging and confusing. When I say “I build depends with Clang”, I’m referring to the host-specific packages. I’m not concerned about which compiler is used for the native tools, as long as they build without issues.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I’m not concerned about which compiler is used for the native tools, as long as they build without issues.
at least for me the issue was that they did not build at all, because the gcc version was ancient on OpenSuse Leap (or Alma Linux 8, ....)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If using something like a Nix devShell with no gcc available at all, compilation just fails too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I’m not concerned about which compiler is used for the native tools, as long as they build without issues.
at least for me the issue was that they did not build at all, because the gcc version was ancient on OpenSuse Leap (or Alma Linux 8, ....)
The same issue occurs on NetBSD.
1757764 to
b1b559f
Compare
|
|
||
| apt install bison g++ ninja-build pkgconf python3 xz-utils | ||
| ``` | ||
| apt install bison g++ ninja-build pkgconf python3 xz-utils |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Other native package, not only native_qt, require g++.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree, but going to leave that from this PR for now.
hebasto
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK b1b559f.
Previously one had to read the Makefile (and various *.mk configuration files) to see how to correctly override CC and CXX when building native depends packages. Detail this in README.md to make it clearer.
As we are touching the file use modern codeblock syntax throughout.
b1b559f to
7dd714a
Compare
hebasto
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
re-ACK 7dd714a.
|
re-ACK 7dd714a 🔔 Show signatureSignature: |
|
Could use |
fanquake
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See #33902 (comment).
Fixes: #33859
Previously one had to read the Makefile (and various *.mk configuration
files) to see how to correctly override CC and CXX when building native
depends packages.
Detail this in README.md to make it clearer.