Concept ACK

More diversity in testing is better.

FWIW, it seems like a few tests are missing from test_bitcoin.vcxproj – is that intentional?

$ 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
* descriptor_tests.cpp missing in test_bitcoin.vcxproj
* key_io_tests.cpp missing in test_bitcoin.vcxproj
* merkleblock_tests.cpp missing in test_bitcoin.vcxproj
* txvalidation_tests.cpp missing in test_bitcoin.vcxproj

@practicalswift I'm not sure if it is intentional. Maybe @sipsorcery forgot to add them. I'll add those files after the test pass.

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?

I guess https://ci.appveyor.com/projects/new on "Public GitHub repositories"?

@MarcoFalke Yes, note that appveyor does not connect with github team. So the owner need to change his account name on appveyor to "bitcoin" to get "bitcoin" project prefix(https://ci.appveyor.com/account), but that is not necessary.

Tested ACK a87072f

@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).

<ItemGroup>
  <ClCompile Include="..\..\src\test\*.cpp" Exclude="..\..\src\test\test_bitcoin_fuzzy.cpp" />
  <ClCompile Include="..\..\src\wallet\test\*.cpp" />
  </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.

@sipsorcery Would it be possible to use wildcards in the other project files to make the maintenance of these scripts easier? :-)

It must run on pull requests. Otherwise we can't enable it on the master branch without having to fix each failure post merge.

@practicalswift That might not happen on other project files because that would cause link error.

@MarcoFalke Yes, it can run on pull request https://ci.appveyor.com/project/ken2812221/bitcoin/build/1.0.151

@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 :-).

Tested ACK 1f6ff04

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 microsoft/vcpkg#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:

cd c:\tools\vcpkg
git 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.

@sipsorcery maybe pulling and just ckecking out specifc commit so that updates on appveryor does not have impact.

Before doing a PR I'll try and ascertain how/when AppVeyor update vcpkg https://help.appveyor.com/discussions/problems/15817-vcpkg-updates.

@MarcoFalke Did you have any trouble while setting up Appveyor CI?

Cool they replied on https://help.appveyor.com/discussions/problems/15817-vcpkg-updates

install:
- cmd: |
    cd "C:\Tools\vcpkg"
    git pull
    .\bootstrap-vcpkg.bat
    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.

@laanwj @ken2812221 I have set this up through DrahtBot. Though, it wouldn't run on pull requests, it seems.

@MarcoFalke Can you take a look at https://github.com/bitcoin/bitcoin/settings/hooks
It should enable pull request

@sipsorcery We would definitely recommend what @NicolasDorier suggests -- the best fix here is to checkout a specific vcpkg git commit in your appveyor.yml:

cd C:\Tools\vcpkg
git checkout <some sha, perhaps today's master>
.\bootstrap-vcpkg.bat
cd %appveyor_build_folder%

Whenever you want to update your dependencies (say, to fix issues like this), you just need to change the commit id.

@MarcoFalke Could you enable rolling build for Appveyor?
https://www.appveyor.com/docs/build-configuration/#rolling-builds

Hmm, that would make it impossible to run multiple pull requests?

@MarcoFalke If you push to the same PR, it would cancel the previous one and does not affect other PRs.

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.

The project files for the msvc build are currently not automatically synchronised with the Makefiles. Whenever a new class file is added or removed the msvc build will break. Until we write a script to keep them in sync I agree with @jonasschnelli in that blocking a PR based on the msvc build is not ideal.

@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:

1. Leave the Visual Studio project files as is and manually update the project files as class files are added and removed. Over the last 12 months I’d estimate I’ve updated the project files about a dozen times,
2. Investigate switching to a wildcard project file,
3. Switch from Makefile to CMake to allow a wider variety of platform builds from a single configuration.

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.