build: Cleaned up and consolidated msbuild files (no code changes) #16235

1. 
   sipsorcery commented at 8:12 PM on June 18, 2019: member

   This PR does the following:

   • Big cleanup of the msbuild project files and consolidation of common build parameters into a single common include,
   • Gets ready for switch to building with Visual Studio 2019 and Platform Toolset v142,
   • Will allow me to rebase #15529 to include the Qt Projects in the msbuild config.
2. fanquake added the label Build system on Jun 18, 2019
3. fanquake added the label Windows on Jun 18, 2019
4. 
   MarcoFalke commented at 8:45 PM on June 18, 2019: member

   The linter complains about white space. Idk, are you using the windows-new-line-encoding?

5. 
   MarcoFalke commented at 8:45 PM on June 18, 2019: member

   Any suggestions on who is qualified to review this?

6. 
   sipsorcery commented at 9:04 PM on June 18, 2019: member

   The linter complains about white space. Idk, are you using the windows-new-line-encoding? @MarcoFalke not intentionally.

   Is there a trick to running the linter script locally? I tried test/lint/lint-all.sh from the root of my git repo but it didn't do anything useful.

7. 
   sipsorcery commented at 9:05 PM on June 18, 2019: member

   Any suggestions on who is qualified to review this? @ken2812221 @NicolasDorier ?

8. 
   practicalswift commented at 9:09 PM on June 18, 2019: contributor

   Concept ACK: nice cleanup!

9. sipsorcery force-pushed on Jun 18, 2019
10. 
    DrahtBot commented at 10:01 PM on June 18, 2019: member

    <!--e57a25ab6845829454e8d69fc972939a-->

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

    <!--174a7506f384e20aa4161008e828411d-->

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #15704 (Move Win32 defines to configure.ac to ensure they are globally defined by luke-jr)
    • #15529 (Add Qt programs to msvc build by sipsorcery)

    If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

11. fanquake requested review from ken2812221 on Jun 18, 2019
12. fanquake requested review from NicolasDorier on Jun 18, 2019
13. 
    fanquake commented at 1:37 AM on June 19, 2019: member

    Concept ACK for such a large reduction in Windows specific build config. Is this a result of their build system improving, or you figuring out a better way to make this work?

    Gets ready for switch to building with Visual Studio 2019 and Platform Toolset v142

    Does this drop compatibility with older versions of Visual Studio?

    I will review in so far as spinning up a Windows VM and making sure the project opens / builds inside Visual Studio.

14. in build_msvc/common.init.vcxproj:10 in 886460da32 outdated

       5 | -        <WindowsTargetPlatformVersion_10 Condition="'$(WindowsTargetPlatformVersion_10)' == ''">$(Registry:HKEY_LOCAL_MACHINE\SOFTWARE\Wow6432Node\Microsoft\Microsoft SDKs\Windows\v10.0@ProductVersion)</WindowsTargetPlatformVersion_10>
       6 | -        <!-- Sometimes the version in the registry has to .0 suffix, and sometimes it doesn't. Check and add it -->
       7 | -        <WindowsTargetPlatformVersion_10 Condition="'$(WindowsTargetPlatformVersion_10)' != '' and !$(WindowsTargetPlatformVersion_10.EndsWith('.0'))">$(WindowsTargetPlatformVersion_10).0</WindowsTargetPlatformVersion_10>
       8 | -
       9 | -        <WindowsTargetPlatformVersion>$(WindowsTargetPlatformVersion_10)</WindowsTargetPlatformVersion>
      10 | -    </PropertyGroup>
    

    ---

    ---

    

    NicolasDorier commented at 6:34 AM on June 19, 2019:

    Why you remove this? I found out that it is the best way to get bitcoin to build on several versions of visual studio.

    ---

    

    sipsorcery commented at 8:03 AM on June 19, 2019:

    Restored.

15. 
    NicolasDorier commented at 6:36 AM on June 19, 2019: contributor

    Concept ACK. Will test. Though you removed the check in registry to make bitcoin build on wider configs.

    I noticed that visual studio or msbuild via command line would get quite capricious if the exact right version of SDK is not installed without my registry hack.

16. Cleaned up and consolidated msvc build files to allow faster builds and easier migration to vs2019. 750d1bc7df
17. sipsorcery force-pushed on Jun 19, 2019
18. 
    sipsorcery commented at 8:11 AM on June 19, 2019: member

    Is this a result of their build system improving, or you figuring out a better way to make this work?

    It's a result of a lot of optimising combined with trial and error on the .vcxproj files. Typically the msbuild project files are left alone and managed by Visual Studio. There's no need to edit them manually. That doesn't make them very maintainable when every change needs to be conscientiously reviewed.

    Does this drop compatibility with older versions of Visual Studio?

    The build still runs successfully on appveyor with the Visual Studio 2017 image. I suspect the build should work correctly on VS 2012 and 2015 but I haven't tested.

19. 
    fanquake commented at 8:21 AM on June 19, 2019: member

    ACK 750d1bc7df5ec6505a8fb92dabface887c23817c

    I setup a new Windows 10 VM using this guide, used this branch + installed rapidcheck (#16241) and successfully built a working bitcoind and all tests seem to be passing.

    

    The build still runs successfully on appveyor with the Visual Studio 2017 image. I suspect the build should work correctly on VS 2012 and 2015 but I haven't tested.

    Ok. That sounds fine, but will defer to the other Windows contributors as to what is acceptable.

20. 
    NicolasDorier commented at 12:30 PM on June 19, 2019: contributor

    tACK 750d1bc . Good to see the boilerplate go away.

21. NicolasDorier approved
22. MarcoFalke renamed this:
    Cleaned up and consolidated msbuild files (no code changes)
    build: Cleaned up and consolidated msbuild files (no code changes)
    on Jun 19, 2019
23. MarcoFalke referenced this in commit 0221420d1a on Jun 19, 2019
24. MarcoFalke merged this on Jun 19, 2019
25. MarcoFalke closed this on Jun 19, 2019

    ---

26. sipsorcery deleted the branch on Jun 19, 2019
27. MarcoFalke referenced this in commit b1344eac5f on Jun 19, 2019
28. DrahtBot locked this on Dec 16, 2021

Contributors

sipsorceryMarcoFalkepracticalswiftDrahtBotfanquakeNicolasDorier

Review Requested
ken2812221