Skip to content

cmake, msvc: Allow to build for default x64-windows triplet#104

Merged
hebasto merged 5 commits into
cmake-stagingfrom
240225-cmake-BT
Feb 28, 2024
Merged

cmake, msvc: Allow to build for default x64-windows triplet#104
hebasto merged 5 commits into
cmake-stagingfrom
240225-cmake-BT

Conversation

@hebasto
Copy link
Copy Markdown
Owner

@hebasto hebasto commented Feb 25, 2024

The x64-windows triplet is the default one when building on Windows. All dependencies are linked dynamically, which is similar to how they are linked on POSIX systems (Linux, macOS) when building without depends.

CI builds both x64-windows and x64-windows-static triplets. Additionally dumpbin /imports output is provided for the bitcoind.exe binaries to observe the triplet effect.

@hebasto
Copy link
Copy Markdown
Owner Author

hebasto commented Feb 25, 2024

Friendly ping @sipsorcery :)

@hebasto hebasto marked this pull request as draft February 25, 2024 15:14
@hebasto
Copy link
Copy Markdown
Owner Author

hebasto commented Feb 25, 2024

Not sure about the exact reason of test_bitcoin.exe link failure...

@sipsorcery
Copy link
Copy Markdown

@hebasto what's the reason to use the vcpkg x64-windows triplet?

Last time I checked all the Bitcoin Core exe's were statically linked to the CRT and therefore won't work with the /MD flag (dynamically link against the runtime library) that the x64-windows will produce.

@hebasto
Copy link
Copy Markdown
Owner Author

hebasto commented Feb 25, 2024

what's the reason to use the vcpkg x64-windows triplet?

The main reason is to minimize vcpkg dependencies as some packages for x64-windows-static triplet depend on the same package for x64-windows triplet.

Last time I checked all the Bitcoin Core exe's were statically linked to the CRT and therefore won't work with the /MD flag (dynamically link against the runtime library) that the x64-windows will produce.

Binaries with /MD flag compile, link and run fine, except for test_bitcoin.exe (for example, in this branch).

@sipsorcery
Copy link
Copy Markdown

Binaries with /MD flag compile, link and run fine, except for test_bitcoin.exe (for example, in this branch).

Is that only done for CI purposes? Or is the idea to use dynamic runtime linking for Bitcoin Core exes built with msvc and cmake?

If it's just for CI it seems a bit unusual to do the build being tested differently to the main build.

@hebasto
Copy link
Copy Markdown
Owner Author

hebasto commented Feb 25, 2024

Binaries with /MD flag compile, link and run fine, except for test_bitcoin.exe (for example, in this branch).

Is that only done for CI purposes? Or is the idea to use dynamic runtime linking for Bitcoin Core exes built with msvc and cmake?

The idea is to suggest a choice to the user :)

@sipsorcery
Copy link
Copy Markdown

The idea is to suggest a choice to the user :)

OK, makes sense.

Have you been able to do a dynamically linked build of the Gui with Qt? That was always the trickiest one. It would actually be very handy to be able to use a non-static Qt build since they can be downloaded from Qt whereas static ones have to be custom built.

I'd assumed that whole Qt static plugin situation meant I had to be a statically linked build but maybe not?

@hebasto
Copy link
Copy Markdown
Owner Author

hebasto commented Feb 25, 2024

Have you been able to do a dynamically linked build of the Gui with Qt?

Yes. Here is an excerpt from the debug.log:

2024-02-25T13:35:30Z Qt 5.15.10 (dynamic), plugin=windows (dynamic)
2024-02-25T13:35:30Z No static plugins.
2024-02-25T13:35:30Z Style: windowsvista / QWindowsVistaStyle
2024-02-25T13:35:30Z System: Windows 11 Version 2009, x86_64-little_endian-llp64
2024-02-25T13:35:30Z Screen: \\.\DISPLAY1 1600x900, pixel ratio=1.0

@hebasto
Copy link
Copy Markdown
Owner Author

hebasto commented Feb 25, 2024

Not sure about the exact reason of test_bitcoin.exe link failure...

I've submitted a bug report upstream.

Use `/MD` flag when building for the `x64-windows` triplet.
Allow to build for default `x64-windows` triplet.

This change effectively renames the current preset "vs2022" to
"vs2022-static", which affects the "Win64 native" CI job.
@hebasto hebasto marked this pull request as ready for review February 26, 2024 18:08
@hebasto
Copy link
Copy Markdown
Owner Author

hebasto commented Feb 26, 2024

Rebased and undrafted.

CI builds both x64-windows and x64-windows-static triplets now. Additionally dumpbin /imports output is provided for the bitcoind.exe binaries to observe the triplet effect.

cc @sipsorcery

Comment thread src/randomenv.cpp
#include <sys/auxv.h>
#endif

#ifndef _MSC_VER
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

?

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The extern char** environ; declaration is not necessary when compiling with MSVC, and it causes warning C4273 "inconsistent dll linkage".

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe fix / workaround the warning, rather than adding more #ifdefs to our RNG code?

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe fix ... the warning

According to the docs:

To fix it, delete the redeclaration

@sipsorcery
Copy link
Copy Markdown

Where do I specify the triplet when doing the cmake build? Any chance you could update your previous instructions?

FWIW I've kicked off the build and it's defaulting to x64-windows. If it works it will be the first time I've ever built a dynamically linked Bitcoin Core exe.

@hebasto
Copy link
Copy Markdown
Owner Author

hebasto commented Feb 26, 2024

Where do I specify the triplet when doing the cmake build?

Specifying a preset is enough. Use --preset vs2022 or --preset vs2022-static.

Any chance you could update your previous instructions?

Thanks for a reminder! Updated.

@sipsorcery
Copy link
Copy Markdown

tACK #41322c1732ae3d02fee0489342e9fb031092dd2b (correct commit, I started the build prior to the follow on commits).

First time I've ever done a non-static msvc build. All the exe's seem to work although load noticeably slower than the static build, which is not surprising.

@hebasto hebasto merged commit 1a976ca into cmake-staging Feb 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants