Use of test APIs

[dg0yt]

While reviewing changes to vcpkg ports, I noticed that @talregev showed a dependency of msh3 on msquic_platform: https://github.com/microsoft/vcpkg/issues/41200 https://github.com/microsoft/vcpkg/pull/42547#discussion_r1880067111

I would assume that this is unexpected, with CMake target msquic carrying the complete public interface of msquic, and with msquic_platform being a private implementation detail.

Following the symols:

msh3.cpp.obj : error LNK2001: unresolved external symbol CxPlatGetSelfSignedCert
msh3.cpp.obj : error LNK2001: unresolved external symbol CxPlatFreeSelfSignedCert

I find that

• the call to CxPlatGetSelfSignedCert is behind a runtime condition on MSH3_CREDENTIAL_TYPE_SELF_SIGNED_CERTIFICATE: https://github.com/nibanks/msh3/blob/ecdacc9929abe6f9b91c31d409a9dcb1a0fe56e1/lib/msh3.cpp#L401-L402
• MSH3_CREDENTIAL_TYPE_SELF_SIGNED_CERTIFICATE is behind a build-time condition on MSH3_TEST_MODE: https://github.com/nibanks/msh3/blob/ecdacc9929abe6f9b91c31d409a9dcb1a0fe56e1/msh3.h#L85-L87
• the declaration of CxPlatGetSelfSignedCert is behind a build-time condition on QUIC_TEST_APIS: https://github.com/microsoft/msquic/blob/21e5b41e4e9f88d809de4c78e461b57641bfca8e/src/inc/quic_platform.h#L560-L604

Are the build-time test API macros meant to be defined at all for production builds (read: for the vcpkg ports?) If no, I would assume that msh3's calls to those functions must be moved behind build-time guards. And then, msh3 would no longer need to explicitly link to msquic_platform.

[nibanks]

I am actually leaning towards including the definitions and dependencies, because it makes it much easier to right tools on top of this (since tools don't really care about a real certificate).

[dg0yt]

I don't mind about the actual choice. But please put public API in the main lib.

[nibanks]

To get this functionality, it must depend on msquic_platform.

[dg0yt]

With updated vcpkg ports for msquic and msh3, my test port build now arrives at:

/usr/bin/ld: /vcpkg/installed/x64-linux/debug/lib/libmsh3.so: undefined reference to `CxPlatLogAssert'
/usr/bin/ld: /vcpkg/installed/x64-linux/debug/lib/libmsh3.so: undefined reference to `CxPlatFreeSelfSignedCert'
/usr/bin/ld: /vcpkg/installed/x64-linux/debug/lib/libmsh3.so: undefined reference to `quic_bugcheck'
/usr/bin/ld: /vcpkg/installed/x64-linux/debug/lib/libmsh3.so: undefined reference to `CxPlatGetSelfSignedCert'

despite libmsh3.so pulling in /vcpkg/installed/x64-linux/lib/libmsquic.so.2. I would prefer to not have msquic_platform.a as another transitive usage requirement of msh3

[dg0yt]

msquic_platform only exists as a static lib. AFAIU with a shared build of msquic, msh3 starts to depend on two libraries containing msquic_platform code: the msquic shared lib and the msquic_platform static lib. This simply seems wrong to me.

[talregev]

msquic_platform only exists as a static lib. AFAIU with a shared build of msquic, msh3 starts to depend on two libraries containing msquic_platform code: the msquic shared lib and the msquic_platform static lib. This simply seems wrong to me.

I rename the platform target to msquic_platform Also I exported the msquic_platform for missing symbols for msh3. As I see you deal differently in vcpkg with missing symbols of msh3. Maybe we can revert my changes in msquic and do it differently to support msh3?

[dg0yt]

The issue I see is at the binary level (similar to C++ ODR), not at the CMake level.

I rename the platform target to msquic_platform

CMake. This is unrelated to the problem.

Also I exported the msquic_platform for missing symbols for msh3.

CMake. This is unrelated to the problem.

BTW the msquic_ prefix is inferior to proper msquic:: namespacing.

[talregev]

BTW the msquic_ prefix is inferior to proper msquic:: namespacing.

I add msquic_ prefix because when I export the platform, it also create a lib and I didn't want to create a lib with generic name like platform.

I think we can rename it back to platform when the target msquic_platform / platform will be private again.

[dg0yt]

As I see you deal differently in vcpkg with missing symbols of msh3.

FTR this is that patch. Not adding source code, just pulling symbols into the msquic lib. https://github.com/microsoft/vcpkg/blob/4b3b6201414638abfcd8ef4629dfbd958985667e/ports/msquic/exports-for-msh3.diff

[talregev]

As I see you deal differently in vcpkg with missing symbols of msh3.

FTR this is that patch. Not adding source code, just pulling symbols into the msquic lib. https://github.com/microsoft/vcpkg/blob/4b3b6201414638abfcd8ef4629dfbd958985667e/ports/msquic/exports-for-msh3.diff

I tried to apply it on upstream, but it create other problems that I couldn't solve 🙏🏻

[talregev]

The issue I see is at the binary level (similar to C++ ODR), not at the CMake level.

I hope @nibanks can help us with this problem.

[dg0yt]

I think we can rename it back to platform when the target msquic_platform / platform will be private again.

This code churn affects other people's PRs and undermines trust in the project's maturity.

I hope @nibanks can help us with this problem.

That's the point of this issue.

[dg0yt]

The CMake part of the story is this:

For using the exported/imported CMake target representing the msquic_platform static lib, CMake must forward the target's link libraries including private link targets of INTERFACE type (wrapped in $<LINK_ONLY:...>. That's the reason why msquic's CMake build must export more than just msquic and msquic_platform. For msh3, it enough to explicitly link just those two libs, but CMake still has to implicitly pull in everything else (inc, warnings, main_binary_link_args plus transitive link targets).

[nibanks]

msquic_platform only exists as a static lib. AFAIU with a shared build of msquic, msh3 starts to depend on two libraries containing msquic_platform code: the msquic shared lib and the msquic_platform static lib. This simply seems wrong to me.

This is by design. The platform abstraction layer is never meant to be a shared library. It's generally meant to be a lot of mostly inline helpers. Eventually the platform layer will be moved completely to https://github.com/microsoft/cxplat, which will always be a static lib. But that's a longer term effort.

[dg0yt]

The problem which I see now in vcpkg is that the msquic shared lib encapsulates the quicified openssl3, but the msquic_platform static lib comes with openssl3 as a transitive linking requirement, at least in exported CMake config:

set_target_properties(msquic_platform PROPERTIES
  INTERFACE_LINK_LIBRARIES "inc;\$<LINK_ONLY:warnings>;\$<LINK_ONLY:main_binary_link_args>;OpenSSL"
)

where OpenSSL evaluates to linking libssl.a, libcrypto.a (x64-linux). Installing these libs in vcpkg is not an option, with pristine openssl already available.