Build statically some ports on vcpkg

[winterheart]

Pull Request Type

• GitHub Workflow changes
• Documentation or Wiki changes
• Build and Dependency changes
• Runtime changes
  • Render changes
  • Audio changes
  • Input changes
  • Network changes
  • Other changes

Description

Introduce own triplet with selective statically building.

Related Issues

Screenshots (if applicable)

Checklist

• I have tested my changes locally and verified that they work as intended.
• I have documented any new or modified functionality.
• I have reviewed the changes to ensure they do not introduce any unnecessary complexity or duplicate code.
• I understand that by submitting this pull request, I am agreeing to license my contributions under the project's license.

Additional Comments

[Lgt2x]

What's the rationale behind this change?

[Arcnor]

Also, why not just statically link everything, if that's what we prefer? (It will definitely simplify packaging, having only a single EXE for Windows for example, but if we're only doing some I'm not sure what we gain with it)

[JeodC]

Also, why not just statically link everything, if that's what we prefer? (It will definitely simplify packaging, having only a single EXE for Windows for example, but if we're only doing some I'm not sure what we gain with it)

We can't do it for all that I know of. Dmfc is expected to be shared since the game executable and the .d3m netgames both access it. You could try making it a static public, but might not get the results you want.

[Arcnor]

Also, why not just statically link everything, if that's what we prefer? (It will definitely simplify packaging, having only a single EXE for Windows for example, but if we're only doing some I'm not sure what we gain with it)

We can't do it for all that I know of. Dmfc is expected to be shared since the game executable and the .d3m netgames both access it. You could try making it a static public, but might not get the results you want.

This only affects vcpkg dependencies

[JeodC]

Also, why not just statically link everything, if that's what we prefer? (It will definitely simplify packaging, having only a single EXE for Windows for example, but if we're only doing some I'm not sure what we gain with it)

We can't do it for all that I know of. Dmfc is expected to be shared since the game executable and the .d3m netgames both access it. You could try making it a static public, but might not get the results you want.

This only affects vcpkg dependencies

You did say everything :)

[GravisZro]

Also, why not just statically link everything, if that's what we prefer? (It will definitely simplify packaging, having only a single EXE for Windows for example, but if we're only doing some I'm not sure what we gain with it)

You can do that for the Linux build but not the Windows build using MSVC because statically linking MSVC runtime components violates the license. See also: https://opensource.stackexchange.com/questions/7182/redistributing-msvc-runtime-components-with-gpl-application

[Arcnor]

Also, why not just statically link everything, if that's what we prefer? (It will definitely simplify packaging, having only a single EXE for Windows for example, but if we're only doing some I'm not sure what we gain with it)

You can do that for the Linux build but not the Windows build using MSVC because statically linking MSVC runtime components violates the license. See also: https://opensource.stackexchange.com/questions/7182/redistributing-msvc-runtime-components-with-gpl-application

Again, and I'd love for people to read before commenting (specially because I already said so on a few comments before yours), this PR ONLY affects vcpkg dependencies and my comment was ONLY referring to statically link said dependencies.

[GravisZro]

Well, it's the same reason: license incompatibility. spdlog is MIT and gtest is BSD-3 which are both incompatible with the GPL.

Note that zlib license is a permissible license which is compatible with the GPL.

[Arcnor]

Well, it's the same reason: license incompatibility. spdlog is MIT and gtest is BSD-3 which are both incompatible with the GPL.

Note that zlib license is a permissible license which is compatible with the GPL.

There is no issue mixing MIT and GPL and gtest is a testing dependency. I don't think there is any value in continuing this discussion.

[winterheart]

Also, why not just statically link everything, if that's what we prefer? (It will definitely simplify packaging, having only a single EXE for Windows for example, but if we're only doing some I'm not sure what we gain with it)

I've tried also link spdlog statically, but it requires to change whole project to link MSVC's threading library statically and still can't achieve green status on CI. So first try is just link statically ZLIB into binary.

