WIP: Shared builds on windows

[h-vetinari]

Try building shared lib for windows

Continued from #107, also adds myself as maintainer (if desired; otherwise happy to remove) Superseded by #108

[conda-forge-linter]

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

[coryan]

All the changes LGTM, once the builds pass.

[h-vetinari]

@coryan @hmaarrfk
Would appreciate your POV here - this package has a find_package(ZLIB REQUIRED) in cmake config**, and not having it at runtime makes it impossible to load this package in CMake (which also surfaces in the tests here, though only on windows, because the other test envs have a zlib somewhere). I think the best least bad approach is to actually add zlib as a run-dep here. WDYT?

**

-- Found OpenSSL: D:/bld/google-cloud-cpp-split_1661838274590/_test_env/Library/lib/libcrypto.lib (found version "3.0.5")  
CMake Error at D:/bld/google-cloud-cpp-split_1661838274590/_test_env/Library/share/cmake-3.24/Modules/FindPackageHandleStandardArgs.cmake:230 (message):
  Could NOT find ZLIB (missing: ZLIB_LIBRARY ZLIB_INCLUDE_DIR)
Call Stack (most recent call first):
-- Configuring incomplete, errors occurred!
  D:/bld/google-cloud-cpp-split_1661838274590/_test_env/Library/share/cmake-3.24/Modules/FindPackageHandleStandardArgs.cmake:594 (_FPHSA_FAILURE_MESSAGE)
See also "D:/bld/google-cloud-cpp-split_1661838274590/test_tmp/.build/quickstart/CMakeFiles/CMakeOutput.log".
  D:/bld/google-cloud-cpp-split_1661838274590/_test_env/Library/share/cmake-3.24/Modules/FindZLIB.cmake:150 (FIND_PACKAGE_HANDLE_STANDARD_ARGS)
See also "D:/bld/google-cloud-cpp-split_1661838274590/test_tmp/.build/quickstart/CMakeFiles/CMakeError.log".
  D:/bld/google-cloud-cpp-split_1661838274590/_test_env/Library/share/cmake-3.24/Modules/CMakeFindDependencyMacro.cmake:47 (find_package)
  D:/bld/google-cloud-cpp-split_1661838274590/_test_env/Library/lib/cmake/google_cloud_cpp_storage/google_cloud_cpp_storage-config.cmake:23 (find_dependency)
  CMakeLists.txt:21 (find_package)

[coryan]

@coryan @hmaarrfk Would appreciate your POV here - this package has a find_package(ZLIB REQUIRED) in cmake config**, and not having it at runtime makes it impossible to load this package in CMake (which also surfaces in the tests here, though only on windows, because the other test envs have a zlib somewhere). I think the best least bad approach is to actually add zlib as a run-dep here. WDYT?

SGTM

[h-vetinari]

Ayayay, google explicitly sets abseil (and a bunch of others) as a public link interface for this package...

I'm now tempted to rebuild grpc & this against shared abseil, but I'm kinda reluctant to do this because it would force C++17 on any consuming feedstock. The alternative is to add it as a host-dep here and patch out the abseil parts of the link interface.

[h-vetinari]

The alternative is to add it as a host-dep here

NVM, the alternative is to just switch to the corresponding abseil static lib (though we still need to patch it out of the link interface)

[h-vetinari]

the alternative is to just switch to the corresponding abseil static lib (though we still need to patch it out of the link interface)

@coryan, just for context here: a lot of the woes here (and the changes in the abseil-feedstock) come from the ABI-sensitivity of abseil to the C++ standard used to compile it. Upstream abseil just supports the case for one uniform C++ version across the entire stack, but we cannot enforce a consistent C++ version in the entire ecosystem (some still require C++11, some need C++17 features), much less cover all through a single shared library (which obviously has to choose a C++-version and hence ABI).

The escape hatch are static builds of abseil against C++11/14, while the shared lib uses C++17 (and is to be preferred for those who can use C++17).

The problem here (and similarly for grpc-cpp) is that by having target_link_libraries(<target> PUBLI absl::something), this forces CMake to look for abseil as a host-dependence, but for the static library that should explicitly not be necessary anymore.

Right now I'm just trying how I can get this to work (so don't take the changes here, much less the patches) as final.

[coryan]

the alternative is to just switch to the corresponding abseil static lib (though we still need to patch it out of the link interface)

@coryan, just for context here: a lot of the woes here (and the changes in the abseil-feedstock) come from the ABI-sensitivity of abseil to the C++ standard used to compile it.

I am well aware.

Upstream abseil just supports the case for one uniform C++ version across the entire stack, but we cannot enforce a consistent C++ version in the entire ecosystem (some still require C++11, some need C++17 features), much less cover all through a single shared library (which obviously has to choose a C++-version and hence ABI).

The escape hatch are static builds of abseil against C++11/14, while the shared lib uses C++17 (and is to be preferred for those who can use C++17).

A different escape hatch would have been to pin Abseil to its C++11 ABI. You compile abseil with C++11 and then change absl/base/options.h as follows:

https://github.com/googleapis/google-cloud-cpp/blob/404d9a8f29f5fc97f6dd7f2b172b4b81cbf87024/ci/cloudbuild/dockerfiles/fedora-36.Dockerfile#L60

