build: don't show ccache summary with MSVC #31983

pull fanquake wants to merge 1 commits into bitcoin:master from fanquake:msvc_no_ccache changing 1 files +2 −0

fanquake commented at 11:47 AM on March 4, 2025: member

Set WITH_CCACHE to OFF for MSVC, so it doesn't show as ON in the configure summary.

Fixes #31771.
DrahtBot commented at 11:48 AM on March 4, 2025: contributor



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



Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/31983.



Reviews

See the guideline for information on the review process.

Type Reviewers

ACK hebasto

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.
DrahtBot added the label Build system on Mar 4, 2025
hebasto commented at 11:53 AM on March 4, 2025: member

Ccache could be supported for MSVC in the future, which might lead to these changes being reverted. Why not use https://github.com/bitcoin/bitcoin/pull/30861/commits/c19a187c42fe867d61ca5dbd48ae18f15201839f instead?
fanquake commented at 11:58 AM on March 4, 2025: member

Why not use https://github.com/bitcoin/bitcoin/commit/c19a187c42fe867d61ca5dbd48ae18f15201839f instead?

From my read of #30861 nobody seems to agree that we should make this change? #30861 (review)

Type	Reviewers
ACK	hebasto

build: don't use ccache with MSVC

Set WITH_CCACHE to OFF for MSVC, so it doesn't show as ON in the
configure summary.

Fixes #31771.

c718bffc36

in CMakeLists.txt:685 in e682fc335d outdated

 679 | @@ -680,7 +680,9 @@ include(FlagsSummary)
 680 |  flags_summary()
 681 |  message("Attempt to harden executables ......... ${ENABLE_HARDENING}")
 682 |  message("Treat compiler warnings as errors ..... ${WERROR}")
 683 | -message("Use ccache for compiling .............. ${WITH_CCACHE}")
 684 | +if(NOT MSVC)
 685 | +  message("Use ccache for compiling .............. ${WITH_CCACHE}")
 686 | +endif()

hebasto commented at 12:18 PM on March 4, 2025:

The MSVC variable is defined based on the compiler. However, the current ccache settings rely on the <LANG>_COMPILER_LAUNCHER properties, which are considered depending on the generator rather than the compiler.

fanquake commented at 2:54 PM on March 4, 2025:

Reworked this.

fanquake force-pushed on Mar 4, 2025
hebasto approved
hebasto commented at 2:55 PM on March 4, 2025: member

ACK c718bffc361a1227de9deb823c35dd11c8570ddd, I have reviewed the code and it looks OK.
fanquake merged this on Mar 5, 2025
fanquake closed this on Mar 5, 2025
fanquake deleted the branch on Mar 5, 2025

Contributors

Labels

Linked (view graph)

#31771 cmake: incorrectly reporting MSVC as using ccache

build: don't show ccache summary with MSVC #31983

Code Coverage & Benchmarks

Reviews