Skip to content

WIP: Switch appveyor build from Release to Debug#20392

Closed
sipsorcery wants to merge 1 commit intobitcoin:masterfrom
sipsorcery:msvc-appveyor-debug-build
Closed

WIP: Switch appveyor build from Release to Debug#20392
sipsorcery wants to merge 1 commit intobitcoin:masterfrom
sipsorcery:msvc-appveyor-debug-build

Conversation

@sipsorcery
Copy link
Contributor

@sipsorcery sipsorcery commented Nov 15, 2020

As per brief discussion. Visual Studio 2019 and Appveyor images have recently been updated. While it's only a minor update that doesn't break ABI compatibility it does seem to have broken compatibility for Release builds. This apparently occurs if compiler optimisations are turned on.

This PR switches the Appveyor CI job from a Release build to a Debug build in order to fix the CI failures.

Subsequent to this PR I will commence a new build of the Qt static libraries with the latest Visual Studio 2019 toolset so that release build can work again.

@sipsorcery
Copy link
Contributor Author

sipsorcery commented Nov 15, 2020

@hebasto fyi.

@DrahtBot DrahtBot added the Tests label Nov 15, 2020
@hebasto
Copy link
Member

hebasto commented Nov 15, 2020

https://ci.appveyor.com/project/DrahtBot/bitcoin/builds/36314660#L506:

C:\projects\bitcoin\src\test\net_tests.cpp(146): Entering test case "caddrdb_read_corrupted"
unknown location(0): fatal error: in "net_tests/caddrdb_read_corrupted": stack overflow

@hebasto hebasto mentioned this pull request Nov 15, 2020
Merged
maflcko pushed a commit that referenced this pull request Nov 15, 2020
Merge #20395: ci: Use the previous build worker image in AppVeyor
fb7726e 
406097c ci: Use the previous build worker image in AppVeyor (Hennadii Stepanov)