Then Abseil will always use the C++11 type, regardless of what C++ version is in effect at the time it is used.

The problem here (and similarly for grpc-cpp) is that by having target_link_libraries(<target> PUBLI absl::something), this forces CMake to look for abseil as a host-dependence, but for the static library that should explicitly not be necessary anymore.

I think that needs to be PUBLIC. google-cloud-cpp exposes Abseil types in its public APIs (unlike grpc-cpp). The application needs both abseil and google-cloud-cpp headers.

Right now I'm just trying how I can get this to work (so don't take the changes here, much less the patches) as final.

[h-vetinari]

I think that needs to be PUBLIC. google-cloud-cpp exposes Abseil types in its public APIs (unlike grpc-cpp). The application needs both abseil and google-cloud-cpp headers.

That's very relevant/interesting to know, thanks! Though given the issues with that I'm really surprised that exposing abseil types in the public API was a good trade-off.

A different escape hatch would have been to pin Abseil to its C++11 ABI. You compile abseil with C++11 and then change absl/base/options.h as follows:

https://github.com/googleapis/google-cloud-cpp/blob/404d9a8f29f5fc97f6dd7f2b172b4b81cbf87024/ci/cloudbuild/dockerfiles/fedora-36.Dockerfile#L60

Then Abseil will always use the C++11 type, regardless of what C++ version is in effect at the time it is used.

Also very good to know! I'm not really sure I like that approach TBH, but certainly a thing to keep in mind if I can't get the current approach to work.

[h-vetinari]

Then Abseil will always use the C++11 type, regardless of what C++ version is in effect at the time it is used.

Also very good to know! I'm not really sure I like that approach TBH, but certainly a thing to keep in mind if I can't get the current approach to work.