[Lgt2x]

We now only have zlib (and SDL2) as dynamic libs outside of levels/net, but this change still makes sense. I'll test on Windows. Note that toolchain file will need updated for x64 Windows builds.

[winterheart]

Added changes for Windows x64.

[pzychotic]

We build all projects against the dynamic C runtime (compiler option /MD), we can't mix that with a static C runtime (/MT). So set(VCPKG_CRT_LINKAGE dynamic) should be used for static dependencies, too.

[winterheart]

This is not building project statically itself, it's about statically building external dependency (zlib), so we can compile-in statically built zlib into executable. Application itself compiles with /MD option.

[pzychotic]

I know, but that's not the point. All dependencies need to use the same CRT, ergo the same compiler option for either /MD or /MT. It doesn't matter if it's our own code or some external dependency. You can still build zlib as a static lib, but it needs to use the dynamic CRT as everything else in our project.

[winterheart]

I don't follow, why I'm not allow to do what I want to with own code. This PR compiles fine and no any issues regarding static and dynamic linking are present. Resulting binary has statically linked zlib and runs without external DLL. Can you explain please, what is issue here?

[JeodC]

I don't follow, why I'm not allow to do what I want to with own code. This PR compiles fine and no any issues regarding static and dynamic linking are present. Resulting binary has statically linked zlib and runs without external DLL. Can you explain please, what is issue here?

CRT linkage is different from library linkage. You can static the library but still have a dynamic CRT. Static CRT links use their own heap stack, so it risks issues with memory allocation and deallocation when the static CRT link interacts with the dynamic and vice versa. It's nothing to do with library linking. Disclaimer, I pulled this out of a quick research late last night so psychotic please correct me if I'm wrong. :)

[SiriusTR]

This blog post gives some info. https://www.caichinger.com/blog/2016/01/01/mixing_crt_versions/
It's safe for a DLL to use a different CRT than the program that links to it (within reasonable boundaries: sharing STL objects is not safe - you have to use a stable ABI), but not for a statically-linked library.
Edit: Although the blog post has more to do with different versions, not static vs dynamic, if you dynamically link, there's less of a guarantee the versions match. At the bare minimum you'll have CRT object code loaded into your application twice.

[Lgt2x]

@pzychotic @winterheart How should we settle this? I'm no Windows compilation expert, but compiling zlib statically to avoid having to copy over a DLL would improve developer and user experience

[pzychotic]

We can link zlib statically there is no problem with that. We should only change the CRT linkage to dynamic for that also.

I'm still on vacation so I can't test this, but the change needed would be to remove the line set(VCPKG_CRT_LINKAGE static) from the if block and move the set(VCPKG_CRT_LINKAGE dynamic) outside of the else block.

[Lgt2x]

@winterheart could you try this out ? :)

[pzychotic]

Thanks guys for the good explanation, I'm off to vacation and couldn't respond in time Daniel Keymer ***@***.***> schrieb am Do., 23. Mai 2024, 17:08:

…

***@***.**** commented on this pull request. ------------------------------ In cmake/x64-windows-d3.cmake <#319 (comment)> : > @@ -0,0 +1,12 @@ +# Custom triplet for vcpkg that builds some packages statically + +set(VCPKG_TARGET_ARCHITECTURE x64) + +# Link statically selected ports +if(${PORT} MATCHES "zlib") + set(VCPKG_CRT_LINKAGE static) This blog post gives some info. https://www.caichinger.com/blog/2016/01/01/mixing_crt_versions/ It's safe (within reasonable boundaries) for a DLL to use a different CRT than the program that links to it, but not for a statically-linked library. — Reply to this email directly, view it on GitHub <#319 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AANE74ACEUGQKA46SYL54VLZDYO75AVCNFSM6AAAAABHNWD5WKVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDANZUGU3TIMRYGA> . You are receiving this because you commented.Message ID: ***@***.***>