Pull request description:

  This is a workaround as the [recent](https://www.appveyor.com/updates/2020/11/14/) Visual Studio 2019 image update breaks our builds.

  This PR is alternative to #20392 due to its build [failure](https://ci.appveyor.com/project/DrahtBot/bitcoin/builds/36314660).

ACKs for top commit:
  MarcoFalke:
    review ACK 406097c also seems to pass

Tree-SHA512: f9b722d8e67dd7f0745de6da385064630adb27ecbc0a919be47f62217a2bb7a27a6fa00a7536a24bf17500a77160ca3b92b3c8619047171a6f5198b434015221
@sipsorcery
Copy link
Contributor Author

sipsorcery commented Nov 15, 2020

From past experience the msvc compiler can be quite aggressive regarding access to uninitialised/unallocated memory on debug builds. The unit test failure on caddrdb_read_corrupted is likely due to a debug-only assert.

The choices that occur to me are:

  • Continue building as a Release build and update the pre-compiled Qt libraries for VS2019 v16.8.1.

    • Pros:
      • Unit tests will pass
    • Cons:
      • The same ABI incompatibility is likely to occur again for a future VS minor release. In the time Bitcoin Core has been using Appveyor (approx. 2 years I think) this is the first case.
      • New Qt 5.9.8 build required.

  • Switch to a Debug build.

    • Pros;
      • ABI compatibility for VS 2019 minor releases should be preserved since debug builds do not use compiler optimisation flags which are only guaranteed to be compatible for libraries built with identical Visual Studio versions.
    • Cons:
      • Need to fix or disable unit tests causing debug asserts.

@sipsorcery
Switch appveyor build from Release to Debug to avoid ABI incompatibil…
92bf963 
…ites between vcpkg dependencies and the pre-built Qt binaries.
@sipsorcery sipsorcery force-pushed the msvc-appveyor-debug-build branch from 34cfaae to 92bf963 Compare November 15, 2020 11:59
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Nov 15, 2020
Merge bitcoin#20395: ci: Use the previous build worker image in AppVeyor
b03c13d 
406097c ci: Use the previous build worker image in AppVeyor (Hennadii Stepanov)

Pull request description:

  This is a workaround as the [recent](https://www.appveyor.com/updates/2020/11/14/) Visual Studio 2019 image update breaks our builds.

  This PR is alternative to bitcoin#20392 due to its build [failure](https://ci.appveyor.com/project/DrahtBot/bitcoin/builds/36314660).

ACKs for top commit:
  MarcoFalke:
    review ACK 406097c also seems to pass

Tree-SHA512: f9b722d8e67dd7f0745de6da385064630adb27ecbc0a919be47f62217a2bb7a27a6fa00a7536a24bf17500a77160ca3b92b3c8619047171a6f5198b434015221
@decryp2kanon
Copy link
Contributor

decryp2kanon commented Nov 16, 2020 •
edited
Loading

But why build failed?

@decryp2kanon
Copy link
Contributor

decryp2kanon commented Nov 16, 2020 •
edited
Loading

image

C:\projects\bitcoin\src\test\net_tests.cpp(86): Leaving test suite "net_tests"; testing time: 140513us
Test is aborted
Leaving test module "Bitcoin Core Test Suite"; testing time: 1089996937us
*** 1 failure is detected in the test module "Bitcoin Core Test Suite"
Detected memory leaks!

@sipsorcery sipsorcery changed the title Switch appveyor build from Release to Debug WIP: Switch appveyor build from Release to Debug Nov 16, 2020
@sipsorcery
Copy link
Contributor Author

sipsorcery commented Nov 23, 2020

Closing this PR as I've managed to get the msvc release build working again. Will open a new PR for that.

@sipsorcery sipsorcery closed this Nov 23, 2020
sipsorcery added a commit to sipsorcery/bitcoin that referenced this pull request Nov 26, 2020
@sipsorcery
Adjusted msvc compiler and linker settings to remove optimisations th…
6164bb2 
…at are causing sporadic ABI issues on Visual Studio updates.

Changed appveyor config file hash to use a new version of Qt pre-compiled binaries built for Visual Studio 2019 v16.8.1.

See bitcoin#20392 for related discussion.
@sipsorcery sipsorcery mentioned this pull request Nov 26, 2020
Merged
sipsorcery added a commit to sipsorcery/bitcoin that referenced this pull request Dec 2, 2020
@sipsorcery
Adjusted msvc compiler and linker settings to remove optimisations th…
8b99e60 
…at are causing sporadic ABI issues on Visual Studio updates.

Tidied up debug and release configuration blocks in common project file to avoid duplication.

Updated appveyor config to use latest Visual Studio 2019 image.

Changed appveyor config file hash to use a new version of Qt pre-compiled binaries built for Visual Studio 2019 v16.8.1.

Bumped vcpkg version to tag '2020.11-1' for binary caching feature.

See bitcoin#20392 for related discussion.
laanwj added a commit that referenced this pull request Dec 3, 2020
@laanwj
Merge #20506: ci: AppVeyor fixes for Visual Studio 2019 16.8.1 image
3fa6a9f 
2c69381 Removed redundant git pull from appveyor config. (Aaron Clauson)
8b99e60 Adjusted msvc compiler and linker settings to remove optimisations that are causing sporadic ABI issues on Visual Studio updates. (Aaron Clauson)

Pull request description:

  The motivation for this PR is twofold:

    1. Update the Qt binaries used by the appveyor CI job after a recent update to Visual Studio 2019 used in the Appveyor build image resulted in ABI incompatibilities,

    2. Remove optimisations and debug information from the Bitcoin Core `Release` msvc build to reduce the chance of future ABI incompatibility issues for future Visual Studio updates.

  The changes made in this PR are:

   - Changed appveyor config file hash to use a new version of Qt pre-compiled binaries built for Visual Studio 2019 v16.8.1.
   - Adjusted msvc compiler and linker settings to remove optimisations and debug information generation to help avoid future ABI issues on Visual Studio updates.
   - Tidied up debug and release configuration blocks in common project file to avoid duplication.
   - Updated appveyor config to use latest Visual Studio 2019 image.
   - Bumped vcpkg version to tag `2020.11-1` for binary caching feature*.

  See #20392 for related discussion.

  *Binary caching is a new [vcpkg feature](https://github.com/microsoft/vcpkg/blob/master/docs/specifications/binarycaching.md) that allows dependency caching. This PR is not using the feature but by updating the vcpkg version it means it can be optionally used by other contributors in their own Appveyor configs. By caching the vcpkg dependencies using this feature my build times are reduced by approx 10 minutes.

ACKs for top commit:
  MarcoFalke:
    Concept ACK 2c69381
  laanwj:
    Concept ACK 2c69381

Tree-SHA512: 372f5b8b3b5f7e56b78045f9fc06a22fd9f5366d06e99e2f1eaad6d741680da74d0e6371e7bc580cb41f4d6696b2101b950167309d33e98242578b458ace813a
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Dec 3, 2020
@laanwj
Merge bitcoin#20506: ci: AppVeyor fixes for Visual Studio 2019 16.8.1…
e754836 
… image

2c69381 Removed redundant git pull from appveyor config. (Aaron Clauson)
8b99e60 Adjusted msvc compiler and linker settings to remove optimisations that are causing sporadic ABI issues on Visual Studio updates. (Aaron Clauson)

Pull request description:

  The motivation for this PR is twofold:

    1. Update the Qt binaries used by the appveyor CI job after a recent update to Visual Studio 2019 used in the Appveyor build image resulted in ABI incompatibilities,

    2. Remove optimisations and debug information from the Bitcoin Core `Release` msvc build to reduce the chance of future ABI incompatibility issues for future Visual Studio updates.

  The changes made in this PR are:

   - Changed appveyor config file hash to use a new version of Qt pre-compiled binaries built for Visual Studio 2019 v16.8.1.
   - Adjusted msvc compiler and linker settings to remove optimisations and debug information generation to help avoid future ABI issues on Visual Studio updates.
   - Tidied up debug and release configuration blocks in common project file to avoid duplication.
   - Updated appveyor config to use latest Visual Studio 2019 image.
   - Bumped vcpkg version to tag `2020.11-1` for binary caching feature*.

  See bitcoin#20392 for related discussion.

  *Binary caching is a new [vcpkg feature](https://github.com/microsoft/vcpkg/blob/master/docs/specifications/binarycaching.md) that allows dependency caching. This PR is not using the feature but by updating the vcpkg version it means it can be optionally used by other contributors in their own Appveyor configs. By caching the vcpkg dependencies using this feature my build times are reduced by approx 10 minutes.

ACKs for top commit:
  MarcoFalke:
    Concept ACK 2c69381
  laanwj:
    Concept ACK 2c69381

Tree-SHA512: 372f5b8b3b5f7e56b78045f9fc06a22fd9f5366d06e99e2f1eaad6d741680da74d0e6371e7bc580cb41f4d6696b2101b950167309d33e98242578b458ace813a
maflcko pushed a commit to maflcko/bitcoin-core that referenced this pull request Dec 16, 2020
@sipsorcery
Adjusted msvc compiler and linker settings to remove optimisations th…
249d61a 
…at are causing sporadic ABI issues on Visual Studio updates.

Tidied up debug and release configuration blocks in common project file to avoid duplication.

Updated appveyor config to use latest Visual Studio 2019 image.

Changed appveyor config file hash to use a new version of Qt pre-compiled binaries built for Visual Studio 2019 v16.8.1.

Bumped vcpkg version to tag '2020.11-1' for binary caching feature.

See bitcoin#20392 for related discussion.

Github-Pull: bitcoin#20506
Rebased-From: 8b99e60
hebasto pushed a commit to hebasto/bitcoin that referenced this pull request Dec 16, 2020
@sipsorcery @hebasto
Adjusted msvc compiler and linker settings to remove optimisations th…
55980f8 
…at are causing sporadic ABI issues on Visual Studio updates.

Tidied up debug and release configuration blocks in common project file to avoid duplication.

Updated appveyor config to use latest Visual Studio 2019 image.

Changed appveyor config file hash to use a new version of Qt pre-compiled binaries built for Visual Studio 2019 v16.8.1.

Bumped vcpkg version to tag '2020.11-1' for binary caching feature.

See bitcoin#20392 for related discussion.

Github-Pull: bitcoin#20506
Rebased-From: 8b99e60
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Feb 15, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

No reviews

Assignees

No one assigned

Labels

Tests

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@sipsorcery @hebasto @decryp2kanon @DrahtBot