ci: Set MSVC toolset version explicitly #28934

1. 
   hebasto commented at 3:57 pm on November 24, 2023: member

   This PR is an alternative to #28905 and reverts it.

   To avoid toolset version incompatibilities, which result in errors like this:

   0LINK : fatal error C1900: Il mismatch between 'P1' version '20230904' and 'P2' version '20221215' [D:\a\bitcoin\bitcoin\build_msvc\test_bitcoin\test_bitcoin.vcxproj]
   1LINK : fatal error LNK1257: code generation failed [D:\a\bitcoin\bitcoin\build_msvc\test_bitcoin\test_bitcoin.vcxproj]
   2LINK : fatal error LNK1327: failure during running link.exe [D:\a\bitcoin\bitcoin\build_msvc\test_bitcoin\test_bitcoin.vcxproj]
   

   it is enough to set it explicitly in the vcpkg triplet file (see the second commit). The VCToolsVersion environment variable is set by the ilammy/msvc-dev-cmd action.

   Please note that the #28905 is not optimal:

   I guess this is something we’ll just have to maintain forever? That’s a shame, because it also adds ~30% runtime to this CI job.

2. ci: Run vcpkg with path prefix

   The GHA VS installation includes its own vcpkg package manager, which is
   available since VS 17.6. This change avoids any ambiguity about which
   copy of vcpkg we run.

   4335e55359
3. ci: Set MSVC toolset version explicitly

   This change avoids toolset incompatibilities that cause linker errors.

   1a889f7ea0
4. Revert "ci: Avoid toolset ambiguity that MSVC can't handle"

   This reverts commit 91d5bd8ac9a28725c735f8e6900bc85673bb190a.

   70100f8584
5. 
   DrahtBot commented at 3:57 pm on November 24, 2023: contributor

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

   Code Coverage

   For detailed information about the code coverage, see the test coverage report.

   Reviews

   See the guideline for information on the review process.

   Type	Reviewers
   ACK	pablomartin4btc
   Stale ACK	sipsorcery

   If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

6. DrahtBot added the label Tests on Nov 24, 2023
7. pablomartin4btc approved
8. 
   pablomartin4btc commented at 5:31 pm on November 24, 2023: member

   ACK 70100f8584c762cbc6cbc09d05cc4e9d22a3ec75 since I’ve reviewed to be reverted #28905.
9. 
   sipsorcery commented at 7:47 pm on November 25, 2023: contributor

   Does the CI job still cache the vcpkg dependencies? Will this PR still work if there a minor version update on the build image?
10. 
    hebasto commented at 7:51 pm on November 25, 2023: member

    Does the CI job still cache the vcpkg dependencies?

    It does.

    Will this PR still work if there a minor version update on the build image?

    I’ve added the toolset version to the cache key: https://github.com/bitcoin/bitcoin/blob/70100f8584c762cbc6cbc09d05cc4e9d22a3ec75/.github/workflows/ci.yml#L250

    so the cache will be invalidated if it is changed.

11. 
    sipsorcery commented at 7:54 pm on November 25, 2023: contributor

    utACK 4335e553596a0ad5b136d3aa9fc898c906d226b4.

    utACK 70100f8584c762cbc6cbc09d05cc4e9d22a3ec75.

12. DrahtBot requested review from sipsorcery on Nov 25, 2023
13. 
    hebasto commented at 7:56 pm on November 25, 2023: member

    @sipsorcery

    utACK 4335e55.

    Wrong hash?

    :)

14. 
    hebasto commented at 10:55 am on November 27, 2023: member

    cc @maflcko @fanquake
15. 
    maflcko commented at 11:10 am on November 27, 2023: member

    Seems fine to do this, if this makes CI faster and there are no major downsides
16. fanquake merged this on Nov 28, 2023
17. fanquake closed this on Nov 28, 2023

    ---

18. hebasto deleted the branch on Nov 28, 2023
19. bitcoin locked this on Nov 27, 2024

Contributors
hebasto DrahtBot pablomartin4btc sipsorcery maflcko

Review Requested
sipsorcery

Labels
Tests