Introduce Appveyor CI for MSVC. This would require the owner adding appveyor to this repo. Also fix some MSVC incompatible code.
This appveyor.yml file is modified from @sipsorcery and @NicolasDorier ’s code in #12613.
Appveyor CI result: https://ci.appveyor.com/project/ken2812221/bitcoin/build/1.0.151
Concept ACK
More diversity in testing is better.
FWIW, it seems like a few tests are missing from test_bitcoin.vcxproj – is that intentional?
0$ for FILE in $(git ls-files -- "*_tests.cpp"); do FILE_BASENAME=$(basename ${FILE}); grep -q ${FILE_BASENAME} build_msvc/test_bitcoin/test_bitcoin.vcxproj || echo "* ${FILE_BASENAME} missing in test_bitcoin.vcxproj"; done
1* descriptor_tests.cpp missing in test_bitcoin.vcxproj
2* key_io_tests.cpp missing in test_bitcoin.vcxproj
3* merkleblock_tests.cpp missing in test_bitcoin.vcxproj
4* txvalidation_tests.cpp missing in test_bitcoin.vcxproj
btw, msbuild support wildcard replacing all ..\..\src\test\xxxxx_tests.cpp by one ..\..\src\test\*.cpp might work.
EDIT: Or maybe ..\..\src\test\*_tests.cpp
utACK a87072f4b9ce2e4020dd2a1ca4ed7cb227757090
Could you please post instructions for maintainers how to enable this appveyor thing?
Tested ACK https://github.com/bitcoin/bitcoin/commit/a87072f4b9ce2e4020dd2a1ca4ed7cb227757090 @NicolasDorier’s suggestion is a good one and would help avoid files missing in the future (it was human error on my behalf that caused them to be missing in the first place).
0<ItemGroup>
1 <ClCompile Include="..\..\src\test\*.cpp" Exclude="..\..\src\test\test_bitcoin_fuzzy.cpp" />
2 <ClCompile Include="..\..\src\wallet\test\*.cpp" />
3 </ItemGroup>
@MarcoFalke if there is a webhook on the bitcoin/bitcoin project under Settings->WebHooks and we can get it that would allow us to initiate the AppVeyor builds automatically whenever there is a commit. At the moment I kick off a build whenever I see a Twitter commit message.
@practicalswift For the other projects it’s not really possible to use wildcards if we also want to maintain the same build components as the Makefile. In particular libbitcoin_common, libbitcoin_util, libbitcoin_consensus and libbitcoin_server all include *.cpp files from /src.
One alternative solution would be to write a script that parses the Makefile to automatically build the Visual Studio project files. I’ll put it on the list :-).
A small note on the AppVeyor build in this PR.
The URL to install the bzip dependency in the AppVeyor version of vcpkg (the Microsoft C++ dependency that makes building bitcoin-core straight forward) is currently broken as per https://github.com/Microsoft/vcpkg/pull/4046.
The consequence is that anyone that forks bitcoin-core and creates their own AppVeyor job will end up with a failing job. At some point AppVeyor will update their vcpkg version and this will be fixed. In the meantime it’s possible to workaround the problem by manually updating the vcpkg files using:
0cd c:\tools\vcpkg
1git pull
I don’t think it’s worth adding these commands to the appveyor.yml as it would potentially result in dependency updates that a user may not be expecting.
Cool they replied on https://help.appveyor.com/discussions/problems/15817-vcpkg-updates
0install:
1- cmd: |
2 cd "C:\Tools\vcpkg"
3 git pull
4 .\bootstrap-vcpkg.bat
5 cd %appveyor_build_folder%
@MarcoFalke , you will need that for now if you want to include to github. That said they will fix in few weeks.
@sipsorcery We would definitely recommend what @NicolasDorier suggests – the best fix here is to checkout a specific vcpkg git commit in your appveyor.yml:
0cd C:\Tools\vcpkg
1git checkout <some sha, perhaps today's master>
2.\bootstrap-vcpkg.bat
3cd %appveyor_build_folder%
Whenever you want to update your dependencies (say, to fix issues like this), you just need to change the commit id.
Post merge: I’m unsure about the usefulness of this. This requires documentation. I have no clue how to make #14032 pass AppVeyor… Somehow I have the feeling that this sets an unexpected heavy burden on those not primarily interested in MSVC support.
IMO MSVC compatibility should be maintained by those interested in it.
@sipsorcery Agreed!
Could a way to solve this be to auto-generate the project files (or at least the parts referencing files) to make sure they’re automatically changed when new files are added/removed?
The only input needed for such auto-generation would be find src/ -type f? :-)
See previous discussion in #11526 (comment) and #11526 (comment) about auto-generation.
blocking a PR based on the msvc build is not ideal.
From irc yesterday (https://botbot.me/freenode/bitcoin-core-dev/msg/103712220/), it sounds like appveyor failures will not actually block merges, so they might not be a real problem (though annoying to look at).
Could a way to solve this be to auto-generate the project files
An easier way to alleviate this might just be to use wildcards more places in the project files. Autogeneration could help or hurt, depending how it is implemented.
The options I can think of:
For my own purposes option 1 is by far the most useful. Having a Visual Studio project configuration that directly reflects the current bitcoin core libraries makes building and debugging straight forward and is the by far the biggest benefit for me. It’s also not a great burden to update the project files when the build breaks but that’s based on only building the master branch.
For options 2 and 3 it would make building on Windows easier but be worse for debugging and comprehending the code.