https://ci.appveyor.com/project/ken2812221/bitcoin/build/patch-4.407
The build time reduced from 18 mins to 7 mins.
*.iobj and *.ipdb is a MSVC built-in cache. (Link-time cache)
Any potential drawbacks?
I am not sure if there is any. This is my first time trying this stuff.
Oh, there is one. We cannot use appveyor logger anymore, it’s incompatible with clcache.
@ken2812221 Actually there are only two warnings (C4244 and C4267) that make the log output unreadable due to the sheer amount of them. Could you try disabling those two warnings specifically instead of disabling all warnings?
I analyzed the warnings:
0$ grep ": warning " log.txt | cut -f4- -d" " | cut -f1 -d: | grep ^C | sort | uniq -c
1 1 C4018 == signed/unsigned mismatch
2 8 C4101 == unreferenced local variable
32056 C4244 == conversion from […] to […], possible loss of data
41482 C4267 == conversion from […] to […], possible loss of data
5 1 C4305 == truncation from 'int' to 'bool'
6 1 C4312 == conversion from 'int' to 'void *' of greater size
7 1 C4334 == result of 32-bit shift implicitly converted to 64 bits (was 64-bit shift intended?)
8 8 C4715 == not all control paths return a value
The total number of warnings 3558 is reduced to only 20 warnings when excluding C4244 and C4267.
Having warnings enabled adds additional value to the appveyor integration by giving access to the MSVC warnings (which are good!) also to developers who don’t use Windows. I know I enjoy having access to them :-)
@ken2812221 Excellent!
utACK 637f88c7e3142dde123b40982bb6418e640c3e42
C4715: '[…]': not all control paths return a value too? We intentionally assert(false); unconditionally (in default:) in eight places which we don’t want to change. This triggers C4715. That warning is not actionable in our case so I think disabling it is a net win.
Concept ACK, but I don’t know enough of appveyor and clcache to actually review this.
Maybe @sipsorcery ?
@ken2812221 are you currently able to get the build based on the appveyor.yml file to work? My attempts are failing with:
LINK : fatal error C1047: The object or library file ‘C:\Tools\vcpkg\installed\x64-windows-static\lib\libzmq-mt-s-4_3_1.lib’ was created with an older compiler than other objects; rebuild old objects and libraries [C:\projects\bitcoin-72c17\build_msvc\bitcoind\bitcoind.vcxproj] LINK : fatal error LNK1257: code generation failed [C:\projects\bitcoin-72c17\build_msvc\bitcoind\bitcoind.vcxproj]
So far I’ve been unsuccessful in getting the libzmq dependency to update (my mingw32 build is also failing because of libzmq so something has changed there recently). I have managed to get my custom AppVeyor Bitcoin build that doesn’t use appveyor.yml to work so I’m pretty sure this failure is due to the libzmq dependency needing to be updated.
tACK
Build time is reduced from ~22mins to ~11mins. @ken2812221 do you think it would be worth adding artifacts to the AppVeyor? It would be nice to be able to use the results of the build even if it’s just for sanity testing.
0artifacts:
1- path: build_msvc\%PLATFORM%\%CONFIGURATION%\*.exe
2 name: Executables
do you think it would be worth adding artifacts to the AppVeyor?
There is a previous discussion in #13855. Many devs thought that uploading executables is not a good idea.