Scratch that, I do like the idea. But unless we only build for the C++11 ABI (which I don't think is great for indirection/performance for those who could also use the C++17 stdlib), we'd still get a virality problem where shared builds of libabseil and libabseil-compat (say) could not co-exist in the same environment, and if e.g. google-cloud-cpp were to choose -compat on windows, it wouldn't be co-installable with any package that decides to compile against C++17 libabseil.

The static build approach seems to be much more work than hoped for, but I liked the idea of separating the internal ABI concerns of a package from those of the ecosystem. Now that google-cloud-cpp has abseil types in its API, I'm tempted to just move it to C++17 shared builds.

[h-vetinari]

we'd still get a virality problem

... unless there'd be a way to do "fat" abseil builds that contain the symbols for both ABIs (C++11/17)?!

[coryan]

we'd still get a virality problem

... unless there'd be a way to do "fat" abseil builds that contain the symbols for both ABIs (C++11/17)?!

I fear that would create ODR violations. Any function absl::string_view Foo() cannot coexist with std::string_view Foo() in the same C++ program (a good example would be absl::string_view absl::Status::message() const).

Sometimes you can get that to work (e.g. if one version of the function is only used in a .so and the symbol is not exported). This becomes a carefully balanced house of cards: one little mistake and the symbols leak and the whole thing tumbles down.

[h-vetinari]

Sometimes you can get that to work (e.g. if one version of the function is only used in a .so and the symbol is not exported). This becomes a carefully balanced house of cards: one little mistake and the symbols leak and the whole thing tumbles down.

Yeah, you're right... 😑

What are your thoughts on the impact of using the C++11 ABI unversally on performance / binary footprint? The issue to me is that most packages can use C++17 shared builds just fine, so we'd be pessimizing everyone just for some corner cases.

For now, I think the least bad option might be to move forward with C++17 builds of google-cloud-cpp (against shared C++17 abseil), and potentially having C++14 builds if necessary (like for abseil).

[h-vetinari]

Ah well, damned if you do (shared), damned if you don't (static): abseil/abseil-cpp#1265

[coryan]

Sometimes you can get that to work (e.g. if one version of the function is only used in a .so and the symbol is not exported). This becomes a carefully balanced house of cards: one little mistake and the symbols leak and the whole thing tumbles down.

Yeah, you're right...

What are your thoughts on the impact of using the C++11 ABI universally on performance / binary footprint?
The issue to me is that most packages can use C++17 shared builds just fine, so we'd be pessimizing everyone just for some corner cases.

I suspect the cognitive load of dealing with absl::* types and std::* types is more troublesome that the CPU or memory overheads. Developers using Abseil would need to know that "Oh, in this ecosystem Abseil is compiled and pinned to C++11, but in this other ecosystem is compiled with C++17". Beyond that, the overhead is bound to be very specific to the application and how the application (and intermediate libraries) use Abseil.

For now, I think the least bad option might be to move forward with C++17 builds of google-cloud-cpp (against shared C++17 abseil), and potentially having C++14 builds if necessary (like for abseil).

Another solution is to compile Abseil with C++17 and enforce C++17 for anything downstream by patching these lines to say cxx_std_17:

https://github.com/abseil/abseil-cpp/blob/43d3c7a4e290ee96684735daf7b1d528c30a7943/CMake/AbseilHelpers.cmake#L282
https://github.com/abseil/abseil-cpp/blob/43d3c7a4e290ee96684735daf7b1d528c30a7943/CMake/AbseilHelpers.cmake#L330

I have argued that this should be the default behavior in Abseil, i.e., if it was compiled with C++17 it should require C++17 downstream because .. it does.

[h-vetinari]

I suspect the cognitive load of dealing with absl::* types and std::* types is more troublesome that the CPU or memory overheads.

Good point. I've slowly come around to that as well, see conda-forge/abseil-cpp-feedstock#45 (feel free to comment if you like).

Another solution is to compile Abseil with C++17 and enforce C++17 for anything downstream by patching these lines to say cxx_std_17:

I agree with this. I even had experimental patches to that effect lined up, but they never got published in one of our GA builds.

I have argued that this should be the default behavior in Abseil, i.e., if it was compiled with C++17 it should require C++17 downstream because .. it does.

Also agreed; that is, unless one does the thing with abseil/base/options.h you mentioned 🙃

[isuruf]

Windows errors should be fixed by abseil/abseil-cpp#1269

[h-vetinari]

Windows errors should be fixed by abseil/abseil-cpp#1269

So I backported this patch, and indeed the missing symbols are now being found 🥳

OTOH, the build is still stuck because

LINK : fatal error LNK1181: cannot open input file 'google\cloud\google_cloud_cpp_common.lib'

My first guess from (painful) experience is that this happens because no symbols are being exported (__declspec(dllexport) and all that jazz), leading to a non-existent import library to go with the DLL. The easiest workaround for that is CMAKE_WINDOWS_EXPORT_ALL_SYMBOLS, or otherwise, we'd have to patch in something like ABSL_DLL and annotate all symbols to be exported...

[h-vetinari]

So, I'm almost certain that this fails now because the static grpc on windows was built against the C++11 abseil ABI, but we're now compiling against C++17 here.

I've tried switching to shared grpc-builds in conda-forge/grpc-cpp-feedstock#237 but this fails with linking errors. I'm now trying to build grpc against C++17 shared abseil in conda-forge/grpc-cpp-feedstock#239.

[coryan]

FWIW, I just looked at the logs. These errors look like a bug in google-cloud-cpp:

LINK: command "C:\PROGRA~2\MICROS~2\2019\ENTERP~1\VC\Tools\MSVC\1429~1.301\bin\Hostx64\x64\link.exe /nologo @CMakeFiles\google_cloud_cpp_grpc_utils.rsp /out:google\cloud\google_cloud_cpp_grpc_utils.dll /implib:google\cloud\google_cloud_cpp_grpc_utils.lib /pdb:google\cloud\google_cloud_cpp_grpc_utils.pdb /dll /version:2.4 /machine:x64 /INCREMENTAL:NO /DEF:google\cloud\CMakeFiles\google_cloud_cpp_grpc_utils.dir\.\exports.def /MANIFEST /MANIFESTFILE:google\cloud\google_cloud_cpp_grpc_utils.dll.manifest" failed (exit code 1120) with the following output:
   Creating library google\cloud\google_cloud_cpp_grpc_utils.lib and object google\cloud\google_cloud_cpp_grpc_utils.exp
grpc_error_delegate.cc.obj : error LNK2019: unresolved external symbol "char const * const google::cloud::v2_4_0::internal::kStatusPayloadGrpcProto" (?kStatusPayloadGrpcProto@internal@v2_4_0@cloud@google@@3QBDB) referenced in function "class google::cloud::v2_4_0::Status __cdecl google::cloud::v2_4_0::MakeStatusFromRpcError(class grpc::Status const &)" (?MakeStatusFromRpcError@v2_4_0@cloud@google@@YA?AVStatus@123@AEBV4grpc@@@Z)
log_wrapper_helpers.cc.obj : error LNK2001: unresolved external symbol "char const * const google::cloud::v2_4_0::internal::kStatusPayloadGrpcProto" (?kStatusPayloadGrpcProto@internal@v2_4_0@cloud@google@@3QBDB)

We should change that to a function. I just created googleapis/google-cloud-cpp#10205 to track that work.

[h-vetinari]

We should change that to a function. I just created googleapis/google-cloud-cpp#10205 to track that work.

Great, thanks a lot!

[coryan]

FWIW. The fixes for googleapis/google-cloud-cpp#10205 are already available in Conda.

[conda-forge-webservices]

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

[h-vetinari]

Hm, running into missing symbols, more precisely virtual tables. I remembered this from somewhere else, where I had left myself a link to a related SO. Not sure if it applies here. I was hoping we might get further this time, but I don't have time to start playing with patches... 🙈

[1100/5763] Linking CXX shared library google\cloud\accessapproval\google_cloud_cpp_accessapproval.dll
FAILED: google/cloud/accessapproval/google_cloud_cpp_accessapproval.dll google/cloud/accessapproval/google_cloud_cpp_accessapproval.lib 
cmd.exe /C "cmd.exe /C "%BUILD_PREFIX%\Library\bin\cmake.exe -E __create_def %SRC_DIR%\build\google\cloud\accessapproval\CMakeFiles\google_cloud_cpp_accessapproval.dir\.\exports.def %SRC_DIR%\build\google\cloud\accessapproval\CMakeFiles\google_cloud_cpp_accessapproval.dir\.\exports.def.objs && cd %SRC_DIR%\build" && %BUILD_PREFIX%\Library\bin\cmake.exe -E vs_link_dll --intdir=google\cloud\accessapproval\CMakeFiles\google_cloud_cpp_accessapproval.dir --rc=C:\PROGRA~2\WI3CF2~1\10\bin\100226~1.0\x64\rc.exe --mt=C:\PROGRA~2\WI3CF2~1\10\bin\100226~1.0\x64\mt.exe --manifests  -- C:\PROGRA~1\MICROS~2\2022\ENTERP~1\VC\Tools\MSVC\1429~1.301\bin\HostX64\x64\link.exe /nologo @CMakeFiles\google_cloud_cpp_accessapproval.rsp  /out:google\cloud\accessapproval\google_cloud_cpp_accessapproval.dll /implib:google\cloud\accessapproval\google_cloud_cpp_accessapproval.lib /pdb:google\cloud\accessapproval\google_cloud_cpp_accessapproval.pdb /dll /version:2.9 /machine:x64 /INCREMENTAL:NO  /DEF:google\cloud\accessapproval\CMakeFiles\google_cloud_cpp_accessapproval.dir\.\exports.def  && cd ."
LINK: command "C:\PROGRA~1\MICROS~2\2022\ENTERP~1\VC\Tools\MSVC\1429~1.301\bin\HostX64\x64\link.exe /nologo @CMakeFiles\google_cloud_cpp_accessapproval.rsp /out:google\cloud\accessapproval\google_cloud_cpp_accessapproval.dll /implib:google\cloud\accessapproval\google_cloud_cpp_accessapproval.lib /pdb:google\cloud\accessapproval\google_cloud_cpp_accessapproval.pdb /dll /version:2.9 /machine:x64 /INCREMENTAL:NO /DEF:google\cloud\accessapproval\CMakeFiles\google_cloud_cpp_accessapproval.dir\.\exports.def /MANIFEST /MANIFESTFILE:google\cloud\accessapproval\google_cloud_cpp_accessapproval.dll.manifest" failed (exit code 1120) with the following output:
   Creating library google\cloud\accessapproval\google_cloud_cpp_accessapproval.lib and object google\cloud\accessapproval\google_cloud_cpp_accessapproval.exp
access_approval_client.cc.obj : error LNK2019: unresolved external symbol "const google::cloud::accessapproval::v1::ListApprovalRequestsMessage::`vftable'" (??_7ListApprovalRequestsMessage@v1@accessapproval@cloud@google@@6B@) referenced in function "public: __cdecl google::cloud::accessapproval::v1::ListApprovalRequestsMessage::ListApprovalRequestsMessage(class google::cloud::accessapproval::v1::ListApprovalRequestsMessage &&)" (??0ListApprovalRequestsMessage@v1@accessapproval@cloud@google@@QEAA@$$QEAV01234@@Z)
access_approval_connection_impl.cc.obj : error LNK2001: unresolved external symbol "const google::cloud::accessapproval::v1::ListApprovalRequestsMessage::`vftable'" (??_7ListApprovalRequestsMessage@v1@accessapproval@cloud@google@@6B@)
access_approval_client.cc.obj : error LNK2019: unresolved external symbol "const google::cloud::accessapproval::v1::GetApprovalRequestMessage::`vftable'" (??_7GetApprovalRequestMessage@v1@accessapproval@cloud@google@@6B@) referenced in function "public: __cdecl google::cloud::accessapproval::v1::GetApprovalRequestMessage::GetApprovalRequestMessage(void)" (??0GetApprovalRequestMessage@v1@accessapproval@cloud@google@@QEAA@XZ)
access_approval_client.cc.obj : error LNK2019: unresolved external symbol "const google::cloud::accessapproval::v1::GetAccessApprovalSettingsMessage::`vftable'" (??_7GetAccessApprovalSettingsMessage@v1@accessapproval@cloud@google@@6B@) referenced in function "public: __cdecl google::cloud::accessapproval::v1::GetAccessApprovalSettingsMessage::GetAccessApprovalSettingsMessage(void)" (??0GetAccessApprovalSettingsMessage@v1@accessapproval@cloud@google@@QEAA@XZ)
access_approval_client.cc.obj : error LNK2019: unresolved external symbol "const google::cloud::accessapproval::v1::UpdateAccessApprovalSettingsMessage::`vftable'" (??_7UpdateAccessApprovalSettingsMessage@v1@accessapproval@cloud@google@@6B@) referenced in function "public: __cdecl google::cloud::accessapproval::v1::UpdateAccessApprovalSettingsMessage::UpdateAccessApprovalSettingsMessage(void)" (??0UpdateAccessApprovalSettingsMessage@v1@accessapproval@cloud@google@@QEAA@XZ)
access_approval_client.cc.obj : error LNK2019: unresolved external symbol "const google::cloud::accessapproval::v1::DeleteAccessApprovalSettingsMessage::`vftable'" (??_7DeleteAccessApprovalSettingsMessage@v1@accessapproval@cloud@google@@6B@) referenced in function "public: __cdecl google::cloud::accessapproval::v1::DeleteAccessApprovalSettingsMessage::DeleteAccessApprovalSettingsMessage(void)" (??0DeleteAccessApprovalSettingsMessage@v1@accessapproval@cloud@google@@QEAA@XZ)
access_approval_client.cc.obj : error LNK2019: unresolved external symbol "const google::cloud::accessapproval::v1::GetAccessApprovalServiceAccountMessage::`vftable'" (??_7GetAccessApprovalServiceAccountMessage@v1@accessapproval@cloud@google@@6B@) referenced in function "public: __cdecl google::cloud::accessapproval::v1::GetAccessApprovalServiceAccountMessage::GetAccessApprovalServiceAccountMessage(void)" (??0GetAccessApprovalServiceAccountMessage@v1@accessapproval@cloud@google@@QEAA@XZ)
access_approval_connection.cc.obj : error LNK2019: unresolved external symbol "const google::cloud::accessapproval::v1::ApprovalRequest::`vftable'" (??_7ApprovalRequest@v1@accessapproval@cloud@google@@6B@) referenced in function "public: class std::optional<class google::cloud::accessapproval::v1::ApprovalRequest> & __cdecl std::optional<class google::cloud::accessapproval::v1::ApprovalRequest>::operator=<class google::cloud::accessapproval::v1::ApprovalRequest,0>(class google::cloud::accessapproval::v1::ApprovalRequest &&)" (??$?4VApprovalRequest@v1@accessapproval@cloud@google@@$0A@@?$optional@VApprovalRequest@v1@accessapproval@cloud@google@@@std@@QEAAAEAV01@$$QEAVApprovalRequest@v1@accessapproval@cloud@google@@@Z)
access_approval_connection_impl.cc.obj : error LNK2001: unresolved external symbol "const google::cloud::accessapproval::v1::ApprovalRequest::`vftable'" (??_7ApprovalRequest@v1@accessapproval@cloud@google@@6B@)
access_approval_logging_decorator.cc.obj : error LNK2001: unresolved external symbol "const google::cloud::accessapproval::v1::ApprovalRequest::`vftable'" (??_7ApprovalRequest@v1@accessapproval@cloud@google@@6B@)
access_approval_stub.cc.obj : error LNK2001: unresolved external symbol "const google::cloud::accessapproval::v1::ApprovalRequest::`vftable'" (??_7ApprovalRequest@v1@accessapproval@cloud@google@@6B@)
access_approval_connection_impl.cc.obj : error LNK2019: unresolved external symbol "const google::cloud::accessapproval::v1::AccessApprovalSettings::`vftable'" (??_7AccessApprovalSettings@v1@accessapproval@cloud@google@@6B@) referenced in function "class google::cloud::v2_9_1::StatusOr<class google::cloud::accessapproval::v1::AccessApprovalSettings> __cdecl google::cloud::v2_9_1::internal::RetryLoopImpl<class <lambda_c84bc0c00aa5381408520fb7cb4f149e>,class google::cloud::accessapproval::v1::UpdateAccessApprovalSettingsMessage,class std::function<void __cdecl(class std::chrono::duration<__int64,struct std::ratio<1,1000> >)>,0>(class std::unique_ptr<class google::cloud::v2_9_1::internal::RetryPolicy,struct std::default_delete<class google::cloud::v2_9_1::internal::RetryPolicy> >,class std::unique_ptr<class google::cloud::v2_9_1::internal::BackoffPolicy,struct std::default_delete<class google::cloud::v2_9_1::internal::BackoffPolicy> >,enum google::cloud::v2_9_1::Idempotency,class <lambda_c84bc0c00aa5381408520fb7cb4f149e> &&,class google::cloud::accessapproval::v1::UpdateAccessApprovalSettingsMessage const &,char const *,class std::function<void __cdecl(class std::chrono::duration<__int64,struct std::ratio<1,1000> >)>)" (??$RetryLoopImpl@V<lambda_c84bc0c00aa5381408520fb7cb4f149e>@@VUpdateAccessApprovalSettingsMessage@v1@accessapproval@cloud@google@@V?$function@$$A6AXV?$duration@_JU?$ratio@$00$0DOI@@std@@@chrono@std@@@Z@std@@$0A@@internal@v2_9_1@cloud@google@@YA?AV?$StatusOr@VAccessApprovalSettings@v1@accessapproval@cloud@google@@@123@V?$unique_ptr@VRetryPolicy@internal@v2_9_1@cloud@google@@U?$default_delete@VRetryPolicy@internal@v2_9_1@cloud@google@@@std@@@std@@V?$unique_ptr@VBackoffPolicy@internal@v2_9_1@cloud@google@@U?$default_delete@VBackoffPolicy@internal@v2_9_1@cloud@google@@@std@@@6@W4Idempotency@123@$$QEAV<lambda_c84bc0c00aa5381408520fb7cb4f149e>@@AEBVUpdateAccessApprovalSettingsMessage@v1@accessapproval@23@PEBDV?$function@$$A6AXV?$duration@_JU?$ratio@$00$0DOI@@std@@@chrono@std@@@Z@6@@Z)
access_approval_logging_decorator.cc.obj : error LNK2001: unresolved external symbol "const google::cloud::accessapproval::v1::AccessApprovalSettings::`vftable'" (??_7AccessApprovalSettings@v1@accessapproval@cloud@google@@6B@)
access_approval_stub.cc.obj : error LNK2001: unresolved external symbol "const google::cloud::accessapproval::v1::AccessApprovalSettings::`vftable'" (??_7AccessApprovalSettings@v1@accessapproval@cloud@google@@6B@)
access_approval_connection_impl.cc.obj : error LNK2019: unresolved external symbol "const google::cloud::accessapproval::v1::AccessApprovalServiceAccount::`vftable'" (??_7AccessApprovalServiceAccount@v1@accessapproval@cloud@google@@6B@) referenced in function "class google::cloud::v2_9_1::StatusOr<class google::cloud::accessapproval::v1::AccessApprovalServiceAccount> __cdecl google::cloud::v2_9_1::internal::RetryLoopImpl<class <lambda_5abf93e51319dc5132c13b7f35024c40>,class google::cloud::accessapproval::v1::GetAccessApprovalServiceAccountMessage,class std::function<void __cdecl(class std::chrono::duration<__int64,struct std::ratio<1,1000> >)>,0>(class std::unique_ptr<class google::cloud::v2_9_1::internal::RetryPolicy,struct std::default_delete<class google::cloud::v2_9_1::internal::RetryPolicy> >,class std::unique_ptr<class google::cloud::v2_9_1::internal::BackoffPolicy,struct std::default_delete<class google::cloud::v2_9_1::internal::BackoffPolicy> >,enum google::cloud::v2_9_1::Idempotency,class <lambda_5abf93e51319dc5132c13b7f35024c40> &&,class google::cloud::accessapproval::v1::GetAccessApprovalServiceAccountMessage const &,char const *,class std::function<void __cdecl(class std::chrono::duration<__int64,struct std::ratio<1,1000> >)>)" (??$RetryLoopImpl@V<lambda_5abf93e51319dc5132c13b7f35024c40>@@VGetAccessApprovalServiceAccountMessage@v1@accessapproval@cloud@google@@V?$function@$$A6AXV?$duration@_JU?$ratio@$00$0DOI@@std@@@chrono@std@@@Z@std@@$0A@@internal@v2_9_1@cloud@google@@YA?AV?$StatusOr@VAccessApprovalServiceAccount@v1@accessapproval@cloud@google@@@123@V?$unique_ptr@VRetryPolicy@internal@v2_9_1@cloud@google@@U?$default_delete@VRetryPolicy@internal@v2_9_1@cloud@google@@@std@@@std@@V?$unique_ptr@VBackoffPolicy@internal@v2_9_1@cloud@google@@U?$default_delete@VBackoffPolicy@internal@v2_9_1@cloud@google@@@std@@@6@W4Idempotency@123@$$QEAV<lambda_5abf93e51319dc5132c13b7f35024c40>@@AEBVGetAccessApprovalServiceAccountMessage@v1@accessapproval@23@PEBDV?$function@$$A6AXV?$duration@_JU?$ratio@$00$0DOI@@std@@@chrono@std@@@Z@6@@Z)
access_approval_logging_decorator.cc.obj : error LNK2001: unresolved external symbol "const google::cloud::accessapproval::v1::AccessApprovalServiceAccount::`vftable'" (??_7AccessApprovalServiceAccount@v1@accessapproval@cloud@google@@6B@)
access_approval_stub.cc.obj : error LNK2001: unresolved external symbol "const google::cloud::accessapproval::v1::AccessApprovalServiceAccount::`vftable'" (??_7AccessApprovalServiceAccount@v1@accessapproval@cloud@google@@6B@)
access_approval_connection_impl.cc.obj : error LNK2019: unresolved external symbol "const google::cloud::accessapproval::v1::ListApprovalRequestsResponse::`vftable'" (??_7ListApprovalRequestsResponse@v1@accessapproval@cloud@google@@6B@) referenced in function "public: __cdecl google::cloud::accessapproval::v1::ListApprovalRequestsResponse::ListApprovalRequestsResponse(class google::cloud::accessapproval::v1::ListApprovalRequestsResponse &&)" (??0ListApprovalRequestsResponse@v1@accessapproval@cloud@google@@QEAA@$$QEAV01234@@Z)
access_approval_logging_decorator.cc.obj : error LNK2001: unresolved external symbol "const google::cloud::accessapproval::v1::ListApprovalRequestsResponse::`vftable'" (??_7ListApprovalRequestsResponse@v1@accessapproval@cloud@google@@6B@)
access_approval_stub.cc.obj : error LNK2001: unresolved external symbol "const google::cloud::accessapproval::v1::ListApprovalRequestsResponse::`vftable'" (??_7ListApprovalRequestsResponse@v1@accessapproval@cloud@google@@6B@)
access_approval_metadata_decorator.cc.obj : error LNK2019: unresolved external symbol "struct google::cloud::accessapproval::v1::AccessApprovalSettingsDefaultTypeInternal google::cloud::accessapproval::v1::_AccessApprovalSettings_default_instance_" (?_AccessApprovalSettings_default_instance_@v1@accessapproval@cloud@google@@3UAccessApprovalSettingsDefaultTypeInternal@1234@A) referenced in function "public: virtual class google::cloud::v2_9_1::StatusOr<class google::cloud::accessapproval::v1::AccessApprovalSettings> __cdecl google::cloud::accessapproval_v1_internal::v2_9_1::AccessApprovalMetadata::UpdateAccessApprovalSettings(class grpc::ClientContext &,class google::cloud::accessapproval::v1::UpdateAccessApprovalSettingsMessage const &)" (?UpdateAccessApprovalSettings@AccessApprovalMetadata@v2_9_1@accessapproval_v1_internal@cloud@google@@UEAA?AV?$StatusOr@VAccessApprovalSettings@v1@accessapproval@cloud@google@@@245@AEAVClientContext@grpc@@AEBVUpdateAccessApprovalSettingsMessage@v1@accessapproval@45@@Z)
google\cloud\accessapproval\google_cloud_cpp_accessapproval.dll : fatal error LNK1120: 11 unresolved externals

[coryan]

You may be running into this:

For global data symbols, __declspec(dllimport) must still be used when compiling against the code in the .dll.

https://cmake.org/cmake/help/latest/prop_tgt/WINDOWS_EXPORT_ALL_SYMBOLS.html

The virtual table seems like a data symbol. I have tried to fix this before, but gave up as gRPC does not support DLLs for generated code. More details in this bug

[h-vetinari]

Yeah, this is likely it. Btw, I managed to get grpc to build as a shared library on windows, but the effort to make it work (precisely around that dllimport & export) is substantial, and there's still something going wrong.

[coryan]

Yeah, this is likely it. Btw, I managed to get grpc to build as a shared library on windows, but the effort to make it work (precisely around that dllimport & export) is substantial, and there's still something going wrong.

That is really cool work. I wonder if you would be willing to upstream these changes? Yes, they still need work, but I think it is great progress. I don't work in gRPC, but I know some folks that do. I will ask if they are interested in supporting Windows DLLs and ping you with their response.

Separately: You probably realized this, even with that work, google-cloud-cpp would still need some support in the code generated by gRPC.

[h-vetinari]

I wonder if you would be willing to upstream these changes? Yes, they still need work, but I think it is great progress. I don't work in gRPC, but I know some folks that do. I will ask if they are interested in supporting Windows DLLs and ping you with their response.

Would be happy to of course! Though for now I didn't feel ready to "bother" upstream grpc with what I was doing. I had opened grpc/grpc#30838 at least, which is the other part of that feedstock PR (now only CMake after upstream grpc already fixed the pkgconfig metadata). But if people are interested, I can open a PR for discussion.

Separately: You probably realized this, even with that work, google-cloud-cpp would still need some support in the code generated by gRPC.

Haha, I already battled with this when trying to build sentencepiece as a shared lib... 🙃
(patching generated output is a whole 'nother level of pain; I haven't had the heart to start raising protobuf issues for these though)

[h-vetinari]

Thought I'd give this another whirl. Not much different from last time:

[1143/6050] Linking CXX shared library google\cloud\accesscontextmanager\google_cloud_cpp_accesscontextmanager.dll
FAILED: google/cloud/accesscontextmanager/google_cloud_cpp_accesscontextmanager.dll google/cloud/accesscontextmanager/google_cloud_cpp_accesscontextmanager.lib 
cmd.exe /C "cmd.exe /C "%BUILD_PREFIX%\Library\bin\cmake.exe -E __create_def %SRC_DIR%\build\google\cloud\accesscontextmanager\CMakeFiles\google_cloud_cpp_accesscontextmanager.dir\.\exports.def %SRC_DIR%\build\google\cloud\accesscontextmanager\CMakeFiles\google_cloud_cpp_accesscontextmanager.dir\.\exports.def.objs && cd %SRC_DIR%\build" && %BUILD_PREFIX%\Library\bin\cmake.exe -E vs_link_dll --intdir=google\cloud\accesscontextmanager\CMakeFiles\google_cloud_cpp_accesscontextmanager.dir --rc=C:\PROGRA~2\WI3CF2~1\10\bin\100226~1.0\x64\rc.exe --mt=C:\PROGRA~2\WI3CF2~1\10\bin\100226~1.0\x64\mt.exe --manifests  -- C:\PROGRA~1\MICROS~2\2022\ENTERP~1\VC\Tools\MSVC\1429~1.301\bin\HostX64\x64\link.exe /nologo @CMakeFiles\google_cloud_cpp_accesscontextmanager.rsp  /out:google\cloud\accesscontextmanager\google_cloud_cpp_accesscontextmanager.dll /implib:google\cloud\accesscontextmanager\google_cloud_cpp_accesscontextmanager.lib /pdb:google\cloud\accesscontextmanager\google_cloud_cpp_accesscontextmanager.pdb /dll /version:2.11 /machine:x64 /INCREMENTAL:NO  /DEF:google\cloud\accesscontextmanager\CMakeFiles\google_cloud_cpp_accesscontextmanager.dir\.\exports.def  && cd ."
LINK: command "C:\PROGRA~1\MICROS~2\2022\ENTERP~1\VC\Tools\MSVC\1429~1.301\bin\HostX64\x64\link.exe /nologo @CMakeFiles\google_cloud_cpp_accesscontextmanager.rsp /out:google\cloud\accesscontextmanager\google_cloud_cpp_accesscontextmanager.dll /implib:google\cloud\accesscontextmanager\google_cloud_cpp_accesscontextmanager.lib /pdb:google\cloud\accesscontextmanager\google_cloud_cpp_accesscontextmanager.pdb /dll /version:2.11 /machine:x64 /INCREMENTAL:NO /DEF:google\cloud\accesscontextmanager\CMakeFiles\google_cloud_cpp_accesscontextmanager.dir\.\exports.def /MANIFEST /MANIFESTFILE:google\cloud\accesscontextmanager\google_cloud_cpp_accesscontextmanager.dll.manifest" failed (exit code 1120) with the following output:
   Creating library google\cloud\accesscontextmanager\google_cloud_cpp_accesscontextmanager.lib and object google\cloud\accesscontextmanager\google_cloud_cpp_accesscontextmanager.exp
access_context_manager_client.cc.obj : error LNK2019: unresolved external symbol "const google::identity::accesscontextmanager::v1::ListAccessPoliciesRequest::`vftable'" (??_7ListAccessPoliciesRequest@v1@accesscontextmanager@identity@google@@6B@) referenced in function "public: __cdecl google::identity::accesscontextmanager::v1::ListAccessPoliciesRequest::ListAccessPoliciesRequest(class google::identity::accesscontextmanager::v1::ListAccessPoliciesRequest &&)" (??0ListAccessPoliciesRequest@v1@accesscontextmanager@identity@google@@QEAA@$$QEAV01234@@Z)
access_context_manager_connection_impl.cc.obj : error LNK2001: unresolved external symbol "const google::identity::accesscontextmanager::v1::ListAccessPoliciesRequest::`vftable'" (??_7ListAccessPoliciesRequest@v1@accesscontextmanager@identity@google@@6B@)
access_context_manager_client.cc.obj : error LNK2019: unresolved external symbol "const google::identity::accesscontextmanager::v1::GetAccessPolicyRequest::`vftable'" (??_7GetAccessPolicyRequest@v1@accesscontextmanager@identity@google@@6B@) referenced in function "public: __cdecl google::identity::accesscontextmanager::v1::GetAccessPolicyRequest::GetAccessPolicyRequest(void)" (??0GetAccessPolicyRequest@v1@accesscontextmanager@identity@google@@QEAA@XZ)
access_context_manager_client.cc.obj : error LNK2019: unresolved external symbol "const google::identity::accesscontextmanager::v1::UpdateAccessPolicyRequest::`vftable'" (??_7UpdateAccessPolicyRequest@v1@accesscontextmanager@identity@google@@6B@) referenced in function "public: __cdecl google::identity::accesscontextmanager::v1::UpdateAccessPolicyRequest::UpdateAccessPolicyRequest(void)" (??0UpdateAccessPolicyRequest@v1@accesscontextmanager@identity@google@@QEAA@XZ)
access_context_manager_connection_impl.cc.obj : error LNK2001: unresolved external symbol "const google::identity::accesscontextmanager::v1::UpdateAccessPolicyRequest::`vftable'" (??_7UpdateAccessPolicyRequest@v1@accesscontextmanager@identity@google@@6B@)
access_context_manager_client.cc.obj : error LNK2019: unresolved external symbol "const google::identity::accesscontextmanager::v1::DeleteAccessPolicyRequest::`vftable'" (??_7DeleteAccessPolicyRequest@v1@accesscontextmanager@identity@google@@6B@) referenced in function "public: __cdecl google::identity::accesscontextmanager::v1::DeleteAccessPolicyRequest::DeleteAccessPolicyRequest(void)" (??0DeleteAccessPolicyRequest@v1@accesscontextmanager@identity@google@@QEAA@XZ)
access_context_manager_connection_impl.cc.obj : error LNK2001: unresolved external symbol "const google::identity::accesscontextmanager::v1::DeleteAccessPolicyRequest::`vftable'" (??_7DeleteAccessPolicyRequest@v1@accesscontextmanager@identity@google@@6B@)
[etc.]

[h-vetinari]

So I started looking at this again, here's the list of missing symbols (from the last build, as of 2.11.0):

const google::cloud::accessapproval::v1::AccessApprovalServiceAccount::vftable
const google::cloud::accessapproval::v1::AccessApprovalSettings::vftable
const google::cloud::accessapproval::v1::AccessApprovalSettingsDefaultTypeInternal
const google::cloud::accessapproval::v1::ApprovalRequest::vftable
const google::cloud::accessapproval::v1::DeleteAccessApprovalSettingsMessage::vftable
const google::cloud::accessapproval::v1::GetAccessApprovalServiceAccountMessage::vftable
const google::cloud::accessapproval::v1::GetAccessApprovalSettingsMessage::vftable
const google::cloud::accessapproval::v1::GetApprovalRequestMessage::vftable
const google::cloud::accessapproval::v1::ListApprovalRequestsMessage::vftable
const google::cloud::accessapproval::v1::ListApprovalRequestsResponse::vftable
const google::cloud::accessapproval::v1::UpdateAccessApprovalSettingsMessage::vftable

Normally, I know roughly how to tackle this - add a switch for __declspec(dllexport) / __declspec(dllimport), mark the respective classes, etc. - however, I cannot find the definitions of many of those classes.

I believe that a lot of those come through accessapproval.pb.h, which presumably gets generated from accessapproval.proto, which however comes from a separate repo. That makes patching here a bunch harder, though I guess we could override EXTERNAL_GOOGLEAPIS_SOURCE to point to a patched local copy.

The key issue however is how to pass the required information through the protobuf-generation step. I don't have any experience there, but I doubt that protobuf has capabilities for that? Overall, this still looks like a very tall order...